From 7f56c76ff93fdbfa8953c2ed55e92305a624bf42 Mon Sep 17 00:00:00 2001 From: Felipe Renan Date: Mon, 15 Oct 2018 18:23:15 -0300 Subject: [PATCH 1/3] Refactor transactions count by address Now, the app is going to count how many transactions an specific address has sent instead of an estimated count. The reasons why we are doing that are: - we have a inaccurate count (bad user experience) - it's slower As we already store the nonce when the app index transactions, we can query this value by address to know how many transactions the address has sent. --- .../controllers/address_controller.ex | 2 +- .../templates/address/index.html.eex | 2 +- .../templates/address/overview.html.eex | 3 +- .../lib/block_scout_web/views/address_view.ex | 2 +- apps/explorer/lib/explorer/chain.ex | 41 +++++++------------ .../lib/explorer/chain/transaction.ex | 17 ++++++++ .../test/explorer/chain/transaction_test.exs | 36 ++++++++++++++++ apps/explorer/test/explorer/chain_test.exs | 22 ++++++++-- 8 files changed, 91 insertions(+), 34 deletions(-) diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/address_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/address_controller.ex index 93ea2a688e..3c29ffa4f3 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/address_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/address_controller.ex @@ -18,7 +18,7 @@ defmodule BlockScoutWeb.AddressController do end def transaction_count(%Address{} = address) do - Chain.address_to_transactions_estimated_count(address) + Chain.total_transactions_sent_by_address(address) end def validation_count(%Address{} = address) do diff --git a/apps/block_scout_web/lib/block_scout_web/templates/address/index.html.eex b/apps/block_scout_web/lib/block_scout_web/templates/address/index.html.eex index 8f9c5e7397..b7dc6c7405 100644 --- a/apps/block_scout_web/lib/block_scout_web/templates/address/index.html.eex +++ b/apps/block_scout_web/lib/block_scout_web/templates/address/index.html.eex @@ -25,7 +25,7 @@ <%= transaction_count(address) %> - <%= gettext "Transactions" %> + <%= gettext "Transactions sent" %> diff --git a/apps/block_scout_web/lib/block_scout_web/templates/address/overview.html.eex b/apps/block_scout_web/lib/block_scout_web/templates/address/overview.html.eex index 01bfa54b55..ac9b0f1b1a 100644 --- a/apps/block_scout_web/lib/block_scout_web/templates/address/overview.html.eex +++ b/apps/block_scout_web/lib/block_scout_web/templates/address/overview.html.eex @@ -24,7 +24,8 @@ <%= Cldr.Number.to_string!(@transaction_count, format: "#,###") %> - <%= gettext("Transactions") %> + + <%= gettext("Transactions sent") %> <%= if validator?(@validation_count) do %> <%= Cldr.Number.to_string!(@validation_count, format: "#,###") %> diff --git a/apps/block_scout_web/lib/block_scout_web/views/address_view.ex b/apps/block_scout_web/lib/block_scout_web/views/address_view.ex index 4d15e2de85..2d241b01a2 100644 --- a/apps/block_scout_web/lib/block_scout_web/views/address_view.ex +++ b/apps/block_scout_web/lib/block_scout_web/views/address_view.ex @@ -160,7 +160,7 @@ defmodule BlockScoutWeb.AddressView do def token_title(%Token{name: name, symbol: symbol}), do: "#{name} (#{symbol})" def transaction_count(%Address{} = address) do - Chain.address_to_transactions_estimated_count(address) + Chain.total_transactions_sent_by_address(address) end def trimmed_hash(%Hash{} = hash) do diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index afb8e68b71..43c9df5a7f 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -180,35 +180,22 @@ defmodule Explorer.Chain do end @doc """ - Gets an estimated count of `t:Explorer.Chain.Transaction.t/0` to or from the `address` based on the estimated rows - resulting in an EXPLAIN of the query plan for the count query. + Get the total number of transactions sent by the given address according to the last block indexed. + + We have to increment +1 in the last nonce result because it works like an array position, the first + nonce has the value 0. When last nonce is nil, it considers that the given address has 0 transactions. """ - @spec address_to_transactions_estimated_count(Address.t()) :: non_neg_integer() - def address_to_transactions_estimated_count(%Address{hash: address_hash}) do - {:ok, %Postgrex.Result{rows: result}} = - Repo.query( - """ - EXPLAIN SELECT COUNT(DISTINCT t.hash) FROM - ( - SELECT t0.hash FROM transactions AS t0 WHERE t0.from_address_hash = $1 - UNION - SELECT t0.hash FROM transactions AS t0 WHERE t0.to_address_hash = $1 - UNION - SELECT t0.hash FROM transactions AS t0 WHERE t0.created_contract_address_hash = $1 - UNION - SELECT tt.transaction_hash AS hash FROM token_transfers AS tt - WHERE tt.from_address_hash = $1 - UNION - SELECT tt.transaction_hash AS hash FROM token_transfers AS tt - WHERE tt.to_address_hash = $1 - ) as t - """, - [address_hash.bytes] - ) + @spec total_transactions_sent_by_address(Address.t()) :: non_neg_integer() + def total_transactions_sent_by_address(%Address{hash: address_hash}) do + last_nonce = + address_hash + |> Transaction.last_nonce_by_address_query() + |> Repo.one() - {[unique_explain], _} = List.pop_at(result, 1) - [[_ | [rows]]] = Regex.scan(~r/rows=(\d+)/, unique_explain) - String.to_integer(rows) + case last_nonce do + nil -> 0 + value -> value + 1 + end end @doc """ diff --git a/apps/explorer/lib/explorer/chain/transaction.ex b/apps/explorer/lib/explorer/chain/transaction.ex index 1e814e43ed..fa69b98fe2 100644 --- a/apps/explorer/lib/explorer/chain/transaction.ex +++ b/apps/explorer/lib/explorer/chain/transaction.ex @@ -543,4 +543,21 @@ defmodule Explorer.Chain.Transaction do distinct: :hash ) end + + @doc """ + Builds an `Ecto.Query` to fetch the last nonce from the given address hash. + + The last nonce value means the total of transactions that the given address has sent through the + chain. Also, the query uses the last `block_number` to get the last nonce because this column is + indexed in DB, then the query is faster than ordering by last nonce. + """ + def last_nonce_by_address_query(address_hash) do + from( + t in Transaction, + select: t.nonce, + where: t.from_address_hash == ^address_hash, + order_by: [desc: :block_number], + limit: 1 + ) + end end diff --git a/apps/explorer/test/explorer/chain/transaction_test.exs b/apps/explorer/test/explorer/chain/transaction_test.exs index ed246332ca..7855daa2a8 100644 --- a/apps/explorer/test/explorer/chain/transaction_test.exs +++ b/apps/explorer/test/explorer/chain/transaction_test.exs @@ -177,4 +177,40 @@ defmodule Explorer.Chain.TransactionTest do assert result == [transaction_c.block_number, transaction_b.block_number, transaction_a.block_number] end end + + describe "last_nonce_by_address_query/1" do + test "returns the nonce value from the last block" do + address = insert(:address) + + :transaction + |> insert(nonce: 100, from_address: address) + |> with_block(insert(:block, number: 1000)) + + :transaction + |> insert(nonce: 300, from_address: address) + |> with_block(insert(:block, number: 2000)) + + last_nonce = + address.hash + |> Transaction.last_nonce_by_address_query() + |> Repo.one() + + assert last_nonce == 300 + end + + test "considers only from_address in transactions" do + address = insert(:address) + + :transaction + |> insert(nonce: 100, to_address: address) + |> with_block(insert(:block, number: 1000)) + + last_nonce = + address.hash + |> Transaction.last_nonce_by_address_query() + |> Repo.one() + + assert last_nonce == nil + end + end end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 2faa65939f..66cab8d6ac 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -344,9 +344,25 @@ defmodule Explorer.ChainTest do end end - describe "address_to_transactions_estimated_count/1" do - test "returns integer" do - assert is_integer(Chain.address_to_transactions_estimated_count(build(:address))) + describe "total_transactions_sent_by_address/1" do + test "increments +1 in the last nonce result" do + address = insert(:address) + + :transaction + |> insert(nonce: 100, from_address: address) + |> with_block(insert(:block, number: 1000)) + + assert Chain.total_transactions_sent_by_address(address) == 101 + end + + test "returns 0 when the address did not send transactions" do + address = insert(:address) + + :transaction + |> insert(nonce: 100, to_address: address) + |> with_block(insert(:block, number: 1000)) + + assert Chain.total_transactions_sent_by_address(address) == 0 end end From fa5c92e8d6d2a99908ac81af75f34792830a2896 Mon Sep 17 00:00:00 2001 From: Felipe Renan Date: Tue, 16 Oct 2018 09:54:39 -0300 Subject: [PATCH 2/3] Generate gettext --- apps/block_scout_web/priv/gettext/default.pot | 20 +++++++++++-------- .../priv/gettext/en/LC_MESSAGES/default.po | 20 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/apps/block_scout_web/priv/gettext/default.pot b/apps/block_scout_web/priv/gettext/default.pot index 11cb19b052..77be08843c 100644 --- a/apps/block_scout_web/priv/gettext/default.pot +++ b/apps/block_scout_web/priv/gettext/default.pot @@ -124,7 +124,7 @@ msgstr "" #, elixir-format #: lib/block_scout_web/templates/address/_tabs.html.eex:31 #: lib/block_scout_web/templates/address/_tabs.html.eex:88 -#: lib/block_scout_web/templates/address/overview.html.eex:31 +#: lib/block_scout_web/templates/address/overview.html.eex:32 #: lib/block_scout_web/templates/address_validation/index.html.eex:32 #: lib/block_scout_web/templates/address_validation/index.html.eex:81 #: lib/block_scout_web/templates/address_validation/index.html.eex:108 @@ -143,8 +143,8 @@ msgid "Clear" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/address/overview.html.eex:73 -#: lib/block_scout_web/templates/address/overview.html.eex:81 +#: lib/block_scout_web/templates/address/overview.html.eex:74 +#: lib/block_scout_web/templates/address/overview.html.eex:82 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:81 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:89 msgid "Close" @@ -272,7 +272,7 @@ msgid "Copyright %{year} POA" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/address/overview.html.eex:42 +#: lib/block_scout_web/templates/address/overview.html.eex:43 msgid "Created by" msgstr "" @@ -621,7 +621,7 @@ msgstr "" #, elixir-format #: lib/block_scout_web/templates/address/overview.html.eex:13 -#: lib/block_scout_web/templates/address/overview.html.eex:72 +#: lib/block_scout_web/templates/address/overview.html.eex:73 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:13 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:13 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:80 @@ -876,8 +876,6 @@ msgstr "" #, elixir-format #: lib/block_scout_web/templates/address/_tabs.html.eex:5 #: lib/block_scout_web/templates/address/_tabs.html.eex:71 -#: lib/block_scout_web/templates/address/index.html.eex:28 -#: lib/block_scout_web/templates/address/overview.html.eex:27 #: lib/block_scout_web/templates/address_transaction/index.html.eex:63 #: lib/block_scout_web/templates/address_validation/index.html.eex:11 #: lib/block_scout_web/templates/address_validation/index.html.eex:65 @@ -993,7 +991,7 @@ msgid "Yes" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/address/overview.html.eex:48 +#: lib/block_scout_web/templates/address/overview.html.eex:49 msgid "at" msgstr "" @@ -1176,3 +1174,9 @@ msgstr "" #: lib/block_scout_web/templates/layout/app.html.eex:47 msgid "Indexing Tokens" msgstr "" + +#, elixir-format +#: lib/block_scout_web/templates/address/index.html.eex:28 +#: lib/block_scout_web/templates/address/overview.html.eex:28 +msgid "Transactions sent" +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 3991ec5cbc..266b3dabbd 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 @@ -124,7 +124,7 @@ msgstr "" #, elixir-format #: lib/block_scout_web/templates/address/_tabs.html.eex:31 #: lib/block_scout_web/templates/address/_tabs.html.eex:88 -#: lib/block_scout_web/templates/address/overview.html.eex:31 +#: lib/block_scout_web/templates/address/overview.html.eex:32 #: lib/block_scout_web/templates/address_validation/index.html.eex:32 #: lib/block_scout_web/templates/address_validation/index.html.eex:81 #: lib/block_scout_web/templates/address_validation/index.html.eex:108 @@ -143,8 +143,8 @@ msgid "Clear" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/address/overview.html.eex:73 -#: lib/block_scout_web/templates/address/overview.html.eex:81 +#: lib/block_scout_web/templates/address/overview.html.eex:74 +#: lib/block_scout_web/templates/address/overview.html.eex:82 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:81 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:89 msgid "Close" @@ -272,7 +272,7 @@ msgid "Copyright %{year} POA" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/address/overview.html.eex:42 +#: lib/block_scout_web/templates/address/overview.html.eex:43 msgid "Created by" msgstr "" @@ -621,7 +621,7 @@ msgstr "" #, elixir-format #: lib/block_scout_web/templates/address/overview.html.eex:13 -#: lib/block_scout_web/templates/address/overview.html.eex:72 +#: lib/block_scout_web/templates/address/overview.html.eex:73 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:13 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:13 #: lib/block_scout_web/templates/tokens/overview/_details.html.eex:80 @@ -876,8 +876,6 @@ msgstr "" #, elixir-format #: lib/block_scout_web/templates/address/_tabs.html.eex:5 #: lib/block_scout_web/templates/address/_tabs.html.eex:71 -#: lib/block_scout_web/templates/address/index.html.eex:28 -#: lib/block_scout_web/templates/address/overview.html.eex:27 #: lib/block_scout_web/templates/address_transaction/index.html.eex:63 #: lib/block_scout_web/templates/address_validation/index.html.eex:11 #: lib/block_scout_web/templates/address_validation/index.html.eex:65 @@ -993,7 +991,7 @@ msgid "Yes" msgstr "" #, elixir-format -#: lib/block_scout_web/templates/address/overview.html.eex:48 +#: lib/block_scout_web/templates/address/overview.html.eex:49 msgid "at" msgstr "" @@ -1176,3 +1174,9 @@ msgstr "" #: lib/block_scout_web/templates/layout/app.html.eex:47 msgid "Indexing Tokens" msgstr "" + +#, elixir-format +#: lib/block_scout_web/templates/address/index.html.eex:28 +#: lib/block_scout_web/templates/address/overview.html.eex:28 +msgid "Transactions sent" +msgstr "" From 54de3f985f79a9631a678c98032c4448bd483783 Mon Sep 17 00:00:00 2001 From: Felipe Renan Date: Tue, 16 Oct 2018 18:05:59 -0300 Subject: [PATCH 3/3] Fix the Address page to count only transactions sent --- .../assets/__tests__/pages/address.js | 38 +++++++++++++++---- .../assets/js/pages/address.js | 14 ++++++- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/apps/block_scout_web/assets/__tests__/pages/address.js b/apps/block_scout_web/assets/__tests__/pages/address.js index 426bb4fbe8..02f319caa7 100644 --- a/apps/block_scout_web/assets/__tests__/pages/address.js +++ b/apps/block_scout_web/assets/__tests__/pages/address.js @@ -265,11 +265,14 @@ describe('RECEIVED_NEW_PENDING_TRANSACTION_BATCH', () => { describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { test('single transaction', () => { - const state = initialState + const state = Object.assign({}, initialState, { + addressHash: '0x111' + }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', msgs: [{ - transactionHtml: 'test' + transactionHtml: 'test', + fromAddressHash: '0x111' }] } const output = reducer(state, action) @@ -310,7 +313,6 @@ describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { expect(output.newTransactions).toEqual([]) expect(output.batchCountAccumulator).toEqual(11) - expect(output.transactionCount).toEqual(11) }) test('single transaction after single transaction', () => { const state = Object.assign({}, initialState, { @@ -379,6 +381,28 @@ describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { expect(output.newTransactions).toEqual([]) expect(output.batchCountAccumulator).toEqual(22) }) + test('increments the transactions count only when the address sent transactions', () => { + const state = Object.assign({}, initialState, { + newTransactions: [], + addressHash: '0x111', + transactionCount: 1 + }) + const action = { + type: 'RECEIVED_NEW_TRANSACTION_BATCH', + msgs: [{ + transactionHtml: 'test 12', + fromAddressHash: '0x111', + toAddressHash: '0x222' + },{ + transactionHtml: 'test 13', + fromAddressHash: '0x222', + toAddressHash: '0x111' + }] + } + const output = reducer(state, action) + + expect(output.transactionCount).toEqual(2) + }) test('after disconnection', () => { const state = Object.assign({}, initialState, { channelDisconnected: true @@ -397,12 +421,14 @@ describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { test('on page 2', () => { const state = Object.assign({}, initialState, { beyondPageOne: true, - transactionCount: 1 + transactionCount: 1, + addressHash: '0x111' }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', msgs: [{ - transactionHtml: 'test' + transactionHtml: 'test', + fromAddressHash: '0x111' }] } const output = reducer(state, action) @@ -488,7 +514,6 @@ describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { expect(output.newTransactions).toEqual(['test']) expect(output.batchCountAccumulator).toEqual(0) - expect(output.transactionCount).toEqual(1) }) test('large batch of transactions', () => { const state = initialState @@ -532,6 +557,5 @@ describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { const output = reducer(state, action) expect(output.newTransactions).toEqual([]) - expect(output.transactionCount).toEqual(11) }) }) diff --git a/apps/block_scout_web/assets/js/pages/address.js b/apps/block_scout_web/assets/js/pages/address.js index 9a6bc7e5ac..d8e7beb849 100644 --- a/apps/block_scout_web/assets/js/pages/address.js +++ b/apps/block_scout_web/assets/js/pages/address.js @@ -11,6 +11,18 @@ import { loadTokenBalanceDropdown } from '../lib/token_balance_dropdown' const BATCH_THRESHOLD = 10 +const incrementTransactionsCount = (transactions, addressHash, currentValue) => { + const reducer = (accumulator, {fromAddressHash}) => { + if (fromAddressHash === addressHash) { + accumulator++ + } + + return accumulator + } + + return transactions.reduce(reducer, currentValue) +} + export const initialState = { addressHash: null, balance: null, @@ -109,7 +121,7 @@ export function reducer (state = initialState, action) { case 'RECEIVED_NEW_TRANSACTION_BATCH': { if (state.channelDisconnected) return state - const transactionCount = state.transactionCount + action.msgs.length + const transactionCount = incrementTransactionsCount(action.msgs, state.addressHash, state.transactionCount) if (state.beyondPageOne) return Object.assign({}, state, { transactionCount })