From ba2fb3557b776cbd72914b50937836af5957aec1 Mon Sep 17 00:00:00 2001
From: pasqu4le <pasqu4le@gmail.com>
Date: Fri, 19 Jul 2019 14:01:52 +0200
Subject: [PATCH 1/4] Improve controllers queries

Problem: after #2111, #2249, #2216 and #2305 a few remaining controllers still have not been checked for possible improvements.

Solution: check them and remove unnecessary preloads and queries.
---
 .../address_contract_controller.ex            |  12 +-
 .../address_read_contract_controller.ex       |  12 +-
 .../controllers/api/rpc/block_controller.ex   |   3 +-
 .../controllers/chain_controller.ex           |  12 +-
 apps/explorer/lib/explorer/chain.ex           |  47 ++++----
 .../lib/explorer/smart_contract/reader.ex     | 104 ++++++++++--------
 apps/explorer/test/explorer/chain_test.exs    |  14 ++-
 .../explorer/smart_contract/reader_test.exs   |   2 +-
 8 files changed, 131 insertions(+), 75 deletions(-)

diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/address_contract_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/address_contract_controller.ex
index bc4df8c4be..5db900b12b 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/address_contract_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/address_contract_controller.ex
@@ -8,8 +8,18 @@ defmodule BlockScoutWeb.AddressContractController do
   alias Indexer.Fetcher.CoinBalanceOnDemand
 
   def index(conn, %{"address_id" => address_hash_string}) do
+    address_options = [
+      necessity_by_association: %{
+        :contracts_creation_internal_transaction => :optional,
+        :names => :optional,
+        :smart_contract => :optional,
+        :token => :optional,
+        :contracts_creation_transaction => :optional
+      }
+    ]
+
     with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string),
-         {:ok, address} <- Chain.find_contract_address(address_hash) do
+         {:ok, address} <- Chain.find_contract_address(address_hash, address_options, true) do
       render(
         conn,
         "index.html",
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/address_read_contract_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/address_read_contract_controller.ex
index d57aa30807..0849689dce 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/address_read_contract_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/address_read_contract_controller.ex
@@ -15,8 +15,18 @@ defmodule BlockScoutWeb.AddressReadContractController do
   import BlockScoutWeb.AddressController, only: [transaction_count: 1, validation_count: 1]
 
   def index(conn, %{"address_id" => address_hash_string}) do
+    address_options = [
+      necessity_by_association: %{
+        :contracts_creation_internal_transaction => :optional,
+        :names => :optional,
+        :smart_contract => :optional,
+        :token => :optional,
+        :contracts_creation_transaction => :optional
+      }
+    ]
+
     with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string),
-         {:ok, address} <- Chain.find_contract_address(address_hash) do
+         {:ok, address} <- Chain.find_contract_address(address_hash, address_options, true) do
       render(
         conn,
         "index.html",
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex
index 91569a96da..16d73d420f 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex
@@ -8,8 +8,7 @@ defmodule BlockScoutWeb.API.RPC.BlockController do
   def getblockreward(conn, params) do
     with {:block_param, {:ok, unsafe_block_number}} <- {:block_param, Map.fetch(params, "blockno")},
          {:ok, block_number} <- ChainWeb.param_to_block_number(unsafe_block_number),
-         block_options = [necessity_by_association: %{transactions: :optional}],
-         {:ok, block} <- Chain.number_to_block(block_number, block_options) do
+         {:ok, block} <- Chain.number_to_block(block_number) do
       reward = Chain.block_reward(block)
 
       render(conn, :block_reward, block: block, reward: reward)
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/chain_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/chain_controller.ex
index 40a0fd3bbb..2c16cd1b03 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/chain_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/chain_controller.ex
@@ -2,7 +2,7 @@ defmodule BlockScoutWeb.ChainController do
   use BlockScoutWeb, :controller
 
   alias BlockScoutWeb.ChainView
-  alias Explorer.{Chain, PagingOptions, Repo}
+  alias Explorer.{Chain, PagingOptions}
   alias Explorer.Chain.{Address, Block, Transaction}
   alias Explorer.Chain.Supply.RSK
   alias Explorer.Counters.AverageBlockTime
@@ -72,9 +72,15 @@ defmodule BlockScoutWeb.ChainController do
   def chain_blocks(conn, _params) do
     if ajax?(conn) do
       blocks =
-        [paging_options: %PagingOptions{page_size: 4}]
+        [
+          paging_options: %PagingOptions{page_size: 4},
+          necessity_by_association: %{
+            [miner: :names] => :optional,
+            :transactions => :optional,
+            :rewards => :optional
+          }
+        ]
         |> Chain.list_blocks()
-        |> Repo.preload([[miner: :names], :transactions, :rewards])
         |> Enum.map(fn block ->
           %{
             chain_block_html:
diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex
index 17116658bf..59836a90a6 100644
--- a/apps/explorer/lib/explorer/chain.ex
+++ b/apps/explorer/lib/explorer/chain.ex
@@ -930,29 +930,40 @@ defmodule Explorer.Chain do
     Repo.all(query)
   end
 
-  @spec find_contract_address(Hash.t()) :: {:ok, Address.t()} | {:error, :not_found}
-  def find_contract_address(%Hash{byte_count: unquote(Hash.Address.byte_count())} = hash) do
+  @doc """
+  Finds an `t:Explorer.Chain.Address.t/0` that has the provided `t:Explorer.Chain.Address.t/0` `hash` and a contract.
+
+  ## Options
+
+    * `:necessity_by_association` - use to load `t:association/0` as `:required` or `:optional`.  If an association is
+      `:required`, and the `t:Explorer.Chain.Address.t/0` has no associated record for that association,
+      then the `t:Explorer.Chain.Address.t/0` will not be included in the list.
+
+  Optionally it also accepts a boolean to fetch the `has_decompiled_code?` virtual field or not
+
+  """
+  @spec find_contract_address(Hash.Address.t(), [necessity_by_association_option], boolean()) ::
+          {:ok, Address.t()} | {:error, :not_found}
+  def find_contract_address(
+        %Hash{byte_count: unquote(Hash.Address.byte_count())} = hash,
+        options \\ [],
+        query_decompiled_code_flag \\ false
+      ) do
+    necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
+
     query =
       from(
         address in Address,
-        preload: [
-          :contracts_creation_internal_transaction,
-          :names,
-          :smart_contract,
-          :token,
-          :contracts_creation_transaction
-        ],
         where: address.hash == ^hash and not is_nil(address.contract_code)
       )
 
-    query_with_decompiled_flag = with_decompiled_code_flag(query, hash)
-
-    address = Repo.one(query_with_decompiled_flag)
-
-    if address do
-      {:ok, address}
-    else
-      {:error, :not_found}
+    query
+    |> join_associations(necessity_by_association)
+    |> with_decompiled_code_flag(hash, query_decompiled_code_flag)
+    |> Repo.one()
+    |> case do
+      nil -> {:error, :not_found}
+      address -> {:ok, address}
     end
   end
 
@@ -3276,8 +3287,6 @@ defmodule Explorer.Chain do
 
   defp staking_pool_filter(query, _), do: query
 
-  defp with_decompiled_code_flag(query, hash, use_option \\ true)
-
   defp with_decompiled_code_flag(query, _hash, false), do: query
 
   defp with_decompiled_code_flag(query, hash, true) do
diff --git a/apps/explorer/lib/explorer/smart_contract/reader.ex b/apps/explorer/lib/explorer/smart_contract/reader.ex
index 72b40f55a4..cc3aae5e83 100644
--- a/apps/explorer/lib/explorer/smart_contract/reader.ex
+++ b/apps/explorer/lib/explorer/smart_contract/reader.ex
@@ -8,7 +8,7 @@ defmodule Explorer.SmartContract.Reader do
 
   alias EthereumJSONRPC.Contract
   alias Explorer.Chain
-  alias Explorer.Chain.Hash
+  alias Explorer.Chain.{Hash, SmartContract}
 
   @typedoc """
   Map of functions to call with the values for the function to be called with.
@@ -34,6 +34,8 @@ defmodule Explorer.SmartContract.Reader do
   @doc """
   Queries the contract functions on the blockchain and returns the call results.
 
+  Optionally accepts the abi if it has already been fetched.
+
   ## Examples
 
   Note that for this example to work the database must be up to date with the
@@ -57,14 +59,20 @@ defmodule Explorer.SmartContract.Reader do
       )
       # => %{"sum" => {:error, "Data overflow encoding int, data `abc` cannot fit in 256 bits"}}
   """
-  @spec query_verified_contract(Hash.Address.t(), functions()) :: functions_results()
-  def query_verified_contract(address_hash, functions) do
+  @spec query_verified_contract(Hash.Address.t(), functions(), SmartContract.abi() | nil) :: functions_results()
+  def query_verified_contract(address_hash, functions, mabi \\ nil) do
     contract_address = Hash.to_string(address_hash)
 
     abi =
-      address_hash
-      |> Chain.address_hash_to_smart_contract()
-      |> Map.get(:abi)
+      case mabi do
+        nil ->
+          address_hash
+          |> Chain.address_hash_to_smart_contract()
+          |> Map.get(:abi)
+
+        _ ->
+          mabi
+      end
 
     query_contract(contract_address, abi, functions)
   end
@@ -156,41 +164,41 @@ defmodule Explorer.SmartContract.Reader do
   """
   @spec read_only_functions(Hash.t()) :: [%{}]
   def read_only_functions(contract_address_hash) do
-    contract_address_hash
-    |> Chain.address_hash_to_smart_contract()
-    |> Map.get(:abi, [])
-    |> Enum.filter(& &1["constant"])
-    |> fetch_current_value_from_blockchain(contract_address_hash, [])
-    |> Enum.reverse()
-  end
-
-  def fetch_current_value_from_blockchain(
-        [%{"inputs" => []} = function | tail],
-        contract_address_hash,
-        acc
-      ) do
-    values =
-      fetch_from_blockchain(contract_address_hash, %{
-        name: function["name"],
-        args: function["inputs"],
-        outputs: function["outputs"]
-      })
+    abi =
+      contract_address_hash
+      |> Chain.address_hash_to_smart_contract()
+      |> Map.get(:abi)
 
-    formatted = Map.replace!(function, "outputs", values)
+    case abi do
+      nil ->
+        []
 
-    fetch_current_value_from_blockchain(tail, contract_address_hash, [formatted | acc])
+      _ ->
+        abi
+        |> Enum.filter(& &1["constant"])
+        |> Enum.map(&fetch_current_value_from_blockchain(&1, abi, contract_address_hash))
+    end
   end
 
-  def fetch_current_value_from_blockchain([function | tail], contract_address_hash, acc) do
-    values = link_outputs_and_values(%{}, Map.get(function, "outputs", []), function["name"])
+  defp fetch_current_value_from_blockchain(function, abi, contract_address_hash) do
+    values =
+      case function do
+        %{"inputs" => []} ->
+          name = function["name"]
+          args = function["inputs"]
+          outputs = function["outputs"]
+
+          contract_address_hash
+          |> query_verified_contract(%{name => normalize_args(args)}, abi)
+          |> link_outputs_and_values(outputs, name)
 
-    formatted = Map.replace!(function, "outputs", values)
+        _ ->
+          link_outputs_and_values(%{}, Map.get(function, "outputs", []), function["name"])
+      end
 
-    fetch_current_value_from_blockchain(tail, contract_address_hash, [formatted | acc])
+    Map.replace!(function, "outputs", values)
   end
 
-  def fetch_current_value_from_blockchain([], _contract_address_hash, acc), do: acc
-
   @doc """
   Fetches the blockchain value of a function that requires arguments.
   """
@@ -201,23 +209,27 @@ defmodule Explorer.SmartContract.Reader do
 
   @spec query_function(Hash.t(), %{name: String.t(), args: [term()]}) :: [%{}]
   def query_function(contract_address_hash, %{name: name, args: args}) do
-    function =
+    abi =
       contract_address_hash
       |> Chain.address_hash_to_smart_contract()
-      |> Map.get(:abi, [])
-      |> Enum.filter(fn function -> function["name"] == name end)
-      |> List.first()
-
-    fetch_from_blockchain(contract_address_hash, %{
-      name: name,
-      args: args,
-      outputs: function["outputs"]
-    })
-  end
+      |> Map.get(:abi)
+
+    outputs =
+      case abi do
+        nil ->
+          nil
+
+        _ ->
+          function =
+            abi
+            |> Enum.filter(fn function -> function["name"] == name end)
+            |> List.first()
+
+          function["outputs"]
+      end
 
-  defp fetch_from_blockchain(contract_address_hash, %{name: name, args: args, outputs: outputs}) do
     contract_address_hash
-    |> query_verified_contract(%{name => normalize_args(args)})
+    |> query_verified_contract(%{name => normalize_args(args)}, abi)
     |> link_outputs_and_values(outputs, name)
   end
 
diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs
index 38e61dee7d..fc4f3eab85 100644
--- a/apps/explorer/test/explorer/chain_test.exs
+++ b/apps/explorer/test/explorer/chain_test.exs
@@ -2480,7 +2480,17 @@ defmodule Explorer.ChainTest do
         insert(:address, contract_code: Factory.data("contract_code"), smart_contract: nil, names: [])
         |> Repo.preload([:contracts_creation_internal_transaction, :contracts_creation_transaction, :token])
 
-      response = Chain.find_contract_address(address.hash)
+      options = [
+        necessity_by_association: %{
+          :contracts_creation_internal_transaction => :optional,
+          :names => :optional,
+          :smart_contract => :optional,
+          :token => :optional,
+          :contracts_creation_transaction => :optional
+        }
+      ]
+
+      response = Chain.find_contract_address(address.hash, options, true)
 
       assert response == {:ok, address}
     end
@@ -2527,7 +2537,7 @@ defmodule Explorer.ChainTest do
     end
 
     test "with block without transactions", %{block: block, emission_reward: emission_reward} do
-      assert emission_reward.reward == Chain.block_reward(block)
+      assert emission_reward.reward == Chain.block_reward(block.number)
     end
   end
 
diff --git a/apps/explorer/test/explorer/smart_contract/reader_test.exs b/apps/explorer/test/explorer/smart_contract/reader_test.exs
index ab63b74d1e..82f0a54385 100644
--- a/apps/explorer/test/explorer/smart_contract/reader_test.exs
+++ b/apps/explorer/test/explorer/smart_contract/reader_test.exs
@@ -102,7 +102,7 @@ defmodule Explorer.SmartContract.ReaderTest do
     end
   end
 
-  describe "query_verified_contract/2" do
+  describe "query_verified_contract/3" do
     test "correctly returns the results of the smart contract functions" do
       hash =
         :smart_contract

From d528d4d1f5857318c45ea1720fc4ba61843ff072 Mon Sep 17 00:00:00 2001
From: pasqu4le <pasqu4le@gmail.com>
Date: Fri, 19 Jul 2019 14:41:00 +0200
Subject: [PATCH 2/4] Reduce chain function input to hashes (or ids) where
 possible

Problem: a lot of function take an entity as an parameter, but only use its hash/id.
This makes tracing the necessary scope of functions harder and is one of the cause for unnecessary queries/code execution.

Solution: refactor these functions to take only a hash (or an id).
---
 .../controllers/api/rpc/block_controller.ex   |  2 +-
 .../api/rpc/transaction_controller.ex         |  2 +-
 .../block_transaction_controller.ex           |  4 +-
 ...saction_internal_transaction_controller.ex |  2 +-
 .../controllers/transaction_log_controller.ex |  2 +-
 .../transaction_raw_trace_controller.ex       |  2 +-
 .../transaction_token_transfer_controller.ex  |  2 +-
 apps/explorer/lib/explorer/chain.ex           | 48 ++++++----------
 apps/explorer/test/explorer/chain_test.exs    | 56 +++++++++----------
 9 files changed, 54 insertions(+), 66 deletions(-)

diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex
index 16d73d420f..835e19bf3e 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/block_controller.ex
@@ -9,7 +9,7 @@ defmodule BlockScoutWeb.API.RPC.BlockController do
     with {:block_param, {:ok, unsafe_block_number}} <- {:block_param, Map.fetch(params, "blockno")},
          {:ok, block_number} <- ChainWeb.param_to_block_number(unsafe_block_number),
          {:ok, block} <- Chain.number_to_block(block_number) do
-      reward = Chain.block_reward(block)
+      reward = Chain.block_reward(block_number)
 
       render(conn, :block_reward, block: block, reward: reward)
     else
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/transaction_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/transaction_controller.ex
index 344dbe1037..565ea4b6a5 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/transaction_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/transaction_controller.ex
@@ -10,7 +10,7 @@ defmodule BlockScoutWeb.API.RPC.TransactionController do
          {:format, {:ok, transaction_hash}} <- to_transaction_hash(txhash_param),
          {:transaction, {:ok, transaction}} <- transaction_from_hash(transaction_hash),
          paging_options <- paging_options(params) do
-      logs = Chain.transaction_to_logs(transaction, paging_options)
+      logs = Chain.transaction_to_logs(transaction_hash, paging_options)
       {logs, next_page} = split_list_by_page(logs)
 
       render(conn, :gettxinfo, %{
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 15b06f1edd..a1e4cbeb09 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
@@ -26,7 +26,7 @@ defmodule BlockScoutWeb.BlockTransactionController do
           paging_options(params)
         )
 
-      transactions_plus_one = Chain.block_to_transactions(block, full_options)
+      transactions_plus_one = Chain.block_to_transactions(block.hash, full_options)
 
       {transactions, next_page} = split_list_by_page(transactions_plus_one)
 
@@ -89,7 +89,7 @@ defmodule BlockScoutWeb.BlockTransactionController do
                :rewards => :optional
              }
            ) do
-      block_transaction_count = Chain.block_to_transaction_count(block)
+      block_transaction_count = Chain.block_to_transaction_count(block.hash)
 
       render(
         conn,
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 159d144cdd..ae847b374d 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
@@ -24,7 +24,7 @@ defmodule BlockScoutWeb.TransactionInternalTransactionController do
           paging_options(params)
         )
 
-      internal_transactions_plus_one = Chain.transaction_to_internal_transactions(transaction, full_options)
+      internal_transactions_plus_one = Chain.transaction_to_internal_transactions(hash, full_options)
 
       {internal_transactions, next_page} = split_list_by_page(internal_transactions_plus_one)
 
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_log_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_log_controller.ex
index 0a87d12493..a4bd12d66b 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_log_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_log_controller.ex
@@ -22,7 +22,7 @@ defmodule BlockScoutWeb.TransactionLogController do
           paging_options(params)
         )
 
-      logs_plus_one = Chain.transaction_to_logs(transaction, full_options)
+      logs_plus_one = Chain.transaction_to_logs(transaction_hash, full_options)
 
       {logs, next_page} = split_list_by_page(logs_plus_one)
 
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 3d090e8a9b..250a6b4442 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,7 +19,7 @@ defmodule BlockScoutWeb.TransactionRawTraceController do
                :token_transfers => :optional
              }
            ) do
-      internal_transactions = Chain.transaction_to_internal_transactions(transaction)
+      internal_transactions = Chain.transaction_to_internal_transactions(hash)
 
       render(
         conn,
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex
index cfb215da3e..5f4a9c8229 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex
@@ -24,7 +24,7 @@ defmodule BlockScoutWeb.TransactionTokenTransferController do
           paging_options(params)
         )
 
-      token_transfers_plus_one = Chain.transaction_to_token_transfers(transaction, full_options)
+      token_transfers_plus_one = Chain.transaction_to_token_transfers(hash, full_options)
 
       {token_transfers, next_page} = split_list_by_page(token_transfers_plus_one)
 
diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex
index 59836a90a6..2c40f6bbc1 100644
--- a/apps/explorer/lib/explorer/chain.ex
+++ b/apps/explorer/lib/explorer/chain.ex
@@ -364,8 +364,8 @@ defmodule Explorer.Chain do
 
   Uncles are not currently accounted for.
   """
-  @spec block_reward(Block.t()) :: Wei.t()
-  def block_reward(%Block{number: block_number}) do
+  @spec block_reward(Block.block_number()) :: Wei.t()
+  def block_reward(block_number) do
     query =
       from(
         block in Block,
@@ -415,8 +415,8 @@ defmodule Explorer.Chain do
       `:key` (a tuple of the lowest/oldest `{index}`) and. Results will be the transactions older than
       the `index` that are passed.
   """
-  @spec block_to_transactions(Block.t(), [paging_options | necessity_by_association_option]) :: [Transaction.t()]
-  def block_to_transactions(%Block{hash: block_hash}, options \\ []) when is_list(options) do
+  @spec block_to_transactions(Hash.Full.t(), [paging_options | necessity_by_association_option]) :: [Transaction.t()]
+  def block_to_transactions(block_hash, options \\ []) when is_list(options) do
     necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
 
     options
@@ -432,8 +432,8 @@ defmodule Explorer.Chain do
   @doc """
   Counts the number of `t:Explorer.Chain.Transaction.t/0` in the `block`.
   """
-  @spec block_to_transaction_count(Block.t()) :: non_neg_integer()
-  def block_to_transaction_count(%Block{hash: block_hash}) do
+  @spec block_to_transaction_count(Hash.Full.t()) :: non_neg_integer()
+  def block_to_transaction_count(block_hash) do
     query =
       from(
         transaction in Transaction,
@@ -843,7 +843,7 @@ defmodule Explorer.Chain do
   Returns `{:error, :not_found}` if there is no address by that hash present.
   Returns `{:error, :no_balance}` if there is no balance for that address at that block.
   """
-  @spec get_balance_as_of_block(Hash.Address.t(), integer | :earliest | :latest | :pending) ::
+  @spec get_balance_as_of_block(Hash.Address.t(), Block.block_number() | :earliest | :latest | :pending) ::
           {:ok, Wei.t()} | {:error, :no_balance} | {:error, :not_found}
   def get_balance_as_of_block(address, block) when is_integer(block) do
     coin_balance_query =
@@ -967,7 +967,7 @@ defmodule Explorer.Chain do
     end
   end
 
-  @spec find_decompiled_contract_address(Hash.t()) :: {:ok, Address.t()} | {:error, :not_found}
+  @spec find_decompiled_contract_address(Hash.Address.t()) :: {:ok, Address.t()} | {:error, :not_found}
   def find_decompiled_contract_address(%Hash{byte_count: unquote(Hash.Address.byte_count())} = hash) do
     query =
       from(
@@ -2220,14 +2220,10 @@ defmodule Explorer.Chain do
 
   """
 
-  @spec transaction_to_internal_transactions(Transaction.t(), [paging_options | necessity_by_association_option]) :: [
+  @spec transaction_to_internal_transactions(Hash.Full.t(), [paging_options | necessity_by_association_option]) :: [
           InternalTransaction.t()
         ]
-  def transaction_to_internal_transactions(
-        %Transaction{hash: %Hash{byte_count: unquote(Hash.Full.byte_count())} = hash},
-        options \\ []
-      )
-      when is_list(options) do
+  def transaction_to_internal_transactions(hash, options \\ []) when is_list(options) do
     necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
     paging_options = Keyword.get(options, :paging_options, @default_paging_options)
 
@@ -2255,12 +2251,8 @@ defmodule Explorer.Chain do
       the `index` that are passed.
 
   """
-  @spec transaction_to_logs(Transaction.t(), [paging_options | necessity_by_association_option]) :: [Log.t()]
-  def transaction_to_logs(
-        %Transaction{hash: %Hash{byte_count: unquote(Hash.Full.byte_count())} = transaction_hash},
-        options \\ []
-      )
-      when is_list(options) do
+  @spec transaction_to_logs(Hash.Full.t(), [paging_options | necessity_by_association_option]) :: [Log.t()]
+  def transaction_to_logs(transaction_hash, options \\ []) when is_list(options) do
     necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
     paging_options = Keyword.get(options, :paging_options, @default_paging_options)
 
@@ -2287,14 +2279,10 @@ defmodule Explorer.Chain do
       the `index` that are passed.
 
   """
-  @spec transaction_to_token_transfers(Transaction.t(), [paging_options | necessity_by_association_option]) :: [
+  @spec transaction_to_token_transfers(Hash.Full.t(), [paging_options | necessity_by_association_option]) :: [
           TokenTransfer.t()
         ]
-  def transaction_to_token_transfers(
-        %Transaction{hash: %Hash{byte_count: unquote(Hash.Full.byte_count())} = transaction_hash},
-        options \\ []
-      )
-      when is_list(options) do
+  def transaction_to_token_transfers(transaction_hash, options \\ []) when is_list(options) do
     necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
     paging_options = Keyword.get(options, :paging_options, @default_paging_options)
 
@@ -2521,16 +2509,16 @@ defmodule Explorer.Chain do
     |> repo.insert(on_conflict: :nothing, conflict_target: [:address_hash, :name])
   end
 
-  @spec address_hash_to_address_with_source_code(%Explorer.Chain.Hash{}) :: %Explorer.Chain.Address{} | nil
-  def address_hash_to_address_with_source_code(%Explorer.Chain.Hash{} = address_hash) do
+  @spec address_hash_to_address_with_source_code(Hash.Address.t()) :: Address.t() | nil
+  def address_hash_to_address_with_source_code(address_hash) do
     case Repo.get(Address, address_hash) do
       nil -> nil
       address -> Repo.preload(address, [:smart_contract, :decompiled_smart_contracts])
     end
   end
 
-  @spec address_hash_to_smart_contract(%Explorer.Chain.Hash{}) :: %Explorer.Chain.SmartContract{} | nil
-  def address_hash_to_smart_contract(%Explorer.Chain.Hash{} = address_hash) do
+  @spec address_hash_to_smart_contract(Hash.Address.t()) :: SmartContract.t() | nil
+  def address_hash_to_smart_contract(address_hash) do
     query =
       from(
         smart_contract in SmartContract,
diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs
index fc4f3eab85..5ee506982b 100644
--- a/apps/explorer/test/explorer/chain_test.exs
+++ b/apps/explorer/test/explorer/chain_test.exs
@@ -630,7 +630,7 @@ defmodule Explorer.ChainTest do
 
       assert Repo.aggregate(Transaction, :count, :hash) == 0
 
-      assert [] = Chain.block_to_transactions(block)
+      assert [] = Chain.block_to_transactions(block.hash)
     end
 
     test "with transactions" do
@@ -639,7 +639,7 @@ defmodule Explorer.ChainTest do
         |> insert()
         |> with_block()
 
-      assert [%Transaction{hash: ^transaction_hash}] = Chain.block_to_transactions(block)
+      assert [%Transaction{hash: ^transaction_hash}] = Chain.block_to_transactions(block.hash)
     end
 
     test "with transactions can be paginated by {index}" do
@@ -657,7 +657,7 @@ defmodule Explorer.ChainTest do
         |> with_block(block)
 
       assert second_page_hashes ==
-               block
+               block.hash
                |> Chain.block_to_transactions(paging_options: %PagingOptions{key: {index}, page_size: 50})
                |> Enum.map(& &1.hash)
                |> Enum.reverse()
@@ -683,7 +683,7 @@ defmodule Explorer.ChainTest do
         token: token
       )
 
-      fetched_transaction = List.first(Explorer.Chain.block_to_transactions(block))
+      fetched_transaction = List.first(Explorer.Chain.block_to_transactions(block.hash))
       assert fetched_transaction.hash == transaction.hash
       assert length(fetched_transaction.token_transfers) == 2
     end
@@ -693,7 +693,7 @@ defmodule Explorer.ChainTest do
     test "without transactions" do
       block = insert(:block)
 
-      assert Chain.block_to_transaction_count(block) == 0
+      assert Chain.block_to_transaction_count(block.hash) == 0
     end
 
     test "with transactions" do
@@ -702,7 +702,7 @@ defmodule Explorer.ChainTest do
         |> insert()
         |> with_block()
 
-      assert Chain.block_to_transaction_count(block) == 1
+      assert Chain.block_to_transaction_count(block.hash) == 1
     end
   end
 
@@ -2090,7 +2090,7 @@ defmodule Explorer.ChainTest do
     test "with transaction without internal transactions" do
       transaction = insert(:transaction)
 
-      assert [] = Chain.transaction_to_internal_transactions(transaction)
+      assert [] = Chain.transaction_to_internal_transactions(transaction.hash)
     end
 
     test "with transaction with internal transactions returns all internal transactions for a given transaction hash" do
@@ -2117,7 +2117,7 @@ defmodule Explorer.ChainTest do
           transaction_index: transaction.index
         )
 
-      results = [internal_transaction | _] = Chain.transaction_to_internal_transactions(transaction)
+      results = [internal_transaction | _] = Chain.transaction_to_internal_transactions(transaction.hash)
 
       assert 2 == length(results)
 
@@ -2151,7 +2151,7 @@ defmodule Explorer.ChainTest do
                  to_address: %Ecto.Association.NotLoaded{},
                  transaction: %Transaction{block: %Ecto.Association.NotLoaded{}}
                }
-             ] = Chain.transaction_to_internal_transactions(transaction)
+             ] = Chain.transaction_to_internal_transactions(transaction.hash)
 
       assert [
                %InternalTransaction{
@@ -2161,7 +2161,7 @@ defmodule Explorer.ChainTest do
                }
              ] =
                Chain.transaction_to_internal_transactions(
-                 transaction,
+                 transaction.hash,
                  necessity_by_association: %{
                    :from_address => :optional,
                    :to_address => :optional,
@@ -2183,7 +2183,7 @@ defmodule Explorer.ChainTest do
         transaction_index: transaction.index
       )
 
-      result = Chain.transaction_to_internal_transactions(transaction)
+      result = Chain.transaction_to_internal_transactions(transaction.hash)
 
       assert Enum.empty?(result)
     end
@@ -2202,7 +2202,7 @@ defmodule Explorer.ChainTest do
           transaction_index: transaction.index
         )
 
-      actual = Enum.at(Chain.transaction_to_internal_transactions(transaction), 0)
+      actual = Enum.at(Chain.transaction_to_internal_transactions(transaction.hash), 0)
 
       assert {actual.transaction_hash, actual.index} == {expected.transaction_hash, expected.index}
     end
@@ -2222,7 +2222,7 @@ defmodule Explorer.ChainTest do
           transaction_index: transaction.index
         )
 
-      actual = Enum.at(Chain.transaction_to_internal_transactions(transaction), 0)
+      actual = Enum.at(Chain.transaction_to_internal_transactions(transaction.hash), 0)
 
       assert {actual.transaction_hash, actual.index} == {expected.transaction_hash, expected.index}
     end
@@ -2243,7 +2243,7 @@ defmodule Explorer.ChainTest do
           transaction_index: transaction.index
         )
 
-      actual = Enum.at(Chain.transaction_to_internal_transactions(transaction), 0)
+      actual = Enum.at(Chain.transaction_to_internal_transactions(transaction.hash), 0)
 
       assert {actual.transaction_hash, actual.index} == {expected.transaction_hash, expected.index}
     end
@@ -2271,7 +2271,7 @@ defmodule Explorer.ChainTest do
         )
 
       result =
-        transaction
+        transaction.hash
         |> Chain.transaction_to_internal_transactions()
         |> Enum.map(&{&1.transaction_hash, &1.index})
 
@@ -2301,17 +2301,17 @@ defmodule Explorer.ChainTest do
         )
 
       assert [{first_transaction_hash, first_index}, {second_transaction_hash, second_index}] ==
-               transaction
+               transaction.hash
                |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {-1}, page_size: 2})
                |> Enum.map(&{&1.transaction_hash, &1.index})
 
       assert [{first_transaction_hash, first_index}] ==
-               transaction
+               transaction.hash
                |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {-1}, page_size: 1})
                |> Enum.map(&{&1.transaction_hash, &1.index})
 
       assert [{second_transaction_hash, second_index}] ==
-               transaction
+               transaction.hash
                |> Chain.transaction_to_internal_transactions(paging_options: %PagingOptions{key: {0}, page_size: 2})
                |> Enum.map(&{&1.transaction_hash, &1.index})
     end
@@ -2321,7 +2321,7 @@ defmodule Explorer.ChainTest do
     test "without logs" do
       transaction = insert(:transaction)
 
-      assert [] = Chain.transaction_to_logs(transaction)
+      assert [] = Chain.transaction_to_logs(transaction.hash)
     end
 
     test "with logs" do
@@ -2332,7 +2332,7 @@ defmodule Explorer.ChainTest do
 
       %Log{transaction_hash: transaction_hash, index: index} = insert(:log, transaction: transaction)
 
-      assert [%Log{transaction_hash: ^transaction_hash, index: ^index}] = Chain.transaction_to_logs(transaction)
+      assert [%Log{transaction_hash: ^transaction_hash, index: ^index}] = Chain.transaction_to_logs(transaction.hash)
     end
 
     test "with logs can be paginated" do
@@ -2349,7 +2349,7 @@ defmodule Explorer.ChainTest do
         |> Enum.map(& &1.index)
 
       assert second_page_indexes ==
-               transaction
+               transaction.hash
                |> Chain.transaction_to_logs(paging_options: %PagingOptions{key: {log.index}, page_size: 50})
                |> Enum.map(& &1.index)
     end
@@ -2364,7 +2364,7 @@ defmodule Explorer.ChainTest do
 
       assert [%Log{address: %Address{}, transaction: %Transaction{}}] =
                Chain.transaction_to_logs(
-                 transaction,
+                 transaction.hash,
                  necessity_by_association: %{
                    address: :optional,
                    transaction: :optional
@@ -2376,7 +2376,7 @@ defmodule Explorer.ChainTest do
                  address: %Ecto.Association.NotLoaded{},
                  transaction: %Ecto.Association.NotLoaded{}
                }
-             ] = Chain.transaction_to_logs(transaction)
+             ] = Chain.transaction_to_logs(transaction.hash)
     end
   end
 
@@ -2384,7 +2384,7 @@ defmodule Explorer.ChainTest do
     test "without token transfers" do
       transaction = insert(:transaction)
 
-      assert [] = Chain.transaction_to_token_transfers(transaction)
+      assert [] = Chain.transaction_to_token_transfers(transaction.hash)
     end
 
     test "with token transfers" do
@@ -2397,7 +2397,7 @@ defmodule Explorer.ChainTest do
         insert(:token_transfer, transaction: transaction)
 
       assert [%TokenTransfer{transaction_hash: ^transaction_hash, log_index: ^log_index}] =
-               Chain.transaction_to_token_transfers(transaction)
+               Chain.transaction_to_token_transfers(transaction.hash)
     end
 
     test "token transfers necessity_by_association loads associations" do
@@ -2410,7 +2410,7 @@ defmodule Explorer.ChainTest do
 
       assert [%TokenTransfer{token: %Token{}, transaction: %Transaction{}}] =
                Chain.transaction_to_token_transfers(
-                 transaction,
+                 transaction.hash,
                  necessity_by_association: %{
                    token: :optional,
                    transaction: :optional
@@ -2422,7 +2422,7 @@ defmodule Explorer.ChainTest do
                  token: %Ecto.Association.NotLoaded{},
                  transaction: %Ecto.Association.NotLoaded{}
                }
-             ] = Chain.transaction_to_token_transfers(transaction)
+             ] = Chain.transaction_to_token_transfers(transaction.hash)
     end
   end
 
@@ -2533,7 +2533,7 @@ defmodule Explorer.ChainTest do
         |> Decimal.add(Decimal.new(3))
         |> Wei.from(:wei)
 
-      assert expected == Chain.block_reward(block)
+      assert expected == Chain.block_reward(block.number)
     end
 
     test "with block without transactions", %{block: block, emission_reward: emission_reward} do

From 2a7cfead5a43cd9d344015bd25901a99fd18d181 Mon Sep 17 00:00:00 2001
From: pasqu4le <pasqu4le@gmail.com>
Date: Fri, 19 Jul 2019 17:28:06 +0200
Subject: [PATCH 3/4] Check only for existence when possible

Problem: some controller retrieves entire entities from the database when it only needs to check their existence.

Solution: provide Exploerer.Chain functions to check for existence and use them where possible.
---
 .../address_coin_balance_controller.ex        |   6 +-
 .../controllers/address_logs_controller.ex    |   8 +-
 .../decompiled_smart_contract_controller.ex   |  21 +-
 .../v1/verified_smart_contract_controller.ex  |  21 +-
 .../controllers/smart_contract_controller.ex  |   4 +-
 .../controllers/transaction_controller.ex     |   6 +-
 ...saction_internal_transaction_controller.ex |   6 +-
 .../transaction_token_transfer_controller.ex  |   7 +-
 apps/explorer/lib/explorer/chain.ex           | 199 ++++++++++++++++++
 9 files changed, 224 insertions(+), 54 deletions(-)

diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/address_coin_balance_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/address_coin_balance_controller.ex
index 33a01959c9..9267c0dcab 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/address_coin_balance_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/address_coin_balance_controller.ex
@@ -16,7 +16,7 @@ defmodule BlockScoutWeb.AddressCoinBalanceController do
 
   def index(conn, %{"address_id" => address_hash_string, "type" => "JSON"} = params) do
     with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string),
-         {:ok, address} <- Chain.hash_to_address(address_hash, [], false) do
+         :ok <- Chain.check_address_exists(address_hash) do
       full_options = paging_options(params)
 
       coin_balances_plus_one = Chain.address_to_coin_balances(address_hash, full_options)
@@ -32,7 +32,7 @@ defmodule BlockScoutWeb.AddressCoinBalanceController do
             address_coin_balance_path(
               conn,
               :index,
-              address,
+              address_hash,
               Map.delete(next_page_params, "type")
             )
         end
@@ -52,7 +52,7 @@ defmodule BlockScoutWeb.AddressCoinBalanceController do
       :error ->
         unprocessable_entity(conn)
 
-      {:error, :not_found} ->
+      :not_found ->
         not_found(conn)
     end
   end
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/address_logs_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/address_logs_controller.ex
index e349ecd9ef..9e05643187 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/address_logs_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/address_logs_controller.ex
@@ -16,7 +16,7 @@ defmodule BlockScoutWeb.AddressLogsController do
 
   def index(conn, %{"address_id" => address_hash_string, "type" => "JSON"} = params) do
     with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string),
-         {:ok, address} <- Chain.hash_to_address(address_hash, [], false) do
+         :ok <- Chain.check_address_exists(address_hash) do
       logs_plus_one = Chain.address_to_logs(address_hash, paging_options(params))
       {results, next_page} = split_list_by_page(logs_plus_one)
 
@@ -26,7 +26,7 @@ defmodule BlockScoutWeb.AddressLogsController do
             nil
 
           next_page_params ->
-            address_logs_path(conn, :index, address, Map.delete(next_page_params, "type"))
+            address_logs_path(conn, :index, address_hash, Map.delete(next_page_params, "type"))
         end
 
       items =
@@ -74,7 +74,7 @@ defmodule BlockScoutWeb.AddressLogsController do
 
   def search_logs(conn, %{"topic" => topic, "address_id" => address_hash_string} = params) do
     with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string),
-         {:ok, address} <- Chain.hash_to_address(address_hash, [], false) do
+         :ok <- Chain.check_address_exists(address_hash) do
       topic = String.trim(topic)
 
       formatted_topic = if String.starts_with?(topic, "0x"), do: topic, else: "0x" <> topic
@@ -89,7 +89,7 @@ defmodule BlockScoutWeb.AddressLogsController do
             nil
 
           next_page_params ->
-            address_logs_path(conn, :index, address, Map.delete(next_page_params, "type"))
+            address_logs_path(conn, :index, address_hash, Map.delete(next_page_params, "type"))
         end
 
       items =
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/decompiled_smart_contract_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/decompiled_smart_contract_controller.ex
index 1eec849380..7b783b9761 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/decompiled_smart_contract_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/decompiled_smart_contract_controller.ex
@@ -7,8 +7,9 @@ defmodule BlockScoutWeb.API.V1.DecompiledSmartContractController do
   def create(conn, params) do
     if auth_token(conn) == actual_token() do
       with {:ok, hash} <- validate_address_hash(params["address_hash"]),
-           :ok <- smart_contract_exists?(hash),
-           :ok <- decompiled_contract_exists?(params["address_hash"], params["decompiler_version"]) do
+           :ok <- Chain.check_address_exists(hash),
+           {:contract, :not_found} <-
+             {:contract, Chain.check_decompiled_contract_exists(params["address_hash"], params["decompiler_version"])} do
         case Chain.create_decompiled_smart_contract(params) do
           {:ok, decompiled_smart_contract} ->
             send_resp(conn, :created, Jason.encode!(decompiled_smart_contract))
@@ -29,7 +30,7 @@ defmodule BlockScoutWeb.API.V1.DecompiledSmartContractController do
         :not_found ->
           send_resp(conn, :unprocessable_entity, encode(%{error: "address is not found"}))
 
-        :contract_exists ->
+        {:contract, :ok} ->
           send_resp(
             conn,
             :unprocessable_entity,
@@ -41,13 +42,6 @@ defmodule BlockScoutWeb.API.V1.DecompiledSmartContractController do
     end
   end
 
-  defp smart_contract_exists?(address_hash) do
-    case Chain.hash_to_address(address_hash) do
-      {:ok, _address} -> :ok
-      _ -> :not_found
-    end
-  end
-
   defp validate_address_hash(address_hash) do
     case Address.cast(address_hash) do
       {:ok, hash} -> {:ok, hash}
@@ -55,13 +49,6 @@ defmodule BlockScoutWeb.API.V1.DecompiledSmartContractController do
     end
   end
 
-  defp decompiled_contract_exists?(address_hash, decompiler_version) do
-    case Chain.decompiled_code(address_hash, decompiler_version) do
-      {:ok, _} -> :contract_exists
-      _ -> :ok
-    end
-  end
-
   defp auth_token(conn) do
     case get_req_header(conn, "auth_token") do
       [token] -> token
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/verified_smart_contract_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/verified_smart_contract_controller.ex
index 50334a1a45..8b3d3d71eb 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/verified_smart_contract_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/api/v1/verified_smart_contract_controller.ex
@@ -7,8 +7,8 @@ defmodule BlockScoutWeb.API.V1.VerifiedSmartContractController do
 
   def create(conn, params) do
     with {:ok, hash} <- validate_address_hash(params["address_hash"]),
-         :ok <- smart_contract_exists?(hash),
-         :ok <- verified_smart_contract_exists?(hash) do
+         :ok <- Chain.check_address_exists(hash),
+         {:contract, :not_found} <- {:contract, Chain.check_verified_smart_contract_exists(hash)} do
       external_libraries = fetch_external_libraries(params)
 
       case Publisher.publish(hash, params, external_libraries) do
@@ -31,7 +31,7 @@ defmodule BlockScoutWeb.API.V1.VerifiedSmartContractController do
       :not_found ->
         send_resp(conn, :unprocessable_entity, encode(%{error: "address is not found"}))
 
-      :contract_exists ->
+      {:contract, :ok} ->
         send_resp(
           conn,
           :unprocessable_entity,
@@ -40,13 +40,6 @@ defmodule BlockScoutWeb.API.V1.VerifiedSmartContractController do
     end
   end
 
-  defp smart_contract_exists?(address_hash) do
-    case Chain.hash_to_address(address_hash) do
-      {:ok, _address} -> :ok
-      _ -> :not_found
-    end
-  end
-
   defp validate_address_hash(address_hash) do
     case Address.cast(address_hash) do
       {:ok, hash} -> {:ok, hash}
@@ -54,14 +47,6 @@ defmodule BlockScoutWeb.API.V1.VerifiedSmartContractController do
     end
   end
 
-  defp verified_smart_contract_exists?(address_hash) do
-    if Chain.address_hash_to_smart_contract(address_hash) do
-      :contract_exists
-    else
-      :ok
-    end
-  end
-
   defp encode(data) do
     Jason.encode!(data)
   end
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/smart_contract_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/smart_contract_controller.ex
index 70ec6219f3..311feb4591 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/smart_contract_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/smart_contract_controller.ex
@@ -33,7 +33,7 @@ defmodule BlockScoutWeb.SmartContractController do
   def show(conn, params) do
     with true <- ajax?(conn),
          {:ok, address_hash} <- Chain.string_to_address_hash(params["id"]),
-         {:ok, _address} <- Chain.find_contract_address(address_hash),
+         :ok <- Chain.check_contract_address_exists(address_hash),
          outputs =
            Reader.query_function(
              address_hash,
@@ -51,7 +51,7 @@ defmodule BlockScoutWeb.SmartContractController do
       :error ->
         unprocessable_entity(conn)
 
-      {:error, :not_found} ->
+      :not_found ->
         not_found(conn)
 
       _ ->
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 1bedf97c1d..3f849c0e7c 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
@@ -62,15 +62,15 @@ defmodule BlockScoutWeb.TransactionController do
 
   def show(conn, %{"id" => id}) do
     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
+         :ok <- Chain.check_transaction_exists(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
       :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)
+      :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 ae847b374d..ca154a0136 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
@@ -10,7 +10,7 @@ defmodule BlockScoutWeb.TransactionInternalTransactionController do
 
   def index(conn, %{"transaction_id" => hash_string, "type" => "JSON"} = params) do
     with {:ok, hash} <- Chain.string_to_transaction_hash(hash_string),
-         {:ok, transaction} <- Chain.hash_to_transaction(hash) do
+         :ok <- Chain.check_transaction_exists(hash) do
       full_options =
         Keyword.merge(
           [
@@ -37,7 +37,7 @@ defmodule BlockScoutWeb.TransactionInternalTransactionController do
             transaction_internal_transaction_path(
               conn,
               :index,
-              transaction,
+              hash,
               Map.delete(next_page_params, "type")
             )
         end
@@ -66,7 +66,7 @@ defmodule BlockScoutWeb.TransactionInternalTransactionController do
         |> put_view(TransactionView)
         |> render("invalid.html", transaction_hash: hash_string)
 
-      {:error, :not_found} ->
+      :not_found ->
         conn
         |> put_status(404)
         |> put_view(TransactionView)
diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex
index 5f4a9c8229..585a01301f 100644
--- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex
+++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_token_transfer_controller.ex
@@ -10,8 +10,7 @@ defmodule BlockScoutWeb.TransactionTokenTransferController do
 
   def index(conn, %{"transaction_id" => hash_string, "type" => "JSON"} = params) do
     with {:ok, hash} <- Chain.string_to_transaction_hash(hash_string),
-         {:ok, transaction} <-
-           Chain.hash_to_transaction(hash) do
+         :ok <- Chain.check_transaction_exists(hash) do
       full_options =
         Keyword.merge(
           [
@@ -34,7 +33,7 @@ defmodule BlockScoutWeb.TransactionTokenTransferController do
             nil
 
           next_page_params ->
-            transaction_token_transfer_path(conn, :index, transaction, Map.delete(next_page_params, "type"))
+            transaction_token_transfer_path(conn, :index, hash, Map.delete(next_page_params, "type"))
         end
 
       items =
@@ -62,7 +61,7 @@ defmodule BlockScoutWeb.TransactionTokenTransferController do
         |> put_view(TransactionView)
         |> render("invalid.html", transaction_hash: hash_string)
 
-      {:error, :not_found} ->
+      :not_found ->
         conn
         |> put_status(404)
         |> put_view(TransactionView)
diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex
index 2c40f6bbc1..0e01212c37 100644
--- a/apps/explorer/lib/explorer/chain.ex
+++ b/apps/explorer/lib/explorer/chain.ex
@@ -3297,4 +3297,203 @@ defmodule Explorer.Chain do
     |> Base.decode16!(case: :mixed)
     |> TypeDecoder.decode_raw(types)
   end
+
+  @doc """
+  Checks if an `t:Explorer.Chain.Address.t/0` with the given `hash` exists.
+
+  Returns `:ok` if found
+
+      iex> {:ok, %Explorer.Chain.Address{hash: hash}} = Explorer.Chain.create_address(
+      ...>   %{hash: "0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed"}
+      ...> )
+      iex> Explorer.Chain.check_address_exists(hash)
+      :ok
+
+  Returns `:not_found` if not found
+
+      iex> {:ok, hash} = Explorer.Chain.string_to_address_hash("0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed")
+      iex> Explorer.Chain.check_address_exists(hash)
+      :not_found
+
+  """
+  @spec check_address_exists(Hash.Address.t()) :: :ok | :not_found
+  def check_address_exists(address_hash) do
+    address_hash
+    |> address_exists?()
+    |> boolean_to_check_result()
+  end
+
+  @doc """
+  Checks if an `t:Explorer.Chain.Address.t/0` with the given `hash` exists.
+
+  Returns `true` if found
+
+      iex> {:ok, %Explorer.Chain.Address{hash: hash}} = Explorer.Chain.create_address(
+      ...>   %{hash: "0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed"}
+      ...> )
+      iex> Explorer.Chain.address_exists?(hash)
+      true
+
+  Returns `false` if not found
+
+      iex> {:ok, hash} = Explorer.Chain.string_to_address_hash("0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed")
+      iex> Explorer.Chain.address_exists?(hash)
+      false
+
+  """
+  @spec address_exists?(Hash.Address.t()) :: boolean()
+  def address_exists?(address_hash) do
+    query =
+      from(
+        address in Address,
+        where: address.hash == ^address_hash
+      )
+
+    Repo.exists?(query)
+  end
+
+  @doc """
+  Checks if it exists an `t:Explorer.Chain.Address.t/0` that has the provided
+  `t:Explorer.Chain.Address.t/0` `hash` and a contract.
+
+  Returns `:ok` if found and `:not_found` otherwise.
+  """
+  @spec check_contract_address_exists(Hash.Address.t()) :: :ok | :not_found
+  def check_contract_address_exists(address_hash) do
+    address_hash
+    |> contract_address_exists?()
+    |> boolean_to_check_result()
+  end
+
+  @doc """
+  Checks if it exists an `t:Explorer.Chain.Address.t/0` that has the provided
+  `t:Explorer.Chain.Address.t/0` `hash` and a contract.
+
+  Returns `true` if found and `false` otherwise.
+  """
+  @spec contract_address_exists?(Hash.Address.t()) :: boolean()
+  def contract_address_exists?(address_hash) do
+    query =
+      from(
+        address in Address,
+        where: address.hash == ^address_hash and not is_nil(address.contract_code)
+      )
+
+    Repo.exists?(query)
+  end
+
+  @doc """
+  Checks if it exists a `t:Explorer.Chain.DecompiledSmartContract.t/0` for the
+  `t:Explorer.Chain.Address.t/0` with the provided `hash` and with the provided version.
+
+  Returns `:ok` if found and `:not_found` otherwise.
+  """
+  @spec check_decompiled_contract_exists(Hash.Address.t(), String.t()) :: :ok | :not_found
+  def check_decompiled_contract_exists(address_hash, version) do
+    address_hash
+    |> decompiled_contract_exists?(version)
+    |> boolean_to_check_result()
+  end
+
+  @doc """
+  Checks if it exists a `t:Explorer.Chain.DecompiledSmartContract.t/0` for the
+  `t:Explorer.Chain.Address.t/0` with the provided `hash` and with the provided version.
+
+  Returns `true` if found and `false` otherwise.
+  """
+  @spec decompiled_contract_exists?(Hash.Address.t(), String.t()) :: boolean()
+  def decompiled_contract_exists?(address_hash, version) do
+    query =
+      from(contract in DecompiledSmartContract,
+        where: contract.address_hash == ^address_hash and contract.decompiler_version == ^version
+      )
+
+    Repo.exists?(query)
+  end
+
+  @doc """
+  Checks if it exists a verified `t:Explorer.Chain.SmartContract.t/0` for the
+  `t:Explorer.Chain.Address.t/0` with the provided `hash`.
+
+  Returns `:ok` if found and `:not_found` otherwise.
+  """
+  @spec check_verified_smart_contract_exists(Hash.Address.t()) :: :ok | :not_found
+  def check_verified_smart_contract_exists(address_hash) do
+    address_hash
+    |> verified_smart_contract_exists?()
+    |> boolean_to_check_result()
+  end
+
+  @doc """
+  Checks if it exists a verified `t:Explorer.Chain.SmartContract.t/0` for the
+  `t:Explorer.Chain.Address.t/0` with the provided `hash`.
+
+  Returns `true` if found and `false` otherwise.
+  """
+  @spec verified_smart_contract_exists?(Hash.Address.t()) :: boolean()
+  def verified_smart_contract_exists?(address_hash) do
+    query =
+      from(
+        smart_contract in SmartContract,
+        where: smart_contract.address_hash == ^address_hash
+      )
+
+    Repo.exists?(query)
+  end
+
+  @doc """
+  Checks if a `t:Explorer.Chain.Transaction.t/0` with the given `hash` exists.
+
+  Returns `:ok` if found
+
+      iex> %Transaction{hash: hash} = insert(:transaction)
+      iex> Explorer.Chain.check_transaction_exists(hash)
+      :ok
+
+  Returns `:not_found` if not found
+
+      iex> {:ok, hash} = Explorer.Chain.string_to_transaction_hash(
+      ...>   "0x9fc76417374aa880d4449a1f7f31ec597f00b1f6f3dd2d66f4c9c6c445836d8b"
+      ...> )
+      iex> Explorer.Chain.check_transaction_exists(hash)
+      :not_found
+  """
+  @spec check_transaction_exists(Hash.Full.t()) :: :ok | :not_found
+  def check_transaction_exists(hash) do
+    hash
+    |> transaction_exists?()
+    |> boolean_to_check_result()
+  end
+
+  @doc """
+  Checks if a `t:Explorer.Chain.Transaction.t/0` with the given `hash` exists.
+
+  Returns `true` if found
+
+      iex> %Transaction{hash: hash} = insert(:transaction)
+      iex> Explorer.Chain.transaction_exists?(hash)
+      true
+
+  Returns `false` if not found
+
+      iex> {:ok, hash} = Explorer.Chain.string_to_transaction_hash(
+      ...>   "0x9fc76417374aa880d4449a1f7f31ec597f00b1f6f3dd2d66f4c9c6c445836d8b"
+      ...> )
+      iex> Explorer.Chain.transaction_exists?(hash)
+      false
+  """
+  @spec transaction_exists?(Hash.Full.t()) :: boolean()
+  def transaction_exists?(hash) do
+    query =
+      from(
+        transaction in Transaction,
+        where: transaction.hash == ^hash
+      )
+
+    Repo.exists?(query)
+  end
+
+  defp boolean_to_check_result(true), do: :ok
+
+  defp boolean_to_check_result(false), do: :not_found
 end

From 4a46e6eec592e0b1aed26baa9cb3bcaa60e454cc Mon Sep 17 00:00:00 2001
From: pasqu4le <pasqu4le@gmail.com>
Date: Fri, 19 Jul 2019 17:58:47 +0200
Subject: [PATCH 4/4] Update CHANGELOG

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b9c8934f3d..c295387ba5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## Current
 
 ### Features
+- [#2391](https://github.com/poanetwork/blockscout/pull/2391) - Controllers Improvements
 - [#2379](https://github.com/poanetwork/blockscout/pull/2379) - Disable network selector when is empty
 - [#2360](https://github.com/poanetwork/blockscout/pull/2360) - add default evm version to smart contract verification
 - [#2352](https://github.com/poanetwork/blockscout/pull/2352) - Fetch rewards in parallel with transactions