From cccbf129cc64370c675b470538d84faeda9d5a4e Mon Sep 17 00:00:00 2001 From: pasqu4le Date: Wed, 26 Jun 2019 14:15:41 +0200 Subject: [PATCH] More transaction controllers improvements --- CHANGELOG.md | 1 + .../block_transaction_controller.ex | 2 +- .../controllers/transaction_controller.ex | 23 ++++++++----------- ...saction_internal_transaction_controller.ex | 3 ++- .../transaction_raw_trace_controller.ex | 10 +------- apps/explorer/lib/explorer/chain.ex | 9 ++++---- apps/explorer/test/explorer/chain_test.exs | 17 ++++++++------ 7 files changed, 28 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5466e53feb..e98a8f6302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ - [#2204](https://github.com/poanetwork/blockscout/pull/2204) - fix large contract verification - [#2247](https://github.com/poanetwork/blockscout/pull/2247) - hide logs search if there are no logs - [#2248](https://github.com/poanetwork/blockscout/pull/2248) - sort block after query execution for average block time +- [#2249](https://github.com/poanetwork/blockscout/pull/2249) - More transaction controllers improvements ### Chore - [#2127](https://github.com/poanetwork/blockscout/pull/2127) - use previouse chromedriver version diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/block_transaction_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/block_transaction_controller.ex index f54671e795..15b06f1edd 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/block_transaction_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/block_transaction_controller.ex @@ -17,7 +17,7 @@ defmodule BlockScoutWeb.BlockTransactionController do Keyword.merge( [ necessity_by_association: %{ - :block => :required, + :block => :optional, [created_contract_address: :names] => :optional, [from_address: :names] => :required, [to_address: :names] => :optional diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex index b40e6988e0..1bedf97c1d 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex @@ -61,21 +61,16 @@ defmodule BlockScoutWeb.TransactionController do end def show(conn, %{"id" => id}) do - case Chain.string_to_transaction_hash(id) do - {:ok, transaction_hash} -> show_transaction(conn, id, Chain.hash_to_transaction(transaction_hash)) - :error -> conn |> put_status(422) |> render("invalid.html", transaction_hash: id) - end - end - - defp show_transaction(conn, id, {:error, :not_found}) do - conn |> put_status(404) |> render("not_found.html", transaction_hash: id) - end - - defp show_transaction(conn, id, {:ok, %Chain.Transaction{} = transaction}) do - if Chain.transaction_has_token_transfers?(transaction.hash) do - redirect(conn, to: transaction_token_transfer_path(conn, :index, id)) + with {:ok, transaction_hash} <- Chain.string_to_transaction_hash(id), + {:ok, %Chain.Transaction{} = transaction} <- Chain.hash_to_transaction(transaction_hash) do + if Chain.transaction_has_token_transfers?(transaction.hash) do + redirect(conn, to: transaction_token_transfer_path(conn, :index, id)) + else + redirect(conn, to: transaction_internal_transaction_path(conn, :index, id)) + end else - redirect(conn, to: transaction_internal_transaction_path(conn, :index, id)) + :error -> conn |> put_status(422) |> render("invalid.html", transaction_hash: id) + {:error, :not_found} -> conn |> put_status(404) |> render("not_found.html", transaction_hash: id) end end end diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_internal_transaction_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_internal_transaction_controller.ex index 7c24b8d3b9..159d144cdd 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_internal_transaction_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_internal_transaction_controller.ex @@ -17,7 +17,8 @@ defmodule BlockScoutWeb.TransactionInternalTransactionController do necessity_by_association: %{ [created_contract_address: :names] => :optional, [from_address: :names] => :optional, - [to_address: :names] => :optional + [to_address: :names] => :optional, + [transaction: :block] => :optional } ], paging_options(params) diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_raw_trace_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_raw_trace_controller.ex index a73290faab..3d090e8a9b 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_raw_trace_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_raw_trace_controller.ex @@ -19,15 +19,7 @@ defmodule BlockScoutWeb.TransactionRawTraceController do :token_transfers => :optional } ) do - options = [ - necessity_by_association: %{ - [created_contract_address: :names] => :optional, - [from_address: :names] => :optional, - [to_address: :names] => :optional - } - ] - - internal_transactions = Chain.transaction_to_internal_transactions(transaction, options) + internal_transactions = Chain.transaction_to_internal_transactions(transaction) render( conn, diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index e4416030b0..673cec78e7 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -1052,7 +1052,7 @@ defmodule Explorer.Chain do when is_list(options) do necessity_by_association = Keyword.get(options, :necessity_by_association, %{}) - fetch_transactions() + Transaction |> where(hash: ^hash) |> join_associations(necessity_by_association) |> Repo.one() @@ -1953,7 +1953,6 @@ defmodule Explorer.Chain do |> Keyword.get(:paging_options, @default_paging_options) |> fetch_transactions() |> where([transaction], not is_nil(transaction.block_number) and not is_nil(transaction.index)) - |> order_by([transaction], desc: transaction.block_number, desc: transaction.index) |> join_associations(necessity_by_association) |> preload([{:token_transfers, [:token, :from_address, :to_address]}]) |> Repo.all() @@ -2146,7 +2145,7 @@ defmodule Explorer.Chain do |> page_internal_transaction(paging_options) |> limit(^paging_options.page_size) |> order_by([internal_transaction], asc: internal_transaction.index) - |> preload(transaction: :block) + |> preload(:transaction) |> Repo.all() end @@ -2702,9 +2701,9 @@ defmodule Explorer.Chain do @spec transaction_has_token_transfers?(Hash.t()) :: boolean() def transaction_has_token_transfers?(transaction_hash) do - query = from(tt in TokenTransfer, where: tt.transaction_hash == ^transaction_hash, limit: 1, select: 1) + query = from(tt in TokenTransfer, where: tt.transaction_hash == ^transaction_hash) - Repo.one(query) != nil + Repo.exists?(query) end @spec address_tokens_with_balance(Hash.Address.t(), [any()]) :: [] diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index f789a883ea..6f43543a70 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -2110,11 +2110,14 @@ defmodule Explorer.ChainTest do ]) ) - assert internal_transaction.transaction.block.number == block.number + assert internal_transaction.transaction.block_number == block.number end test "with transaction with internal transactions loads associations with in necessity_by_association" do - transaction = insert(:transaction) + transaction = + :transaction + |> insert() + |> with_block() insert(:internal_transaction_create, transaction: transaction, @@ -2127,7 +2130,7 @@ defmodule Explorer.ChainTest do %InternalTransaction{ from_address: %Ecto.Association.NotLoaded{}, to_address: %Ecto.Association.NotLoaded{}, - transaction: %Transaction{} + transaction: %Transaction{block: %Ecto.Association.NotLoaded{}} } ] = Chain.transaction_to_internal_transactions(transaction) @@ -2135,15 +2138,15 @@ defmodule Explorer.ChainTest do %InternalTransaction{ from_address: %Address{}, to_address: nil, - transaction: %Transaction{} + transaction: %Transaction{block: %Block{}} } ] = Chain.transaction_to_internal_transactions( transaction, necessity_by_association: %{ - from_address: :optional, - to_address: :optional, - transaction: :optional + :from_address => :optional, + :to_address => :optional, + [transaction: :block] => :optional } ) end