From ef92e47c6b9ee703cbfd98fa8d265b2eb5476ca8 Mon Sep 17 00:00:00 2001 From: Tim Mecklem Date: Mon, 4 Jun 2018 16:06:02 -0400 Subject: [PATCH 1/6] Remove unnecessary joins to blocks table Co-authored-by: Stamates --- apps/explorer/lib/explorer/chain.ex | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index f490dd73f0..b2edd2c412 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -1639,13 +1639,11 @@ defmodule Explorer.Chain do {:ok, hash} -> from( transaction in query, - inner_join: block in assoc(transaction, :block), join: hash_transaction in Transaction, on: hash_transaction.hash == ^hash, - inner_join: hash_block in assoc(hash_transaction, :block), where: - block.number > hash_block.number or - (block.number == hash_block.number and transaction.index > hash_transaction.index) + transaction.block_number > hash_transaction.block_number or + (transaction.block_number == hash_transaction.block_number and transaction.index > hash_transaction.index) ) :error -> From 563273b2b1f3f80bc5e46658dfcee61e073f7063 Mon Sep 17 00:00:00 2001 From: Tim Mecklem Date: Mon, 4 Jun 2018 22:59:55 -0400 Subject: [PATCH 2/6] Use block_number and index for stable transaction paging Fix paging to function correctly Add paging in other direction Add PagingOptions struct to manage stable paging --- apps/explorer/lib/explorer/chain.ex | 111 ++++++------------ .../lib/explorer/chain/transaction.ex | 7 +- apps/explorer/lib/explorer/paging_options.ex | 13 ++ apps/explorer/test/explorer/chain_test.exs | 11 ++ .../controllers/transaction_controller.ex | 25 ++-- .../templates/transaction/index.html.eex | 24 ++-- .../transaction_controller_test.exs | 45 +++---- 7 files changed, 114 insertions(+), 122 deletions(-) create mode 100644 apps/explorer/lib/explorer/paging_options.ex diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index b2edd2c412..8cfc469f3d 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -3,7 +3,8 @@ defmodule Explorer.Chain do The chain context. """ - import Ecto.Query, only: [from: 2, join: 4, or_where: 3, order_by: 2, order_by: 3, preload: 2, where: 2, where: 3] + import Ecto.Query, + only: [from: 2, join: 4, limit: 2, or_where: 3, order_by: 2, order_by: 3, preload: 2, where: 2, where: 3] alias Ecto.{Changeset, Multi} @@ -19,7 +20,7 @@ defmodule Explorer.Chain do } alias Explorer.Chain.Block.Reward - alias Explorer.Repo + alias Explorer.{PagingOptions, Repo} @typedoc """ The name of an association on the `t:Ecto.Schema.t/0` @@ -45,11 +46,11 @@ defmodule Explorer.Chain do """ @type pagination :: map() - @typep after_hash_option :: {:after_hash, Hash.t()} @typep direction_option :: {:direction, direction} @typep inserted_after_option :: {:inserted_after, DateTime.t()} @typep necessity_by_association_option :: {:necessity_by_association, necessity_by_association} @typep pagination_option :: {:pagination, pagination} + @typep paging_options :: PagingOptions.t() @typep params_option :: {:params, map()} @typep timeout_option :: {:timeout, timeout} @typep timestamps :: %{inserted_at: DateTime.t(), updated_at: DateTime.t()} @@ -1261,80 +1262,40 @@ defmodule Explorer.Chain do end @doc """ - Returns the list of collated transactions that occurred recently (10). + Returns the paged list of collated transactions that occurred recently from newest to oldest using `block_number` + and `index`. - iex> 2 |> insert_list(:transaction) |> with_block() - iex> insert(:transaction) # unvalidated transaction - iex> 8 |> insert_list(:transaction) |> with_block() - iex> recent_collated_transactions = Explorer.Chain.recent_collated_transactions() + iex> newest_first_transactions = 50 |> insert_list(:transaction) |> with_block() |> Enum.reverse() + iex> oldest_seen = Enum.at(newest_first_transactions, 9) + iex> paging_options = %Explorer.PagingOptions{page_size: 10, key: {oldest_seen.block_number, oldest_seen.index}} + iex> recent_collated_transactions = Explorer.Chain.recent_collated_transactions(paging_options: paging_options) iex> length(recent_collated_transactions) 10 - iex> Enum.all?(recent_collated_transactions, fn %Explorer.Chain.Transaction{block_hash: block_hash} -> - ...> !is_nil(block_hash) - ...> end) + iex> hd(recent_collated_transactions).hash == Enum.at(newest_first_transactions, 10).hash true - A `t:Explorer.Chain.Transaction.t/0` `hash` can be supplied to the `:after_hash` option, then only transactions in - after the transaction (with a greater index) in the same block or in a later block (with a greater number) will be - returned. This can be used to generate paging for collated transaction. - - iex> first_block = insert(:block, number: 1) - iex> first_transaction_in_first_block = :transaction |> insert() |> with_block(first_block) - iex> second_transaction_in_first_block = :transaction |> insert() |> with_block(first_block) - iex> second_block = insert(:block, number: 2) - iex> first_transaction_in_second_block = :transaction |> insert() |> with_block(second_block) - iex> after_first_transaciton_in_first_block = Explorer.Chain.recent_collated_transactions( - ...> after_hash: first_transaction_in_first_block.hash - ...> ) - iex> length(after_first_transaciton_in_first_block) - 2 - iex> after_second_transaciton_in_first_block = Explorer.Chain.recent_collated_transactions( - ...> after_hash: second_transaction_in_first_block.hash - ...> ) - iex> length(after_second_transaciton_in_first_block) - 1 - iex> after_first_transaciton_in_second_block = Explorer.Chain.recent_collated_transactions( - ...> after_hash: first_transaction_in_second_block.hash - ...> ) - iex> length(after_first_transaciton_in_second_block) - 0 - - When there are no collated transactions, an empty list is returned. - - iex> insert(:transaction) - iex> Explorer.Chain.recent_collated_transactions() - [] - - Using an unvalidated transaction's hash for `:after_hash` will also yield an empty list. - - iex> %Explorer.Chain.Transaction{hash: hash} = insert(:transaction) - iex> insert(:transaction) - iex> Explorer.Chain.recent_collated_transactions(after_hash: hash) - [] - ## Options * `:necessity_by_association` - use to load `t:association/0` as `:required` or `:optional`. If an association is `:required`, and the `t:Explorer.Chain.InternalTransaction.t/0` has no associated record for that association, then the `t:Explorer.Chain.InternalTransaction.t/0` will not be included in the list. + * `:paging_options` - a `t:Explorer.PagingOptions.t/0` used to specify the `:page_size` and + `:key` (a tuple of the lowest/oldest {block_number, index}) and. Results will be the transactions older than + the block number and index that are passed. """ - @spec recent_collated_transactions([after_hash_option | necessity_by_association_option]) :: [ + @spec recent_collated_transactions([paging_options | necessity_by_association_option]) :: [ Transaction.t() ] def recent_collated_transactions(options \\ []) when is_list(options) do necessity_by_association = Keyword.get(options, :necessity_by_association, %{}) + paging_options = Keyword.get(options, :paging_options, %PagingOptions{page_size: 50}) - query = - from( - transaction in Transaction, - where: not is_nil(transaction.block_number) and not is_nil(transaction.index), - order_by: [desc: transaction.block_number, desc: transaction.index], - limit: 10 - ) - - query - |> after_hash(options) + Transaction + |> where([transaction], not is_nil(transaction.block_number) and not is_nil(transaction.index)) + |> page_transaction(paging_options) + |> limit(^paging_options.page_size) + |> order_by([transaction], desc: transaction.block_number, desc: transaction.index) |> join_associations(necessity_by_association) |> Repo.all() end @@ -1634,23 +1595,6 @@ defmodule Explorer.Chain do |> Repo.paginate(pagination) end - defp after_hash(query, options) do - case Keyword.fetch(options, :after_hash) do - {:ok, hash} -> - from( - transaction in query, - join: hash_transaction in Transaction, - on: hash_transaction.hash == ^hash, - where: - transaction.block_number > hash_transaction.block_number or - (transaction.block_number == hash_transaction.block_number and transaction.index > hash_transaction.index) - ) - - :error -> - query - end - end - @spec changes_list(params :: map, [{:for, module}]) :: {:ok, changes :: map} | {:error, [Changeset.t()]} defp changes_list(params, named_arguments) when is_list(named_arguments) do ecto_schema_module = Keyword.fetch!(named_arguments, :for) @@ -1911,6 +1855,19 @@ defmodule Explorer.Chain do end) end + defp page_transaction(query, nil), do: query + + defp page_transaction(query, %PagingOptions{key: nil}), do: query + + defp page_transaction(query, %PagingOptions{key: {block_number, index}}) do + query + |> where( + [transaction], + transaction.block_number < ^block_number or + (transaction.block_number == ^block_number and transaction.index < ^index) + ) + end + defp reverse_chronologically(query) do from(q in query, order_by: [desc: q.inserted_at, desc: q.hash]) end diff --git a/apps/explorer/lib/explorer/chain/transaction.ex b/apps/explorer/lib/explorer/chain/transaction.ex index dd789b00cc..98185971ce 100644 --- a/apps/explorer/lib/explorer/chain/transaction.ex +++ b/apps/explorer/lib/explorer/chain/transaction.ex @@ -52,6 +52,11 @@ defmodule Explorer.Chain.Transaction do """ @type standard_v :: 0..3 + @typedoc """ + The index of the transaction in its block. + """ + @type transaction_index :: non_neg_integer() + @typedoc """ `t:standard_v/0` + `27` @@ -114,7 +119,7 @@ defmodule Explorer.Chain.Transaction do gas_price: wei_per_gas, gas_used: Gas.t() | nil, hash: Hash.t(), - index: non_neg_integer() | nil, + index: transaction_index | nil, input: Data.t(), internal_transactions: %Ecto.Association.NotLoaded{} | [InternalTransaction.t()], internal_transactions_indexed_at: DateTime.t(), diff --git a/apps/explorer/lib/explorer/paging_options.ex b/apps/explorer/lib/explorer/paging_options.ex new file mode 100644 index 0000000000..1bac2dc71a --- /dev/null +++ b/apps/explorer/lib/explorer/paging_options.ex @@ -0,0 +1,13 @@ +defmodule Explorer.PagingOptions do + @moduledoc """ + Defines paging options for paging by a stable key such as a timestamp or block + number and index. + """ + + @type t :: %__MODULE__{key: key, page_size: page_size} + + @typep key :: any() + @typep page_size :: non_neg_integer() + + defstruct [:key, :page_size] +end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 7b7a31b5dd..d2a5953928 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -861,4 +861,15 @@ defmodule Explorer.ChainTest do assert block_reward.reward == Chain.block_reward(block) end end + + describe "recent_collated_transactions/1" do + test "with no collated transactions it returns an empty list" do + assert [] == Explorer.Chain.recent_collated_transactions() + end + + test "it excludes pending transactions" do + insert(:transaction) + assert [] == Explorer.Chain.recent_collated_transactions() + end + end end diff --git a/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex b/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex index 27dd119498..684703e036 100644 --- a/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex +++ b/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex @@ -1,14 +1,15 @@ defmodule ExplorerWeb.TransactionController do use ExplorerWeb, :controller - alias Explorer.Chain + alias Explorer.{Chain, PagingOptions} def index(conn, params) do - with %{"last_seen_collated_hash" => last_seen_collated_hash_string} <- params, - {:ok, last_seen_collated_hash} <- Chain.string_to_transaction_hash(last_seen_collated_hash_string) do - do_index(conn, after_hash: last_seen_collated_hash) - else - _ -> do_index(conn) + case params do + %{"block_number" => block_number, "index" => index} -> + do_index(conn, paging_options: %PagingOptions{key: {block_number, index}, page_size: 50}) + + _ -> + do_index(conn) end end @@ -16,7 +17,7 @@ defmodule ExplorerWeb.TransactionController do redirect(conn, to: transaction_internal_transaction_path(conn, :index, locale, id)) end - defp do_index(conn, options \\ []) when is_list(options) do + defp do_index(conn, options \\ [paging_options: %PagingOptions{page_size: 50}]) when is_list(options) do full_options = Keyword.merge( [ @@ -30,21 +31,21 @@ defmodule ExplorerWeb.TransactionController do ) transactions = Chain.recent_collated_transactions(full_options) - last_seen_collated_hash = last_seen_collated_hash(transactions) transaction_count = Chain.transaction_count() render( conn, "index.html", - last_seen_collated_hash: last_seen_collated_hash, + earliest: earliest(transactions), transaction_count: transaction_count, transactions: transactions ) end - defp last_seen_collated_hash([]), do: nil + defp earliest([]), do: nil - defp last_seen_collated_hash(transactions) do - List.last(transactions).hash + defp earliest(transactions) do + last = List.last(transactions) + %{block_number: last.block_number, index: last.index} end end diff --git a/apps/explorer_web/lib/explorer_web/templates/transaction/index.html.eex b/apps/explorer_web/lib/explorer_web/templates/transaction/index.html.eex index e512042e3d..fa4cebb868 100644 --- a/apps/explorer_web/lib/explorer_web/templates/transaction/index.html.eex +++ b/apps/explorer_web/lib/explorer_web/templates/transaction/index.html.eex @@ -50,13 +50,13 @@ <%= link( - transaction.block, + transaction.block_number, class: "transactions__link", to: block_path(@conn, :show, @conn.assigns.locale, transaction.block) ) %> - <%= transaction.block.timestamp |> Timex.from_now %> + <%= transaction.block.timestamp %> <%= render ExplorerWeb.AddressView, "_link.html", conn: @conn, address: transaction.from_address %> @@ -73,16 +73,16 @@ - <%= if @last_seen_collated_hash do %> + <%= if @earliest do %> <%= link( - gettext("Next Page"), - class: "button button--secondary button--sm u-float-right mt-3", - to: transaction_path( - @conn, - :index, - @conn.assigns.locale, - %{"last_seen_collated_hash" => to_string(@last_seen_collated_hash)} - ) - ) %> + gettext("Older"), + class: "button button--secondary button--sm u-float-right mt-3", + to: transaction_path( + @conn, + :index, + @conn.assigns.locale, + %{"block_number" => @earliest.block_number, "index" => @earliest.index} + ) + ) %> <% end %> diff --git a/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs b/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs index 944105c9bc..fa94e086e8 100644 --- a/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs +++ b/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs @@ -1,10 +1,11 @@ defmodule ExplorerWeb.TransactionControllerTest do use ExplorerWeb.ConnCase + alias Explorer.Chain.Transaction import ExplorerWeb.Router.Helpers, only: [transaction_path: 4, transaction_internal_transaction_path: 4] describe "GET index/2" do - test "returns a transaction with a receipt", %{conn: conn} do + test "returns a collated transactions", %{conn: conn} do transaction = :transaction |> insert() @@ -22,43 +23,47 @@ defmodule ExplorerWeb.TransactionControllerTest do conn = get(conn, "/en/transactions") - assert length(conn.assigns.transactions) == 1 + assert conn.assigns.transaction_count == 1 end - test "returns no pending transactions", %{conn: conn} do - insert(:transaction) - - conn = get(conn, "/en/transactions") - - assert conn.assigns.transactions == [] - end + test "excludes pending transactions", %{conn: conn} do + %Transaction{hash: hash} = + :transaction + |> insert() + |> with_block() - test "only returns transactions that have a receipt", %{conn: conn} do insert(:transaction) conn = get(conn, "/en/transactions") - assert length(conn.assigns.transactions) == 0 + assert [%Transaction{hash: ^hash}] = conn.assigns.transactions end - test "paginates transactions using the last seen transaction", %{conn: conn} do - transaction = + test "returns next page of results based on last seen transaction", %{conn: conn} do + second_page_hashes = + 50 + |> insert_list(:transaction) + |> with_block() + |> Enum.map(& &1.hash) + + %Transaction{block_number: block_number, index: index} = :transaction |> insert() |> with_block() - conn = - get( - conn, - "/en/transactions", - last_seen_collated_hash: to_string(transaction.hash) - ) + conn = get(conn, "/en/transactions", %{"block_number" => block_number, "index" => index}) - assert conn.assigns.transactions == [] + actual_hashes = + conn.assigns.transactions + |> Enum.map(& &1.hash) + |> Enum.reverse() + + assert second_page_hashes == actual_hashes end test "sends back the number of transactions", %{conn: conn} do insert(:transaction) + |> with_block() conn = get(conn, "/en/transactions") From a66e2b6e8384a234ffc728e33de885c067a087b0 Mon Sep 17 00:00:00 2001 From: Tim Mecklem Date: Tue, 5 Jun 2018 15:16:20 -0400 Subject: [PATCH 3/6] Remove joins to address table Co-authored-by: Stamates --- .../explorer_web/controllers/transaction_controller.ex | 4 +--- .../lib/explorer_web/templates/address/_link.html.eex | 10 +++++----- .../address_internal_transaction/index.html.eex | 4 ++-- .../templates/address_transaction/index.html.eex | 4 ++-- .../templates/block_transaction/index.html.eex | 4 ++-- .../lib/explorer_web/templates/chain/_blocks.html.eex | 2 +- .../templates/chain/_transactions.html.eex | 4 ++-- .../templates/pending_transaction/index.html.eex | 4 ++-- .../explorer_web/templates/transaction/index.html.eex | 4 ++-- .../transaction_internal_transaction/index.html.eex | 4 ++-- .../lib/explorer_web/views/address_view.ex | 3 ++- 11 files changed, 23 insertions(+), 24 deletions(-) diff --git a/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex b/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex index 684703e036..e262d55711 100644 --- a/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex +++ b/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex @@ -22,9 +22,7 @@ defmodule ExplorerWeb.TransactionController do Keyword.merge( [ necessity_by_association: %{ - block: :required, - from_address: :optional, - to_address: :optional + block: :required } ], options diff --git a/apps/explorer_web/lib/explorer_web/templates/address/_link.html.eex b/apps/explorer_web/lib/explorer_web/templates/address/_link.html.eex index d82944a646..96a579f41a 100644 --- a/apps/explorer_web/lib/explorer_web/templates/address/_link.html.eex +++ b/apps/explorer_web/lib/explorer_web/templates/address/_link.html.eex @@ -1,14 +1,14 @@