From 025ed91dcb1222a0eb2ddd3a9aa3bf37ef4b4352 Mon Sep 17 00:00:00 2001 From: Stamates Date: Fri, 6 Jul 2018 16:25:45 -0400 Subject: [PATCH 1/4] Fix multiple results in Repo.One with multiple create type internal transactions --- apps/explorer/lib/explorer/chain.ex | 42 +++++++++++++--------- apps/explorer/test/explorer/chain_test.exs | 16 +++++++++ 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index 0ac1f2bcd7..43dba49efb 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -7,7 +7,6 @@ defmodule Explorer.Chain do only: [ from: 2, join: 4, - join: 5, limit: 2, order_by: 2, order_by: 3, @@ -459,7 +458,7 @@ defmodule Explorer.Chain do |> Keyword.get(:paging_options, @default_paging_options) |> fetch_transactions() |> join(:inner, [transaction], block in assoc(transaction, :block)) - |> where([_, _, block], block.hash == ^block_hash) + |> where([_, block], block.hash == ^block_hash) |> join_associations(necessity_by_association) |> Repo.all() end @@ -2258,9 +2257,20 @@ defmodule Explorer.Chain do defp fetch_transactions(paging_options \\ nil) do Transaction - |> load_contract_creation() - |> select_merge([_, internal_transaction], %{ - created_contract_address_hash: internal_transaction.created_contract_address_hash + |> select_merge([transaction], %{ + created_contract_address_hash: + type( + fragment( + ~s[ + (SELECT i."created_contract_address_hash" + FROM "internal_transactions" AS i + WHERE (i."transaction_hash" = ?) AND (i."type" = 'create') + LIMIT 1) + ], + transaction.hash + ), + Explorer.Chain.Hash.Truncated + ) }) |> order_by([transaction], desc: transaction.block_number, desc: transaction.index) |> handle_paging_options(paging_options) @@ -2508,16 +2518,6 @@ defmodule Explorer.Chain do end) end - defp load_contract_creation(query) do - join( - query, - :left, - [transaction], - internal_transaction in assoc(transaction, :internal_transactions), - internal_transaction.type == ^:create - ) - end - defp page_blocks(query, %PagingOptions{key: nil}), do: query defp page_blocks(query, %PagingOptions{key: {block_number}}) do @@ -2705,9 +2705,17 @@ defmodule Explorer.Chain do defp where_address_fields_match(%Ecto.Query{from: {_table, Transaction}} = query, address_hash, nil) do where( query, - [t, it], + [t], t.to_address_hash == ^address_hash or t.from_address_hash == ^address_hash or - it.created_contract_address_hash == ^address_hash + ^address_hash.bytes in fragment( + ~s[ + (SELECT i."created_contract_address_hash" + FROM "internal_transactions" AS i + WHERE (i."transaction_hash" = ?) AND (i."type" = 'create') + LIMIT 1) + ], + t.hash + ) ) end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index c5966f1692..20bbe7d323 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -335,6 +335,22 @@ defmodule Explorer.ChainTest do necessity_by_association: %{block: :required} ) end + + test "transaction with multiple create internal transactions is returned" do + transaction = + %Transaction{hash: hash_with_block} = + :transaction + |> insert() + |> with_block() + + insert(:internal_transaction, transaction: transaction, index: 0) + + Enum.each(1..3, fn index -> + insert(:internal_transaction_create, transaction: transaction, index: index) + end) + + assert {:ok, %Transaction{hash: ^hash_with_block}} = Chain.hash_to_transaction(hash_with_block) + end end describe "list_blocks/2" do From 4da29590143f0ae3880c7dc4b2f29ed14d68b575 Mon Sep 17 00:00:00 2001 From: Tim Mecklem Date: Mon, 9 Jul 2018 16:21:15 -0400 Subject: [PATCH 2/4] Exclude non-contract-creation parent transactions from contract page When a parent transaction executes a contract that creates other contracts, those contract address pages should not include the parent transaction in the transactions tab since it has non-matching from and to addresses. --- apps/explorer/lib/explorer/chain.ex | 11 +++---- apps/explorer/test/explorer/chain_test.exs | 35 ++++++++++++++++++---- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index 43dba49efb..35c2374a59 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -146,7 +146,7 @@ defmodule Explorer.Chain do SELECT t0."hash" address FROM "transactions" AS t0 LEFT OUTER JOIN "internal_transactions" AS i1 ON (i1."transaction_hash" = t0."hash") AND (i1."type" = 'create') - WHERE (i1."created_contract_address_hash" = $1) + WHERE (i1."created_contract_address_hash" = $1 AND t0."to_address_hash" IS NULL) UNION @@ -2707,15 +2707,16 @@ defmodule Explorer.Chain do query, [t], t.to_address_hash == ^address_hash or t.from_address_hash == ^address_hash or - ^address_hash.bytes in fragment( - ~s[ + (is_nil(t.to_address_hash) and + ^address_hash.bytes in fragment( + ~s[ (SELECT i."created_contract_address_hash" FROM "internal_transactions" AS i WHERE (i."transaction_hash" = ?) AND (i."type" = 'create') LIMIT 1) ], - t.hash - ) + t.hash + )) ) end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 20bbe7d323..fde59dcc6d 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -23,18 +23,17 @@ defmodule Explorer.ChainTest do assert Chain.address_to_transaction_count(address) == 2 end - test "with transactions and contract creation address" do - %Transaction{from_address: address} = insert(:transaction) |> Repo.preload(:from_address) - insert(:transaction, to_address: address) + test "with contract creation transactions the contract address is counted" do + address = insert(:address) insert( :internal_transaction_create, created_contract_address: address, index: 0, - transaction: insert(:transaction) + transaction: insert(:transaction, to_address: nil) ) - assert Chain.address_to_transaction_count(address) == 3 + assert Chain.address_to_transaction_count(address) == 1 end test "doesn't double count addresses when to_address = from_address" do @@ -43,6 +42,19 @@ defmodule Explorer.ChainTest do assert Chain.address_to_transaction_count(address) == 2 end + + test "does not count non-contract-creation parent transactions" do + transaction_with_to_address = + %Transaction{} = + :transaction + |> insert() + |> with_block() + + %InternalTransaction{created_contract_address: address} = + insert(:internal_transaction_create, transaction: transaction_with_to_address, index: 0) + + assert Chain.address_to_transaction_count(address) == 0 + end end describe "address_to_transactions/2" do @@ -104,6 +116,19 @@ defmodule Explorer.ChainTest do Chain.address_to_transactions(address) |> Repo.preload([:block, :to_address, :from_address]) end + test "does not include non-contract-creation parent transactions" do + transaction = + %Transaction{} = + :transaction + |> insert() + |> with_block() + + %InternalTransaction{created_contract_address: address} = + insert(:internal_transaction_create, transaction: transaction, index: 0) + + assert [] == Chain.address_to_transactions(address) + end + test "with transactions can be paginated" do address = insert(:address) From 8efdf3e0e8c4a4ef3415a94af57cf49ae61b452f Mon Sep 17 00:00:00 2001 From: Stamates Date: Tue, 10 Jul 2018 14:03:59 -0400 Subject: [PATCH 3/4] Sort internal_transactions oldest to newest on transaction page --- apps/explorer/lib/explorer/chain.ex | 4 ++-- apps/explorer/test/explorer/chain_test.exs | 4 ++-- .../transaction_internal_transaction/index.html.eex | 2 +- .../transaction_internal_transaction_controller_test.exs | 7 +++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index 35c2374a59..a9ecb293d8 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -2106,7 +2106,7 @@ defmodule Explorer.Chain do |> where_transaction_has_multiple_internal_transactions() |> page_internal_transaction(paging_options) |> limit(^paging_options.page_size) - |> order_by([internal_transaction], desc: internal_transaction.index) + |> order_by([internal_transaction], asc: internal_transaction.index) |> Repo.all() end @@ -2538,7 +2538,7 @@ defmodule Explorer.Chain do end defp page_internal_transaction(query, %PagingOptions{key: {index}}) do - where(query, [internal_transaction], internal_transaction.index < ^index) + where(query, [internal_transaction], internal_transaction.index > ^index) end defp page_logs(query, %PagingOptions{key: nil}), do: query diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index fde59dcc6d..7b7fb27088 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -701,7 +701,7 @@ defmodule Explorer.ChainTest do assert actual.id == expected.id end - test "returns the internal transactions in descending index order" do + test "returns the internal transactions in ascending index order" do transaction = :transaction |> insert() @@ -715,7 +715,7 @@ defmodule Explorer.ChainTest do |> Chain.transaction_to_internal_transactions() |> Enum.map(& &1.id) - assert [second_id, first_id] == result + assert [first_id, second_id] == result end end diff --git a/apps/explorer_web/lib/explorer_web/templates/transaction_internal_transaction/index.html.eex b/apps/explorer_web/lib/explorer_web/templates/transaction_internal_transaction/index.html.eex index 162532dab4..be10a9dcef 100644 --- a/apps/explorer_web/lib/explorer_web/templates/transaction_internal_transaction/index.html.eex +++ b/apps/explorer_web/lib/explorer_web/templates/transaction_internal_transaction/index.html.eex @@ -68,7 +68,7 @@ <%= if @next_page_params do %> <%= link( - gettext("Older"), + gettext("Newer"), class: "button button--secondary button--sm u-float-right mt-3", to: transaction_internal_transaction_path( @conn, diff --git a/apps/explorer_web/test/explorer_web/controllers/transaction_internal_transaction_controller_test.exs b/apps/explorer_web/test/explorer_web/controllers/transaction_internal_transaction_controller_test.exs index a1444ed2f8..d3f44fb39b 100644 --- a/apps/explorer_web/test/explorer_web/controllers/transaction_internal_transaction_controller_test.exs +++ b/apps/explorer_web/test/explorer_web/controllers/transaction_internal_transaction_controller_test.exs @@ -88,13 +88,13 @@ defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do |> insert() |> with_block() + %InternalTransaction{index: index} = insert(:internal_transaction, transaction: transaction, index: 0) + second_page_indexes = 1..50 |> Enum.map(fn index -> insert(:internal_transaction, transaction: transaction, index: index) end) |> Enum.map(& &1.index) - %InternalTransaction{index: index} = insert(:internal_transaction, transaction: transaction, index: 51) - conn = get(conn, transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, transaction.hash), %{ "index" => Integer.to_string(index) @@ -103,7 +103,6 @@ defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do actual_indexes = conn.assigns.internal_transactions |> Enum.map(& &1.index) - |> Enum.reverse() assert second_page_indexes == actual_indexes end @@ -128,7 +127,7 @@ defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do conn = get(conn, transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, transaction.hash)) - assert %{"block_number" => ^number, "index" => 11, "transaction_index" => ^transaction_index} = + assert %{"block_number" => ^number, "index" => 50, "transaction_index" => ^transaction_index} = conn.assigns.next_page_params end From 504f81f3f772dc46d5c7494a140917315663041a Mon Sep 17 00:00:00 2001 From: Stamates Date: Tue, 10 Jul 2018 14:08:57 -0400 Subject: [PATCH 4/4] Internationalization --- apps/explorer_web/priv/gettext/default.pot | 2 +- apps/explorer_web/priv/gettext/en/LC_MESSAGES/default.po | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/explorer_web/priv/gettext/default.pot b/apps/explorer_web/priv/gettext/default.pot index 839778c791..d4fbce011d 100644 --- a/apps/explorer_web/priv/gettext/default.pot +++ b/apps/explorer_web/priv/gettext/default.pot @@ -494,7 +494,6 @@ msgstr "" #: lib/explorer_web/templates/block_transaction/index.html.eex:198 #: lib/explorer_web/templates/pending_transaction/index.html.eex:73 #: lib/explorer_web/templates/transaction/index.html.eex:90 -#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71 msgid "Older" msgstr "" @@ -525,6 +524,7 @@ msgid "Last Updated In Block" msgstr "" #, elixir-format +#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71 #: lib/explorer_web/templates/transaction_log/index.html.eex:73 msgid "Newer" msgstr "" diff --git a/apps/explorer_web/priv/gettext/en/LC_MESSAGES/default.po b/apps/explorer_web/priv/gettext/en/LC_MESSAGES/default.po index b52a441038..0ce6ae4fe4 100644 --- a/apps/explorer_web/priv/gettext/en/LC_MESSAGES/default.po +++ b/apps/explorer_web/priv/gettext/en/LC_MESSAGES/default.po @@ -506,7 +506,6 @@ msgstr "" #: lib/explorer_web/templates/block_transaction/index.html.eex:198 #: lib/explorer_web/templates/pending_transaction/index.html.eex:73 #: lib/explorer_web/templates/transaction/index.html.eex:90 -#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71 msgid "Older" msgstr "" @@ -537,6 +536,7 @@ msgid "Last Updated In Block" msgstr "" #, elixir-format +#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71 #: lib/explorer_web/templates/transaction_log/index.html.eex:73 msgid "Newer" msgstr ""