From 7ca61387627971d6c731745b33e7505cee54ac9f Mon Sep 17 00:00:00 2001 From: Tim Mecklem Date: Mon, 30 Apr 2018 12:58:21 -0400 Subject: [PATCH] Exclude internal transactions with no siblings inside transaction Add pagination Co-authored-by: jkahne Co-authored-by: faultyserver --- apps/explorer/lib/explorer/chain.ex | 23 +-- apps/explorer/test/explorer/chain_test.exs | 115 ++++++------ .../index.html.eex | 174 ++++++++++-------- .../address_transaction/index.html.eex | 2 +- 4 files changed, 156 insertions(+), 158 deletions(-) diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index b5905de121..2153abaecf 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -51,9 +51,8 @@ defmodule Explorer.Chain do @doc """ `t:Explorer.Chain.InternalTransaction/0`s from `address`. - This function excludes any "representative" internal transactions, where the internal transaction is a mirror of the - parent transaction's value, to and from addresses. In the case of multiple internal transactions that have the same - value, to, and from address, it excludes the internal transaction with the lowest id. + This function excludes any internal transactions in the results where the internal transaction has no siblings within + the parent transaction. ## Options @@ -70,31 +69,23 @@ defmodule Explorer.Chain do def address_to_internal_transactions(%Address{id: id}, options \\ []) do necessity_by_association = Keyword.get(options, :necessity_by_association, %{}) direction = Keyword.get(options, :direction) + pagination = Keyword.get(options, :pagination, %{}) InternalTransaction |> join(:inner, [internal_transaction], transaction in assoc(internal_transaction, :transaction)) |> join(:left, [internal_transaction, transaction], block in assoc(transaction, :block)) |> where_address_fields_match(direction, id) |> where( - [it], + [_it, t], fragment( - """ - ? NOT IN ( - SELECT min(it.id) FROM internal_transactions AS it - INNER JOIN transactions AS t ON t.id = it.transaction_id - WHERE it.value = t.value - AND it.from_address_id = t.from_address_id - AND it.to_address_id = t.to_address_id - GROUP BY t.id - ) - """, - it.id + "(SELECT COUNT(sibling.id) FROM internal_transactions as sibling WHERE sibling.transaction_id = ?) > 1", + t.id ) ) |> order_by([it, transaction, block], desc: block.number, desc: transaction.transaction_index, desc: it.index) |> preload(transaction: :block) |> join_associations(necessity_by_association) - |> Repo.all() + |> Repo.paginate(pagination) end @doc """ diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 3048380501..5c922a4c7b 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -589,21 +589,26 @@ defmodule Explorer.ChainTest do end describe "address_to_internal_transactions/1" do - test "with single transaction containing an internal transaction" do + test "with single transaction containing two internal transactions" do address = insert(:address) transaction = insert(:transaction) - %InternalTransaction{id: expected_id} = + %InternalTransaction{id: first_id} = + insert(:internal_transaction, transaction_id: transaction.id, to_address_id: address.id) + + %InternalTransaction{id: second_id} = insert(:internal_transaction, transaction_id: transaction.id, to_address_id: address.id) - result = address |> Chain.address_to_internal_transactions() |> List.first() - assert result.id == expected_id + result = address |> Chain.address_to_internal_transactions() |> Enum.map(fn it -> it.id end) + assert Enum.member?(result, first_id) + assert Enum.member?(result, second_id) end test "loads associations in necessity_by_association" do address = insert(:address) transaction = insert(:transaction, to_address_id: address.id) - insert(:internal_transaction, transaction_id: transaction.id, to_address_id: address.id) + insert(:internal_transaction, transaction_id: transaction.id, to_address_id: address.id, index: 0) + insert(:internal_transaction, transaction_id: transaction.id, to_address_id: address.id, index: 1) assert [ %InternalTransaction{ @@ -611,7 +616,8 @@ defmodule Explorer.ChainTest do to_address: %Ecto.Association.NotLoaded{}, transaction: %Transaction{} } - ] = Chain.address_to_internal_transactions(address) + | _ + ] = Map.get(Chain.address_to_internal_transactions(address), :entries, []) assert [ %InternalTransaction{ @@ -619,14 +625,19 @@ defmodule Explorer.ChainTest do to_address: %Address{}, transaction: %Transaction{} } + | _ ] = - Chain.address_to_internal_transactions( - address, - necessity_by_association: %{ - from_address: :optional, - to_address: :optional, - transaction: :optional - } + Map.get( + Chain.address_to_internal_transactions( + address, + necessity_by_association: %{ + from_address: :optional, + to_address: :optional, + transaction: :optional + } + ), + :entries, + [] ) end @@ -635,71 +646,53 @@ defmodule Explorer.ChainTest do pending_transaction = :transaction |> insert(transaction_index: "3") - first_block = insert(:block, number: 2000) - first_transaction = :transaction |> insert(transaction_index: "10") |> with_block(first_block) - second_transaction = :transaction |> insert(transaction_index: "20") |> with_block(first_block) + %InternalTransaction{id: first_pending} = + insert(:internal_transaction, transaction: pending_transaction, to_address_id: address.id, index: 0) - second_block = insert(:block, number: 4000) - third_transaction = :transaction |> insert(transaction_index: "5") |> with_block(second_block) + %InternalTransaction{id: second_pending} = + insert(:internal_transaction, transaction: pending_transaction, to_address_id: address.id, index: 1) - %InternalTransaction{id: pending_id} = - insert(:internal_transaction, transaction: pending_transaction, to_address_id: address.id, index: 0) + a_block = insert(:block, number: 2000) + first_a_transaction = :transaction |> insert(transaction_index: "10") |> with_block(a_block) - %InternalTransaction{id: first_id} = - insert(:internal_transaction, transaction: first_transaction, to_address_id: address.id, index: 0) + %InternalTransaction{id: first} = + insert(:internal_transaction, transaction: first_a_transaction, to_address_id: address.id, index: 0) - %InternalTransaction{id: second_id} = - insert(:internal_transaction, transaction: second_transaction, to_address_id: address.id, index: 0) + %InternalTransaction{id: second} = + insert(:internal_transaction, transaction: first_a_transaction, to_address_id: address.id, index: 1) + + second_a_transaction = :transaction |> insert(transaction_index: "20") |> with_block(a_block) + + %InternalTransaction{id: third} = + insert(:internal_transaction, transaction: second_a_transaction, to_address_id: address.id, index: 0) - %InternalTransaction{id: third_id} = - insert(:internal_transaction, transaction: third_transaction, to_address_id: address.id, index: 0) + %InternalTransaction{id: fourth} = + insert(:internal_transaction, transaction: second_a_transaction, to_address_id: address.id, index: 1) - %InternalTransaction{id: fourth_id} = - insert(:internal_transaction, transaction: third_transaction, to_address_id: address.id, index: 1) + b_block = insert(:block, number: 6000) + first_b_transaction = :transaction |> insert(transaction_index: "20") |> with_block(b_block) + + %InternalTransaction{id: fifth} = + insert(:internal_transaction, transaction: first_b_transaction, to_address_id: address.id, index: 0) + + %InternalTransaction{id: sixth} = + insert(:internal_transaction, transaction: first_b_transaction, to_address_id: address.id, index: 1) result = address |> Chain.address_to_internal_transactions() + |> Map.get(:entries, []) |> Enum.map(fn internal_transaction -> internal_transaction.id end) - assert [pending_id, fourth_id, third_id, second_id, first_id] == result + assert [second_pending, first_pending, sixth, fifth, fourth, third, second, first] == result end - test "Filters out internal transaction (with lowest id if multiple) that represents the parent transaction" do + test "Excludes internal transactions where they are alone in the parent transaction" do address = insert(:address) transaction = :transaction |> insert(to_address_id: address.id) |> with_block() + insert(:internal_transaction, transaction: transaction, to_address_id: address.id) - %InternalTransaction{id: first_id} = - insert(:internal_transaction, transaction: transaction, to_address_id: address.id, index: 0) - - %InternalTransaction{id: excluded_id} = - insert( - :internal_transaction, - transaction: transaction, - to_address_id: address.id, - index: 1, - value: transaction.value, - from_address_id: transaction.from_address_id - ) - - %InternalTransaction{id: third_id} = - insert( - :internal_transaction, - transaction: transaction, - to_address_id: address.id, - index: 2, - value: transaction.value, - from_address_id: transaction.from_address_id - ) - - result = - address - |> Chain.address_to_internal_transactions() - |> Enum.map(fn internal_transaction -> internal_transaction.id end) - - assert Enum.member?(result, first_id) - refute Enum.member?(result, excluded_id) - assert Enum.member?(result, third_id) + assert %{entries: []} = Chain.address_to_internal_transactions(address) end end diff --git a/apps/explorer_web/lib/explorer_web/templates/address_internal_transaction/index.html.eex b/apps/explorer_web/lib/explorer_web/templates/address_internal_transaction/index.html.eex index 5948f5e763..d71737e482 100644 --- a/apps/explorer_web/lib/explorer_web/templates/address_internal_transaction/index.html.eex +++ b/apps/explorer_web/lib/explorer_web/templates/address_internal_transaction/index.html.eex @@ -18,87 +18,101 @@ ) %> - -
- -
- <%= link( - gettext("All"), - to: address_internal_transaction_path(@conn, :index, @conn.assigns.locale, @conn.params["address_id"]), - class: "address__link address__link--active dropdown-item", - "data-test": "filter_option" - ) %> - <%= link( - gettext("To"), - to: address_internal_transaction_path( - @conn, - :index, - @conn.assigns.locale, - @conn.params["address_id"], - filter: "to" - ), - class: "address__link address__link--active dropdown-item", - "data-test": "filter_option" - ) %> - <%= link( - gettext("From"), - to: address_internal_transaction_path( - @conn, - :index, - @conn.assigns.locale, - @conn.params["address_id"], - filter: "from" - ), - class: "address__link address__link--active dropdown-item", - "data-test": "filter_option" - ) %> +
+ +
+ <%= link( + gettext("All"), + to: address_internal_transaction_path(@conn, :index, @conn.assigns.locale, @conn.params["address_id"]), + class: "address__link address__link--active dropdown-item", + "data-test": "filter_option" + ) %> + <%= link( + gettext("To"), + to: address_internal_transaction_path( + @conn, + :index, + @conn.assigns.locale, + @conn.params["address_id"], + filter: "to" + ), + class: "address__link address__link--active dropdown-item", + "data-test": "filter_option" + ) %> + <%= link( + gettext("From"), + to: address_internal_transaction_path( + @conn, + :index, + @conn.assigns.locale, + @conn.params["address_id"], + filter: "from" + ), + class: "address__link address__link--active dropdown-item", + "data-test": "filter_option" + ) %> +
+
+
+ <%= if length(@page.entries) > 0 do %> + + + + + + + + + + <%= for internal_transaction <- @page do %> + + + + + + + + + + + <% end %> +
<%= gettext "Parent Tx Hash" %><%= gettext "Block" %><%= gettext "Age" %><%= gettext "From" %><%= gettext "To" %><%= gettext "Value" %>
+ <%= link(internal_transaction.transaction.hash, + to: transaction_path(@conn, :show, @conn.assigns.locale, internal_transaction.transaction.hash), + class: "transaction-log__link") %> + + <%= link(internal_transaction.transaction.block.number, + to: block_path(@conn, :show, @conn.assigns.locale, internal_transaction.transaction.block.number), + class: "transaction-log__link") %> + <%= ExplorerWeb.BlockView.age(internal_transaction.transaction.block) %> + <%= link(internal_transaction.from_address.hash, + to: address_path(@conn, :show, @conn.assigns.locale, internal_transaction.from_address.hash), + class: "transaction-log__link") %> + + <%= link(internal_transaction.to_address.hash, + to: address_path(@conn, :show, @conn.assigns.locale, internal_transaction.to_address.hash), + class: "transaction-log__link") %> + <%= ExplorerWeb.TransactionView.value(internal_transaction) %>
+ <% else %> +

<%= gettext "There are no Internal Transactions" %>

+ <% end %>
-
- <%= if length(@page) > 0 do %> - - - - - - - - - - <%= for internal_transaction <- @page do %> - - - - - - - - - - - <% end %> -
<%= gettext "Parent Tx Hash" %><%= gettext "Block" %><%= gettext "Age" %><%= gettext "From" %><%= gettext "To" %><%= gettext "Value" %>
- <%= link(internal_transaction.transaction.hash, - to: transaction_path(@conn, :show, @conn.assigns.locale, internal_transaction.transaction.hash), - class: "transaction-log__link") %> - - <%= link(internal_transaction.transaction.block.number, - to: block_path(@conn, :show, @conn.assigns.locale, internal_transaction.transaction.block.number), - class: "transaction-log__link") %> - <%= ExplorerWeb.BlockView.age(internal_transaction.transaction.block) %> - <%= link(internal_transaction.from_address.hash, - to: address_path(@conn, :show, @conn.assigns.locale, internal_transaction.from_address.hash), - class: "transaction-log__link") %> - - <%= link(internal_transaction.to_address.hash, - to: address_path(@conn, :show, @conn.assigns.locale, internal_transaction.to_address.hash), - class: "transaction-log__link") %> - <%= ExplorerWeb.TransactionView.value(internal_transaction) %>
- <% else %> -

<%= gettext "There are no Internal Transactions" %>

- <% end %> +
+ <%= pagination_links( + @conn, + @page, + ["en", @conn.params["address_id"]], + distance: 1, + filter: @conn.params["filter"], + first: true, + next: Phoenix.HTML.raw("›"), + path: &address_internal_transaction_path/5, + previous: Phoenix.HTML.raw("‹"), + view_style: :bulma + ) %>
diff --git a/apps/explorer_web/lib/explorer_web/templates/address_transaction/index.html.eex b/apps/explorer_web/lib/explorer_web/templates/address_transaction/index.html.eex index 01a184d6eb..aa65c724b8 100644 --- a/apps/explorer_web/lib/explorer_web/templates/address_transaction/index.html.eex +++ b/apps/explorer_web/lib/explorer_web/templates/address_transaction/index.html.eex @@ -20,7 +20,7 @@
-