From 299404ab52bac02143ebb67017c996733d335744 Mon Sep 17 00:00:00 2001 From: Amanda Sposito Date: Wed, 31 Oct 2018 14:23:59 -0300 Subject: [PATCH 1/2] Remove the extra queries from address_to_transactions * The query was performing a left outer join with the token transfers query in order to identify if there were a token transfer related to the address that wasn't a transaction, we removed this join in favor of a match in the where. * Add new index to token_transfers table to improve the where performance. Co-authored-by: Lucas Narciso --- .../address_transaction_controller.ex | 7 +- .../templates/transaction/_tile.html.eex | 5 +- apps/explorer/lib/explorer/chain.ex | 38 +++------ .../lib/explorer/chain/token_transfer.ex | 59 ++++++++++--- .../lib/explorer/chain/transaction.ex | 28 ++++++- apps/explorer/test/explorer/chain_test.exs | 84 ++++++++++++++----- 6 files changed, 158 insertions(+), 63 deletions(-) diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/address_transaction_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/address_transaction_controller.ex index d33d54ccb8..f719d6073d 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/address_transaction_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/address_transaction_controller.ex @@ -19,7 +19,10 @@ defmodule BlockScoutWeb.AddressTransactionController do [created_contract_address: :names] => :optional, [from_address: :names] => :optional, [to_address: :names] => :optional, - :token_transfers => :optional + [token_transfers: :token] => :optional, + [token_transfers: :to_address] => :optional, + [token_transfers: :from_address] => :optional, + [token_transfers: :token_contract_address] => :optional } ] @@ -81,7 +84,7 @@ defmodule BlockScoutWeb.AddressTransactionController do {:ok, address} <- Chain.hash_to_address(address_hash) do pending_options = @transaction_necessity_by_association - |> Keyword.merge(paging_options(%{})) + |> Keyword.merge(paging_options(params)) |> Keyword.merge(current_filter(params)) full_options = put_in(pending_options, [:necessity_by_association, :block], :required) diff --git a/apps/block_scout_web/lib/block_scout_web/templates/transaction/_tile.html.eex b/apps/block_scout_web/lib/block_scout_web/templates/transaction/_tile.html.eex index 2ae13afdb6..9d2d2c5a50 100644 --- a/apps/block_scout_web/lib/block_scout_web/templates/transaction/_tile.html.eex +++ b/apps/block_scout_web/lib/block_scout_web/templates/transaction/_tile.html.eex @@ -52,14 +52,17 @@ <%= if involves_token_transfers?(@transaction) do %>
- <% [first_token_transfer | remaining_token_transfers]= @transaction.token_transfers %> + <% [first_token_transfer | remaining_token_transfers] = @transaction.token_transfers %> + <%= render "_token_transfer.html", address: assigns[:current_address], token_transfer: first_token_transfer %> +
<%= for token_transfer <- remaining_token_transfers do %> <%= render "_token_transfer.html", address: assigns[:current_address], token_transfer: token_transfer %> <% end %>
+ <%= if Enum.any?(remaining_token_transfers) do %>
diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index 8cfb97bfcf..c3c08ef8be 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -219,38 +219,22 @@ defmodule Explorer.Chain do necessity_by_association = Keyword.get(options, :necessity_by_association, %{}) paging_options = Keyword.get(options, :paging_options, @default_paging_options) - transaction_matches = - direction - |> case do - :from -> [:from_address_hash] - :to -> [:to_address_hash, :created_contract_address_hash] - _ -> [:from_address_hash, :to_address_hash, :created_contract_address_hash] - end - |> Enum.map(fn address_field -> - paging_options - |> fetch_transactions() - |> Transaction.where_address_fields_match(address_hash, address_field) - |> join_associations(necessity_by_association) - |> Transaction.preload_token_transfers(address_hash) - |> Repo.all() - |> MapSet.new() - end) - - token_transfer_matches = + {:ok, address_bytes} = Explorer.Chain.Hash.Address.dump(address_hash) + + token_transfers_dynamic = TokenTransfer.dynamic_any_address_fields_match(direction, address_bytes) + + transaction_dynamic = + Transaction.dynamic_where_address_hash_matches(address_hash, direction, token_transfers_dynamic) + + base_query = paging_options |> fetch_transactions() - |> TokenTransfer.where_address_fields_match(address_hash, direction) |> join_associations(necessity_by_association) |> Transaction.preload_token_transfers(address_hash) - |> Repo.all() - |> MapSet.new() - transaction_matches - |> Enum.reduce(token_transfer_matches, &MapSet.union/2) - |> MapSet.to_list() - |> Enum.sort_by(& &1.index, &>=/2) - |> Enum.sort_by(& &1.block_number, &>=/2) - |> Enum.slice(0..paging_options.page_size) + base_query + |> from(where: ^transaction_dynamic) + |> Repo.all() end @doc """ diff --git a/apps/explorer/lib/explorer/chain/token_transfer.ex b/apps/explorer/lib/explorer/chain/token_transfer.ex index 43dba819f6..ccebaa2ee6 100644 --- a/apps/explorer/lib/explorer/chain/token_transfer.ex +++ b/apps/explorer/lib/explorer/chain/token_transfer.ex @@ -151,22 +151,57 @@ defmodule Explorer.Chain.TokenTransfer do ) end - def where_address_fields_match(query, address_hash, :from) do - query - |> join(:left, [transaction], tt in assoc(transaction, :token_transfers)) - |> where([_transaction, tt], tt.from_address_hash == ^address_hash) + @doc """ + Builds a dynamic query expression to identify if there is a token transfer + related to the hash. + """ + def dynamic_any_address_fields_match(:to, address_bytes) do + dynamic( + [t], + t.hash == + fragment( + ~s""" + (SELECT tt.transaction_hash + FROM "token_transfers" AS tt + WHERE (tt."to_address_hash" = ?) + LIMIT 1) + """, + ^address_bytes + ) + ) end - def where_address_fields_match(query, address_hash, :to) do - query - |> join(:left, [transaction], tt in assoc(transaction, :token_transfers)) - |> where([_transaction, tt], tt.to_address_hash == ^address_hash) + def dynamic_any_address_fields_match(:from, address_bytes) do + dynamic( + [t], + t.hash == + fragment( + ~s""" + (SELECT tt.transaction_hash + FROM "token_transfers" AS tt + WHERE (tt."from_address_hash" = ?) + LIMIT 1) + """, + ^address_bytes + ) + ) end - def where_address_fields_match(query, address_hash, _) do - query - |> join(:left, [transaction], tt in assoc(transaction, :token_transfers)) - |> where([_transaction, tt], tt.to_address_hash == ^address_hash or tt.from_address_hash == ^address_hash) + def dynamic_any_address_fields_match(_, address_bytes) do + dynamic( + [t], + t.hash == + fragment( + ~s""" + (SELECT tt.transaction_hash + FROM "token_transfers" AS tt + WHERE ((tt."to_address_hash" = ?) OR (tt."from_address_hash" = ?)) + LIMIT 1) + """, + ^address_bytes, + ^address_bytes + ) + ) end @doc """ diff --git a/apps/explorer/lib/explorer/chain/transaction.ex b/apps/explorer/lib/explorer/chain/transaction.ex index fa69b98fe2..fecf72722c 100644 --- a/apps/explorer/lib/explorer/chain/transaction.ex +++ b/apps/explorer/lib/explorer/chain/transaction.ex @@ -3,7 +3,7 @@ defmodule Explorer.Chain.Transaction do use Explorer.Schema - import Ecto.Query, only: [from: 2, preload: 3, where: 3, subquery: 1] + import Ecto.Query, only: [dynamic: 2, from: 2, preload: 3, subquery: 1, where: 3] alias Ecto.Changeset @@ -427,6 +427,32 @@ defmodule Explorer.Chain.Transaction do where(query, [t], field(t, ^address_field) == ^address_hash) end + @doc """ + Builds a dynamic query expression to identify if there is a transaction + related to the hash. + """ + def dynamic_where_address_hash_matches(address_hash, :to, dynamic) do + dynamic( + [t], + t.to_address_hash == ^address_hash or t.created_contract_address_hash == ^address_hash or ^dynamic + ) + end + + def dynamic_where_address_hash_matches(address_hash, :from, dynamic) do + dynamic( + [t], + t.from_address_hash == ^address_hash or ^dynamic + ) + end + + def dynamic_where_address_hash_matches(address_hash, _, dynamic) do + dynamic( + [t], + t.to_address_hash == ^address_hash or t.from_address_hash == ^address_hash or + t.created_contract_address_hash == ^address_hash or ^dynamic + ) + end + @collated_fields ~w(block_number cumulative_gas_used gas_used index)a @collated_message "can't be blank when the transaction is collated into a block" diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 0ed17acb4e..47d950f143 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -208,15 +208,14 @@ defmodule Explorer.ChainTest do transaction = :transaction - |> insert() + |> insert(to_address: address, to_address_hash: address.hash) |> with_block() - insert(:token_transfer, to_address: address, transaction: transaction) - - transaction = - Transaction - |> Repo.get!(transaction.hash) - |> Repo.preload([:block, :to_address, :from_address, token_transfers: :token]) + insert( + :token_transfer, + to_address: address, + transaction: transaction + ) assert [transaction.hash] == Chain.address_to_transactions(address) @@ -228,33 +227,75 @@ defmodule Explorer.ChainTest do transaction = :transaction - |> insert() + |> insert(to_address: address) |> with_block() - token_transfer = insert(:token_transfer, to_address: address, transaction: transaction) - insert(:token_transfer, to_address: build(:address), transaction: transaction) + token_transfer = + insert( + :token_transfer, + to_address: address, + transaction: transaction + ) + + insert( + :token_transfer, + to_address: build(:address), + transaction: transaction + ) + + transaction = + address + |> Chain.address_to_transactions() + |> List.first() - transaction = Chain.address_to_transactions(address) |> List.first() + token_transfers_related = + Enum.map( + transaction.token_transfers, + &{&1.transaction_hash, &1.log_index} + ) - assert transaction.token_transfers |> Enum.map(&{&1.transaction_hash, &1.log_index}) == [ + assert token_transfers_related == [ {token_transfer.transaction_hash, token_transfer.log_index} ] end test "returns just the token transfers related to the given contract address" do - contract_address = insert(:address, contract_code: Factory.data("contract_code")) + contract_address = + insert( + :address, + contract_code: Factory.data("contract_code") + ) transaction = :transaction - |> insert() + |> insert(to_address: contract_address) |> with_block() - token_transfer = insert(:token_transfer, to_address: contract_address, transaction: transaction) - insert(:token_transfer, to_address: build(:address), transaction: transaction) + token_transfer = + insert( + :token_transfer, + to_address: contract_address, + transaction: transaction + ) - transaction = Chain.address_to_transactions(contract_address) |> List.first() + insert( + :token_transfer, + to_address: build(:address), + transaction: transaction + ) + + transaction = + contract_address + |> Chain.address_to_transactions() + |> List.first() + + token_transfers_contract_address = + Enum.map( + transaction.token_transfers, + &{&1.transaction_hash, &1.log_index} + ) - assert Enum.map(transaction.token_transfers, &{&1.transaction_hash, &1.log_index}) == [ + assert token_transfers_contract_address == [ {token_transfer.transaction_hash, token_transfer.log_index} ] end @@ -289,7 +330,7 @@ defmodule Explorer.ChainTest do address = insert(:address) second_page_hashes = - 50 + 2 |> insert_list(:transaction, from_address: address) |> with_block() |> Enum.map(& &1.hash) @@ -302,7 +343,10 @@ defmodule Explorer.ChainTest do assert second_page_hashes == address |> Chain.address_to_transactions( - paging_options: %PagingOptions{key: {block_number, index}, page_size: 50} + paging_options: %PagingOptions{ + key: {block_number, index}, + page_size: 2 + } ) |> Enum.map(& &1.hash) |> Enum.reverse() From ea6ad8e5a7c2625f387405540e56f07070f03919 Mon Sep 17 00:00:00 2001 From: Amanda Sposito Date: Wed, 31 Oct 2018 15:34:48 -0300 Subject: [PATCH 2/2] Add gettext --- apps/block_scout_web/priv/gettext/default.pot | 4 ++-- apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/block_scout_web/priv/gettext/default.pot b/apps/block_scout_web/priv/gettext/default.pot index eba53626ff..e3b939482d 100644 --- a/apps/block_scout_web/priv/gettext/default.pot +++ b/apps/block_scout_web/priv/gettext/default.pot @@ -1080,12 +1080,12 @@ msgid "View Contract" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/transaction/_tile.html.eex:67 +#: lib/block_scout_web/templates/transaction/_tile.html.eex:70 msgid "View Less Transfers" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/transaction/_tile.html.eex:66 +#: lib/block_scout_web/templates/transaction/_tile.html.eex:69 msgid "View More Transfers" msgstr "" diff --git a/apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po b/apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po index 8aac131ef9..d96c8a2539 100644 --- a/apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po +++ b/apps/block_scout_web/priv/gettext/en/LC_MESSAGES/default.po @@ -1080,12 +1080,12 @@ msgid "View Contract" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/transaction/_tile.html.eex:67 +#: lib/block_scout_web/templates/transaction/_tile.html.eex:70 msgid "View Less Transfers" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/transaction/_tile.html.eex:66 +#: lib/block_scout_web/templates/transaction/_tile.html.eex:69 msgid "View More Transfers" msgstr ""