From d3e258c43173621837802d4a8cc8acb93835b590 Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Tue, 3 Dec 2019 14:35:36 +0300 Subject: [PATCH 1/3] Reorganize queries and indexes for internal_transactions table --- CHANGELOG.md | 2 + .../api/rpc/address_controller_test.exs | 8 +- ...n_internal_transaction_controller_test.exs | 3 +- apps/explorer/lib/explorer/chain.ex | 66 +++++++++++- .../explorer/chain/internal_transaction.ex | 12 +++ apps/explorer/lib/explorer/etherscan.ex | 102 ++++++++++++++---- ...transactions_add_to_address_hash_index.exs | 25 +++++ apps/explorer/test/explorer/chain_test.exs | 26 +++-- .../explorer/test/explorer/etherscan_test.exs | 16 +-- 9 files changed, 218 insertions(+), 42 deletions(-) create mode 100644 apps/explorer/priv/repo/migrations/20191203112646_internal_transactions_add_to_address_hash_index.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 62ce88bd8d..ad65137203 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixes +- [2910](https://github.com/poanetwork/blockscout/pull/2910) - Reorganize queries and indexes for internal_transactions table + ### Chore - [#2896](https://github.com/poanetwork/blockscout/pull/2896) - Disable Parity websockets tests diff --git a/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/address_controller_test.exs b/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/address_controller_test.exs index 135fbe9948..66b98cc661 100644 --- a/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/address_controller_test.exs +++ b/apps/block_scout_web/test/block_scout_web/controllers/api/rpc/address_controller_test.exs @@ -1606,7 +1606,7 @@ defmodule BlockScoutWeb.API.RPC.AddressControllerTest do internal_transaction = :internal_transaction_create - |> insert(transaction: transaction, index: 0, from_address: address) + |> insert(transaction: transaction, index: 0, from_address: address, block_number: block.number) |> with_contract_creation(contract_address) params = %{ @@ -1658,7 +1658,8 @@ defmodule BlockScoutWeb.API.RPC.AddressControllerTest do transaction: transaction, index: 0, type: :reward, - error: "some error" + error: "some error", + block_number: transaction.block_number ] insert(:internal_transaction_create, internal_transaction_details) @@ -1693,7 +1694,8 @@ defmodule BlockScoutWeb.API.RPC.AddressControllerTest do internal_transaction_details = %{ from_address: address, transaction: transaction, - index: index + index: index, + block_number: transaction.block_number } insert(:internal_transaction_create, internal_transaction_details) diff --git a/apps/block_scout_web/test/block_scout_web/controllers/transaction_internal_transaction_controller_test.exs b/apps/block_scout_web/test/block_scout_web/controllers/transaction_internal_transaction_controller_test.exs index 773b51a36c..9d6093e468 100644 --- a/apps/block_scout_web/test/block_scout_web/controllers/transaction_internal_transaction_controller_test.exs +++ b/apps/block_scout_web/test/block_scout_web/controllers/transaction_internal_transaction_controller_test.exs @@ -64,7 +64,8 @@ defmodule BlockScoutWeb.TransactionInternalTransactionControllerTest do assert json_response(conn, 200) - assert Enum.count(items) == 2 + # excluding of internal transactions with type=call and index=0 + assert Enum.count(items) == 1 end test "includes USD exchange rate value for address in assigns", %{conn: conn} do diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index 9576eca1a6..e7fdf09330 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -15,6 +15,7 @@ defmodule Explorer.Chain do preload: 2, select: 2, subquery: 1, + union: 2, union_all: 2, where: 2, where: 3 @@ -193,8 +194,65 @@ defmodule Explorer.Chain do direction = Keyword.get(options, :direction) paging_options = Keyword.get(options, :paging_options, @default_paging_options) - InternalTransaction - |> InternalTransaction.where_address_fields_match(hash, direction) + if direction == nil do + query_to_address_hash_wrapped = + InternalTransaction + |> InternalTransaction.where_address_fields_match(hash, :to_address_hash) + |> common_where_limit_order(paging_options) + |> wrapped_union_subquery() + + query_from_address_hash_wrapped = + InternalTransaction + |> InternalTransaction.where_address_fields_match(hash, :from_address_hash) + |> common_where_limit_order(paging_options) + |> wrapped_union_subquery() + + query_created_contract_address_hash_wrapped = + InternalTransaction + |> InternalTransaction.where_address_fields_match(hash, :created_contract_address_hash) + |> common_where_limit_order(paging_options) + |> wrapped_union_subquery() + + full_query = + query_to_address_hash_wrapped + |> union(^query_from_address_hash_wrapped) + |> union(^query_created_contract_address_hash_wrapped) + + full_wrapped_query = + from( + q in subquery(full_query), + select: q + ) + + full_wrapped_query + |> order_by( + [q], + desc: q.block_number, + desc: q.transaction_index, + desc: q.index + ) + |> preload(transaction: :block) + |> join_associations(necessity_by_association) + |> Repo.all() + else + InternalTransaction + |> InternalTransaction.where_address_fields_match(hash, direction) + |> common_where_limit_order(paging_options) + |> preload(transaction: :block) + |> join_associations(necessity_by_association) + |> Repo.all() + end + end + + def wrapped_union_subquery(query) do + from( + q in subquery(query), + select: q + ) + end + + defp common_where_limit_order(query, paging_options) do + query |> InternalTransaction.where_is_different_from_parent_transaction() |> InternalTransaction.where_block_number_is_not_null() |> page_internal_transaction(paging_options) @@ -205,9 +263,6 @@ defmodule Explorer.Chain do desc: it.transaction_index, desc: it.index ) - |> preload(transaction: :block) - |> join_associations(necessity_by_association) - |> Repo.all() end @doc """ @@ -2438,6 +2493,7 @@ defmodule Explorer.Chain do |> for_parent_transaction(hash) |> join_associations(necessity_by_association) |> where_transaction_has_multiple_internal_transactions() + |> InternalTransaction.where_is_different_from_parent_transaction() |> page_internal_transaction(paging_options) |> limit(^paging_options.page_size) |> order_by([internal_transaction], asc: internal_transaction.index) diff --git a/apps/explorer/lib/explorer/chain/internal_transaction.ex b/apps/explorer/lib/explorer/chain/internal_transaction.ex index 77f7d15da2..b293035284 100644 --- a/apps/explorer/lib/explorer/chain/internal_transaction.ex +++ b/apps/explorer/lib/explorer/chain/internal_transaction.ex @@ -491,6 +491,18 @@ defmodule Explorer.Chain.InternalTransaction do ) end + def where_address_fields_match(query, address_hash, :to_address_hash) do + where(query, [it], it.to_address_hash == ^address_hash) + end + + def where_address_fields_match(query, address_hash, :from_address_hash) do + where(query, [it], it.from_address_hash == ^address_hash) + end + + def where_address_fields_match(query, address_hash, :created_contract_address_hash) do + where(query, [it], it.created_contract_address_hash == ^address_hash) + end + def where_is_different_from_parent_transaction(query) do where( query, diff --git a/apps/explorer/lib/explorer/etherscan.ex b/apps/explorer/lib/explorer/etherscan.ex index 71ee5794f7..a828ee19cb 100644 --- a/apps/explorer/lib/explorer/etherscan.ex +++ b/apps/explorer/lib/explorer/etherscan.ex @@ -3,7 +3,7 @@ defmodule Explorer.Etherscan do The etherscan context. """ - import Ecto.Query, only: [from: 2, where: 3, or_where: 3] + import Ecto.Query, only: [from: 2, where: 3, or_where: 3, union: 2] alias Explorer.Etherscan.Logs alias Explorer.{Chain, Repo} @@ -97,6 +97,7 @@ defmodule Explorer.Etherscan do query |> Chain.where_transaction_has_multiple_internal_transactions() + |> InternalTransaction.where_is_different_from_parent_transaction() |> Repo.all() end @@ -120,27 +121,90 @@ defmodule Explorer.Etherscan do ) do options = Map.merge(@default_options, raw_options) - query = + direction = + case options do + %{filter_by: "to"} -> :to + %{filter_by: "from"} -> :from + _ -> nil + end + + consensus_blocks = from( - it in InternalTransaction, - inner_join: t in assoc(it, :transaction), - inner_join: b in assoc(t, :block), - order_by: [{^options.order_by_direction, t.block_number}], - limit: ^options.page_size, - offset: ^offset(options), - select: - merge(map(it, ^@internal_transaction_fields), %{ - block_timestamp: b.timestamp, - block_number: b.number - }) + b in Block, + where: b.consensus == true ) - query - |> Chain.where_transaction_has_multiple_internal_transactions() - |> where_address_match(address_hash, options) - |> where_start_block_match(options) - |> where_end_block_match(options) - |> Repo.all() + if direction == nil do + query = + from( + it in InternalTransaction, + inner_join: b in subquery(consensus_blocks), + on: it.block_number == b.number, + order_by: [ + {^options.order_by_direction, it.block_number}, + {:desc, it.transaction_index}, + {:desc, it.index} + ], + limit: ^options.page_size, + offset: ^offset(options), + select: + merge(map(it, ^@internal_transaction_fields), %{ + block_timestamp: b.timestamp, + block_number: b.number + }) + ) + + query_to_address_hash_wrapped = + query + |> InternalTransaction.where_address_fields_match(address_hash, :to_address_hash) + |> InternalTransaction.where_is_different_from_parent_transaction() + |> where_start_block_match(options) + |> where_end_block_match(options) + |> Chain.wrapped_union_subquery() + + query_from_address_hash_wrapped = + query + |> InternalTransaction.where_address_fields_match(address_hash, :from_address_hash) + |> InternalTransaction.where_is_different_from_parent_transaction() + |> where_start_block_match(options) + |> where_end_block_match(options) + |> Chain.wrapped_union_subquery() + + query_created_contract_address_hash_wrapped = + query + |> InternalTransaction.where_address_fields_match(address_hash, :created_contract_address_hash) + |> InternalTransaction.where_is_different_from_parent_transaction() + |> where_start_block_match(options) + |> where_end_block_match(options) + |> Chain.wrapped_union_subquery() + + query_to_address_hash_wrapped + |> union(^query_from_address_hash_wrapped) + |> union(^query_created_contract_address_hash_wrapped) + |> Repo.all() + else + query = + from( + it in InternalTransaction, + inner_join: t in assoc(it, :transaction), + inner_join: b in assoc(t, :block), + order_by: [{^options.order_by_direction, t.block_number}], + limit: ^options.page_size, + offset: ^offset(options), + select: + merge(map(it, ^@internal_transaction_fields), %{ + block_timestamp: b.timestamp, + block_number: b.number + }) + ) + + query + |> InternalTransaction.where_address_fields_match(address_hash, direction) + |> InternalTransaction.where_is_different_from_parent_transaction() + |> where_start_block_match(options) + |> where_end_block_match(options) + |> Repo.all() + end end @doc """ diff --git a/apps/explorer/priv/repo/migrations/20191203112646_internal_transactions_add_to_address_hash_index.exs b/apps/explorer/priv/repo/migrations/20191203112646_internal_transactions_add_to_address_hash_index.exs new file mode 100644 index 0000000000..f0ab878760 --- /dev/null +++ b/apps/explorer/priv/repo/migrations/20191203112646_internal_transactions_add_to_address_hash_index.exs @@ -0,0 +1,25 @@ +defmodule Explorer.Repo.Migrations.InternalTransactionsAddToAddressHashIndex do + use Ecto.Migration + + def change do + execute( + "CREATE INDEX IF NOT EXISTS internal_transactions_from_address_hash_partial_index on public.internal_transactions(from_address_hash, block_number DESC, transaction_index DESC, index DESC) WHERE (((type = 'call') AND (index > 0)) OR (type != 'call'));" + ) + + execute( + "CREATE INDEX IF NOT EXISTS internal_transactions_to_address_hash_partial_index on public.internal_transactions(to_address_hash, block_number DESC, transaction_index DESC, index DESC) WHERE (((type = 'call') AND (index > 0)) OR (type != 'call'));" + ) + + execute( + "CREATE INDEX IF NOT EXISTS internal_transactions_created_contract_address_hash_partial_index on public.internal_transactions(created_contract_address_hash, block_number DESC, transaction_index DESC, index DESC) WHERE (((type = 'call') AND (index > 0)) OR (type != 'call'));" + ) + + drop_if_exists( + index( + :internal_transactions, + [:to_address_hash, :from_address_hash, :created_contract_address_hash, :type, :index], + name: "internal_transactions_to_address_hash_from_address_hash_created" + ) + ) + end +end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 5aa8a6b1d2..913930e828 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -2225,7 +2225,8 @@ defmodule Explorer.ChainTest do results = [internal_transaction | _] = Chain.transaction_to_internal_transactions(transaction.hash) - assert 2 == length(results) + # excluding of internal transactions with type=call and index=0 + assert 1 == length(results) assert Enum.all?( results, @@ -2360,7 +2361,7 @@ defmodule Explorer.ChainTest do |> insert() |> with_block() - %InternalTransaction{transaction_hash: first_transaction_hash, index: first_index} = + %InternalTransaction{transaction_hash: _, index: _} = insert(:internal_transaction, transaction: transaction, index: 0, @@ -2381,7 +2382,8 @@ defmodule Explorer.ChainTest do |> Chain.transaction_to_internal_transactions() |> Enum.map(&{&1.transaction_hash, &1.index}) - assert [{first_transaction_hash, first_index}, {second_transaction_hash, second_index}] == result + # excluding of internal transactions with type=call and index=0 + assert [{second_transaction_hash, second_index}] == result end test "pages by index" do @@ -2390,7 +2392,7 @@ defmodule Explorer.ChainTest do |> insert() |> with_block() - %InternalTransaction{transaction_hash: first_transaction_hash, index: first_index} = + %InternalTransaction{transaction_hash: _, index: _} = insert(:internal_transaction, transaction: transaction, index: 0, @@ -2406,19 +2408,27 @@ defmodule Explorer.ChainTest do transaction_index: transaction.index ) - assert [{first_transaction_hash, first_index}, {second_transaction_hash, second_index}] == + %InternalTransaction{transaction_hash: third_transaction_hash, index: third_index} = + insert(:internal_transaction, + transaction: transaction, + index: 2, + block_number: transaction.block_number, + transaction_index: transaction.index + ) + + assert [{second_transaction_hash, second_index}, {third_transaction_hash, third_index}] == transaction.hash |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {-1}, page_size: 2}) |> Enum.map(&{&1.transaction_hash, &1.index}) - assert [{first_transaction_hash, first_index}] == + assert [{second_transaction_hash, second_index}] == transaction.hash |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {-1}, page_size: 1}) |> Enum.map(&{&1.transaction_hash, &1.index}) - assert [{second_transaction_hash, second_index}] == + assert [{third_transaction_hash, third_index}] == transaction.hash - |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {0}, page_size: 2}) + |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {1}, page_size: 2}) |> Enum.map(&{&1.transaction_hash, &1.index}) end end diff --git a/apps/explorer/test/explorer/etherscan_test.exs b/apps/explorer/test/explorer/etherscan_test.exs index 5d1cf7fcb1..e2e9f0c273 100644 --- a/apps/explorer/test/explorer/etherscan_test.exs +++ b/apps/explorer/test/explorer/etherscan_test.exs @@ -533,7 +533,8 @@ defmodule Explorer.EtherscanTest do found_internal_transactions = Etherscan.list_internal_transactions(transaction.hash) - assert length(found_internal_transactions) == 3 + # excluding of internal transactions with type=call and index=0 + assert length(found_internal_transactions) == 2 end test "only returns internal transactions that belong to the transaction" do @@ -571,7 +572,8 @@ defmodule Explorer.EtherscanTest do internal_transactions1 = Etherscan.list_internal_transactions(transaction1.hash) - assert length(internal_transactions1) == 2 + # excluding of internal transactions with type=call and index=0 + assert length(internal_transactions1) == 1 internal_transactions2 = Etherscan.list_internal_transactions(transaction2.hash) @@ -659,7 +661,7 @@ defmodule Explorer.EtherscanTest do |> insert() |> with_block() - for index <- 0..2 do + for index <- 0..3 do internal_transaction_details = %{ transaction: transaction, index: index, @@ -719,7 +721,8 @@ defmodule Explorer.EtherscanTest do internal_transactions1 = Etherscan.list_internal_transactions(address1.hash) - assert length(internal_transactions1) == 3 + # excluding of internal transactions with type=call and index=0 + assert length(internal_transactions1) == 2 internal_transactions2 = Etherscan.list_internal_transactions(address2.hash) @@ -734,7 +737,7 @@ defmodule Explorer.EtherscanTest do |> insert() |> with_block() - for index <- 0..2 do + for index <- 0..3 do internal_transaction_details = %{ transaction: transaction, index: index, @@ -795,7 +798,8 @@ defmodule Explorer.EtherscanTest do expected_block_numbers = [second_block.number, third_block.number] - assert length(found_internal_transactions) == 4 + # excluding of internal transactions with type=call and index=0 + assert length(found_internal_transactions) == 2 for internal_transaction <- found_internal_transactions do assert internal_transaction.block_number in expected_block_numbers From 6ec1ac1a7e54e2d24afedd30347094353a69e66a Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Sun, 8 Dec 2019 11:55:33 +0300 Subject: [PATCH 2/3] Modify logs query --- apps/explorer/lib/explorer/etherscan/logs.ex | 57 +++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/apps/explorer/lib/explorer/etherscan/logs.ex b/apps/explorer/lib/explorer/etherscan/logs.ex index 336eae106b..066daf74ef 100644 --- a/apps/explorer/lib/explorer/etherscan/logs.ex +++ b/apps/explorer/lib/explorer/etherscan/logs.ex @@ -5,8 +5,9 @@ defmodule Explorer.Etherscan.Logs do """ - import Ecto.Query, only: [from: 2, where: 3, subquery: 1, order_by: 3] + import Ecto.Query, only: [from: 2, where: 3, subquery: 1, order_by: 3, union: 2] + alias Explorer.Chain alias Explorer.Chain.{Block, InternalTransaction, Log, Transaction} alias Explorer.Repo @@ -78,25 +79,25 @@ defmodule Explorer.Etherscan.Logs do logs_query = where_topic_match(Log, prepared_filter) + query_to_address_hash_wrapped = + logs_query + |> internal_transaction_query(:to_address_hash, prepared_filter, address_hash) + |> Chain.wrapped_union_subquery() + + query_from_address_hash_wrapped = + logs_query + |> internal_transaction_query(:from_address_hash, prepared_filter, address_hash) + |> Chain.wrapped_union_subquery() + + query_created_contract_address_hash_wrapped = + logs_query + |> internal_transaction_query(:created_contract_address_hash, prepared_filter, address_hash) + |> Chain.wrapped_union_subquery() + internal_transaction_log_query = - from(internal_transaction in InternalTransaction, - join: transaction in assoc(internal_transaction, :transaction), - join: log in ^logs_query, - on: log.transaction_hash == internal_transaction.transaction_hash, - where: internal_transaction.block_number >= ^prepared_filter.from_block, - where: internal_transaction.block_number <= ^prepared_filter.to_block, - where: - internal_transaction.to_address_hash == ^address_hash or - internal_transaction.from_address_hash == ^address_hash or - internal_transaction.created_contract_address_hash == ^address_hash, - select: - merge(map(log, ^@log_fields), %{ - gas_price: transaction.gas_price, - gas_used: transaction.gas_used, - transaction_index: transaction.index, - block_number: transaction.block_number - }) - ) + query_to_address_hash_wrapped + |> union(^query_from_address_hash_wrapped) + |> union(^query_created_contract_address_hash_wrapped) all_transaction_logs_query = from(transaction in Transaction, @@ -256,4 +257,22 @@ defmodule Explorer.Etherscan.Logs do data.transaction_index >= ^transaction_index ) end + + defp internal_transaction_query(logs_query, direction, prepared_filter, address_hash) do + from(internal_transaction in InternalTransaction, + join: transaction in assoc(internal_transaction, :transaction), + join: log in ^logs_query, + on: log.transaction_hash == internal_transaction.transaction_hash, + where: internal_transaction.block_number >= ^prepared_filter.from_block, + where: internal_transaction.block_number <= ^prepared_filter.to_block, + select: + merge(map(log, ^@log_fields), %{ + gas_price: transaction.gas_price, + gas_used: transaction.gas_used, + transaction_index: transaction.index, + block_number: transaction.block_number + }) + ) + |> InternalTransaction.where_address_fields_match(address_hash, direction) + end end From 2271bc69efb96a5e1ae9e8bef438a134adfb8a9a Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Sat, 4 Jan 2020 23:20:48 +0300 Subject: [PATCH 3/3] Fix tests --- apps/explorer/lib/explorer/etherscan/logs.ex | 34 +++++++++++--------- apps/explorer/test/explorer/chain_test.exs | 2 ++ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/apps/explorer/lib/explorer/etherscan/logs.ex b/apps/explorer/lib/explorer/etherscan/logs.ex index b4423981d7..35dbfda1ba 100644 --- a/apps/explorer/lib/explorer/etherscan/logs.ex +++ b/apps/explorer/lib/explorer/etherscan/logs.ex @@ -7,9 +7,8 @@ defmodule Explorer.Etherscan.Logs do import Ecto.Query, only: [from: 2, where: 3, subquery: 1, order_by: 3, union: 2] - alias Explorer.Chain + alias Explorer.{Chain, Repo} alias Explorer.Chain.{Block, InternalTransaction, Log, Transaction} - alias Explorer.Repo @base_filter %{ from_block: nil, @@ -259,20 +258,23 @@ defmodule Explorer.Etherscan.Logs do end defp internal_transaction_query(logs_query, direction, prepared_filter, address_hash) do - from(internal_transaction in InternalTransaction.where_nonpending_block(), - join: transaction in assoc(internal_transaction, :transaction), - join: log in ^logs_query, - on: log.transaction_hash == internal_transaction.transaction_hash, - where: internal_transaction.block_number >= ^prepared_filter.from_block, - where: internal_transaction.block_number <= ^prepared_filter.to_block, - select: - merge(map(log, ^@log_fields), %{ - gas_price: transaction.gas_price, - gas_used: transaction.gas_used, - transaction_index: transaction.index, - block_number: transaction.block_number - }) - ) + query = + from(internal_transaction in InternalTransaction.where_nonpending_block(), + join: transaction in assoc(internal_transaction, :transaction), + join: log in ^logs_query, + on: log.transaction_hash == internal_transaction.transaction_hash, + where: internal_transaction.block_number >= ^prepared_filter.from_block, + where: internal_transaction.block_number <= ^prepared_filter.to_block, + select: + merge(map(log, ^@log_fields), %{ + gas_price: transaction.gas_price, + gas_used: transaction.gas_used, + transaction_index: transaction.index, + block_number: transaction.block_number + }) + ) + + query |> InternalTransaction.where_address_fields_match(address_hash, direction) end end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 52ca4fe016..b37edbc36e 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -2598,6 +2598,8 @@ defmodule Explorer.ChainTest do transaction: transaction, index: 2, block_number: transaction.block_number, + block_hash: transaction.block_hash, + block_index: 2, transaction_index: transaction.index )