Merge pull request #1293 from poanetwork/1287

Don't fetch or import internal transactions for successful value transfers

apps/block_scout_web/lib/block_scout_web/notifier.ex

@@ -69,8 +69,9 @@ defmodule BlockScoutWeb.Notifier do
     end
   end
 
-  def handle_event({:chain_event, :transactions, :realtime, transaction_hashes}) do
+  def handle_event({:chain_event, :transactions, :realtime, transactions}) do
-    transaction_hashes
+    transactions
+    |> Enum.map(& &1.hash)
     |> Chain.hashes_to_transactions(
       necessity_by_association: %{
         :block => :optional,

apps/block_scout_web/test/block_scout_web/channels/address_channel_test.exs

@@ -54,7 +54,7 @@ defmodule BlockScoutWeb.AddressChannelTest do
     test "notified of new_pending_transaction for matching from_address", %{address: address, topic: topic} do
       pending = insert(:transaction, from_address: address)
 
-      Notifier.handle_event({:chain_event, :transactions, :realtime, [pending.hash]})
+      Notifier.handle_event({:chain_event, :transactions, :realtime, [pending]})
 
       assert_receive %Phoenix.Socket.Broadcast{topic: ^topic, event: "pending_transaction", payload: payload},
                      :timer.seconds(5)
@@ -69,7 +69,7 @@ defmodule BlockScoutWeb.AddressChannelTest do
         |> insert(from_address: address)
         |> with_block()
 
-      Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction.hash]})
+      Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction]})
 
       assert_receive %Phoenix.Socket.Broadcast{topic: ^topic, event: "transaction", payload: payload}, :timer.seconds(5)
       assert payload.address.hash == address.hash
@@ -82,7 +82,7 @@ defmodule BlockScoutWeb.AddressChannelTest do
         |> insert(to_address: address)
         |> with_block()
 
-      Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction.hash]})
+      Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction]})
 
       assert_receive %Phoenix.Socket.Broadcast{topic: ^topic, event: "transaction", payload: payload}, :timer.seconds(5)
       assert payload.address.hash == address.hash
@@ -95,7 +95,7 @@ defmodule BlockScoutWeb.AddressChannelTest do
         |> insert(from_address: address, to_address: address)
         |> with_block()
 
-      Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction.hash]})
+      Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction]})
 
       assert_receive %Phoenix.Socket.Broadcast{topic: ^topic, event: "transaction", payload: payload}, :timer.seconds(5)
       assert payload.address.hash == address.hash

apps/block_scout_web/test/block_scout_web/channels/transaction_channel_test.exs

@@ -13,7 +13,7 @@ defmodule BlockScoutWeb.TransactionChannelTest do
       |> insert()
       |> with_block()
 
-    Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction.hash]})
+    Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction]})
 
     receive do
       %Phoenix.Socket.Broadcast{topic: ^topic, event: "transaction", payload: payload} ->
@@ -30,7 +30,7 @@ defmodule BlockScoutWeb.TransactionChannelTest do
 
     pending = insert(:transaction)
 
-    Notifier.handle_event({:chain_event, :transactions, :realtime, [pending.hash]})
+    Notifier.handle_event({:chain_event, :transactions, :realtime, [pending]})
 
     receive do
       %Phoenix.Socket.Broadcast{topic: ^topic, event: "pending_transaction", payload: payload} ->
@@ -50,7 +50,7 @@ defmodule BlockScoutWeb.TransactionChannelTest do
     topic = "transactions:#{Hash.to_string(transaction.hash)}"
     @endpoint.subscribe(topic)
 
-    Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction.hash]})
+    Notifier.handle_event({:chain_event, :transactions, :realtime, [transaction]})
 
     receive do
       %Phoenix.Socket.Broadcast{topic: ^topic, event: "collated", payload: %{}} ->

apps/explorer/lib/explorer/chain/import/runner/transactions.ex

@@ -8,7 +8,7 @@ defmodule Explorer.Chain.Import.Runner.Transactions do
   import Ecto.Query, only: [from: 2]
 
   alias Ecto.{Multi, Repo}
-  alias Explorer.Chain.{Hash, Import, Transaction}
+  alias Explorer.Chain.{Data, Hash, Import, Transaction}
 
   @behaviour Import.Runner
 
@@ -53,26 +53,26 @@ defmodule Explorer.Chain.Import.Runner.Transactions do
           required(:timeout) => timeout,
           required(:timestamps) => Import.timestamps()
         }) :: {:ok, [Hash.t()]}
-  defp insert(repo, changes_list, %{timeout: timeout, timestamps: timestamps} = options)
+  defp insert(repo, changes_list, %{timeout: timeout, timestamps: %{inserted_at: inserted_at} = timestamps} = options)
        when is_list(changes_list) do
     on_conflict = Map.get_lazy(options, :on_conflict, &default_on_conflict/0)
 
-    # order so that row ShareLocks are grabbed in a consistent order
+    ordered_changes_list =
-    ordered_changes_list = Enum.sort_by(changes_list, & &1.hash)
+      changes_list
-
+      |> timestamp_ok_value_transfers(inserted_at)
-    {:ok, transactions} =
+      # order so that row ShareLocks are grabbed in a consistent order
-      Import.insert_changes_list(
+      |> Enum.sort_by(& &1.hash)
-        repo,
+
-        ordered_changes_list,
+    Import.insert_changes_list(
-        conflict_target: :hash,
+      repo,
-        on_conflict: on_conflict,
+      ordered_changes_list,
-        for: Transaction,
+      conflict_target: :hash,
-        returning: [:hash],
+      on_conflict: on_conflict,
-        timeout: timeout,
+      for: Transaction,
-        timestamps: timestamps
+      returning: ~w(block_number index hash internal_transactions_indexed_at)a,
-      )
+      timeout: timeout,
-
+      timestamps: timestamps
-    {:ok, for(transaction <- transactions, do: transaction.hash)}
+    )
   end
 
   defp default_on_conflict do
@@ -129,4 +129,16 @@ defmodule Explorer.Chain.Import.Runner.Transactions do
         )
     )
   end
+
+  defp timestamp_ok_value_transfers(changes_list, timestamp) when is_list(changes_list) do
+    Enum.map(changes_list, &timestamp_ok_value_transfer(&1, timestamp))
+  end
+
+  # A post-Byzantium validated transaction will have a status and if it has no input, it is a value transfer only.
+  # Internal transactions are only needed when status is `:error` to set `error`.
+  defp timestamp_ok_value_transfer(%{status: :ok, input: %Data{bytes: <<>>}} = changes, timestamp) do
+    Map.put(changes, :internal_transactions_indexed_at, timestamp)
+  end
+
+  defp timestamp_ok_value_transfer(changes, _), do: changes
 end

apps/explorer/lib/explorer/chain/transaction.ex

@@ -96,7 +96,17 @@ defmodule Explorer.Chain.Transaction do
    * `input`- data sent along with the transaction
    * `internal_transactions` - transactions (value transfers) created while executing contract used for this
      transaction
-   * `internal_transactions_indexed_at` - when `internal_transactions` were fetched by `Indexer`.
+   * `internal_transactions_indexed_at` - when `internal_transactions` were fetched by `Indexer` or when they do not
+     need to be fetched at `inserted_at`.
+
+     | `status` | `input`    | `internal_transactions_indexed_at`        | `internal_transactions` | Description                                                                             |
+     |----------|------------|-------------------------------------------|-------------------------|-----------------------------------------------------------------------------------------|
+     | `:ok`    | Empty      | `inserted_at`                             | Unfetched               | Simple `value` transfer succeeded.  Internal transactions would be same value transfer. |
+     | `:ok`    | Non-Empty  | When `internal_transactions` are indexed. | Fetched                 | A contract call that succeeded.                                                         |
+     | `:error` | Empty      | When `internal_transactions` are indexed. | Fetched                 | Simple `value` transfer failed. Internal transactions fetched for `error`.              |
+     | `:error` | Non-Empty  | When `internal_transactions` are indexed. | Fetched                 | A contract call that failed.                                                            |
+     | `nil`    | Don't Care | When `internal_transactions` are indexed. | Depends                 | A pending post-Byzantium transaction will only know its status from  receipt.           |
+     | `nil`    | Don't Care | When `internal_transactions` are indexed. | Fetched                 | A pre-Byzantium transaction requires internal transactions to determine status          |
    * `logs` - events that occurred while mining the `transaction`.
    * `nonce` - the number of transaction made by the sender prior to this one
    * `r` - the R field of the signature. The (r, s) is the normal output of an ECDSA signature, where r is computed as

apps/explorer/test/explorer/chain/import_test.exs

@@ -297,11 +297,16 @@ defmodule Explorer.Chain.ImportTest do
                   }
                 ],
                 transactions: [
-                  %Hash{
+                  %Transaction{
-                    byte_count: 32,
+                    block_number: 37,
-                    bytes:
+                    index: 0,
-                      <<83, 189, 136, 72, 114, 222, 62, 72, 134, 146, 136, 27, 174, 236, 38, 46, 123, 149, 35, 77, 57,
+                    hash: %Hash{
-                        101, 36, 140, 57, 254, 153, 47, 255, 212, 51, 229>>
+                      byte_count: 32,
+                      bytes:
+                        <<83, 189, 136, 72, 114, 222, 62, 72, 134, 146, 136, 27, 174, 236, 38, 46, 123, 149, 35, 77, 57,
+                          101, 36, 140, 57, 254, 153, 47, 255, 212, 51, 229>>
+                    },
+                    internal_transactions_indexed_at: nil
                   }
                 ],
                 tokens: [
@@ -481,10 +486,10 @@ defmodule Explorer.Chain.ImportTest do
                        [%{transaction_hash: _, index: _}, %{transaction_hash: _, index: _}]}
     end
 
-    test "publishes transaction hashes data to subscribers on insert" do
+    test "publishes transactions data to subscribers on insert" do
       Subscriber.to(:transactions, :realtime)
       Import.all(@import_data)
-      assert_received {:chain_event, :transactions, :realtime, [%Hash{}]}
+      assert_received {:chain_event, :transactions, :realtime, [%Transaction{}]}
     end
 
     test "publishes token_transfers data to subscribers on insert" do

apps/explorer/test/explorer/chain_test.exs

@@ -1186,11 +1186,16 @@ defmodule Explorer.ChainTest do
                   }
                 ],
                 transactions: [
-                  %Hash{
+                  %Transaction{
-                    byte_count: 32,
+                    block_number: 37,
-                    bytes:
+                    index: 0,
-                      <<83, 189, 136, 72, 114, 222, 62, 72, 134, 146, 136, 27, 174, 236, 38, 46, 123, 149, 35, 77, 57,
+                    hash: %Hash{
-                        101, 36, 140, 57, 254, 153, 47, 255, 212, 51, 229>>
+                      byte_count: 32,
+                      bytes:
+                        <<83, 189, 136, 72, 114, 222, 62, 72, 134, 146, 136, 27, 174, 236, 38, 46, 123, 149, 35, 77, 57,
+                          101, 36, 140, 57, 254, 153, 47, 255, 212, 51, 229>>
+                    },
+                    internal_transactions_indexed_at: nil
                   }
                 ],
                 tokens: [

apps/indexer/lib/indexer/block/catchup/fetcher.ex

@@ -12,6 +12,7 @@ defmodule Indexer.Block.Catchup.Fetcher do
 
   alias Ecto.Changeset
   alias Explorer.Chain
+  alias Explorer.Chain.Transaction
   alias Indexer.{Block, InternalTransaction, Sequence, TokenBalance, Tracer}
   alias Indexer.Memory.Shrinkable
 
@@ -108,7 +109,7 @@ defmodule Indexer.Block.Catchup.Fetcher do
     end
   end
 
-  @async_import_remaining_block_data_options ~w(address_hash_to_fetched_balance_block_number transaction_hash_to_block_number)a
+  @async_import_remaining_block_data_options ~w(address_hash_to_fetched_balance_block_number)a
 
   @impl Block.Fetcher
   def import(_, options) when is_map(options) do
@@ -129,25 +130,25 @@ defmodule Indexer.Block.Catchup.Fetcher do
 
   defp async_import_remaining_block_data(imported, options) do
     async_import_coin_balances(imported, options)
-    async_import_internal_transactions(imported, options)
+    async_import_internal_transactions(imported)
     async_import_tokens(imported)
     async_import_token_balances(imported)
     async_import_uncles(imported)
   end
 
-  defp async_import_internal_transactions(%{transactions: transactions}, %{
+  defp async_import_internal_transactions(%{transactions: transactions}) do
-         transaction_hash_to_block_number: transaction_hash_to_block_number
-       }) do
     transactions
-    |> Enum.map(fn transaction_hash ->
+    |> Enum.flat_map(fn
-      transaction = Map.fetch!(transaction_hash_to_block_number, to_string(transaction_hash))
+      %Transaction{block_number: block_number, index: index, hash: hash, internal_transactions_indexed_at: nil} ->
+        [%{block_number: block_number, index: index, hash: hash}]
 
-      %{block_number: transaction[:block_number], hash: transaction_hash, index: transaction[:index]}
+      %Transaction{internal_transactions_indexed_at: %DateTime{}} ->
+        []
     end)
     |> InternalTransaction.Fetcher.async_fetch(10_000)
   end
 
-  defp async_import_internal_transactions(_, _), do: :ok
+  defp async_import_internal_transactions(_), do: :ok
 
   defp async_import_token_balances(%{address_token_balances: token_balances}) do
     TokenBalance.Fetcher.async_fetch(token_balances)

apps/indexer/lib/indexer/block/fetcher.ex

@@ -15,7 +15,6 @@ defmodule Indexer.Block.Fetcher do
   alias Indexer.Block.Transform
 
   @type address_hash_to_fetched_balance_block_number :: %{String.t() => Block.block_number()}
-  @type transaction_hash_to_block_number :: %{String.t() => Block.block_number()}
 
   @type t :: %__MODULE__{}
 
@@ -26,7 +25,6 @@ defmodule Indexer.Block.Fetcher do
               t,
               %{
                 address_hash_to_fetched_balance_block_number: address_hash_to_fetched_balance_block_number,
-                transaction_hash_to_block_number_option: transaction_hash_to_block_number,
                 addresses: Import.Runner.options(),
                 address_coin_balances: Import.Runner.options(),
                 address_token_balances: Import.Runner.options(),
@@ -161,15 +159,12 @@ defmodule Indexer.Block.Fetcher do
     {address_hash_to_fetched_balance_block_number, import_options} =
       pop_address_hash_to_fetched_balance_block_number(options)
 
-    transaction_hash_to_block_number = get_transaction_hash_to_block_number(import_options)
-
     options_with_broadcast =
       Map.merge(
         import_options,
         %{
           address_hash_to_fetched_balance_block_number: address_hash_to_fetched_balance_block_number,
-          broadcast: broadcast,
+          broadcast: broadcast
-          transaction_hash_to_block_number: transaction_hash_to_block_number
         }
       )
 
@@ -248,14 +243,6 @@ defmodule Indexer.Block.Fetcher do
     {address_hash_to_fetched_balance_block_number, import_options}
   end
 
-  defp get_transaction_hash_to_block_number(options) do
-    options
-    |> get_in([:transactions, :params, Access.all()])
-    |> Enum.into(%{}, fn %{block_number: block_number, hash: hash, index: index} ->
-      {hash, %{block_number: block_number, index: index}}
-    end)
-  end
-
   defp pop_hash_fetched_balance_block_number(
          %{
            fetched_coin_balance_block_number: fetched_coin_balance_block_number,

apps/indexer/lib/indexer/block/realtime/fetcher.ex

@@ -84,7 +84,7 @@ defmodule Indexer.Block.Realtime.Fetcher do
 
   defp new_max_number(number, max_number_seen), do: max(number, max_number_seen)
 
-  @import_options ~w(address_hash_to_fetched_balance_block_number transaction_hash_to_block_number)a
+  @import_options ~w(address_hash_to_fetched_balance_block_number)a
 
   @impl Block.Fetcher
   def import(
@@ -302,16 +302,21 @@ defmodule Indexer.Block.Realtime.Fetcher do
   end
 
   defp transactions_params_to_fetch_internal_transactions_params(transactions_params) do
-    Enum.map(transactions_params, &transaction_params_to_fetch_internal_transaction_params/1)
+    Enum.flat_map(transactions_params, &transaction_params_to_fetch_internal_transaction_params_list/1)
   end
 
-  defp transaction_params_to_fetch_internal_transaction_params(%{
+  # Input-less transactions are value-transfers only, so their internal transactions do not need to be indexed
+  defp transaction_params_to_fetch_internal_transaction_params_list(%{input: "0x"}) do
+    []
+  end
+
+  defp transaction_params_to_fetch_internal_transaction_params_list(%{
          block_number: block_number,
          hash: hash,
          transaction_index: transaction_index
        })
        when is_integer(block_number) do
-    %{block_number: block_number, hash_data: to_string(hash), transaction_index: transaction_index}
+    [%{block_number: block_number, hash_data: to_string(hash), transaction_index: transaction_index}]
   end
 
   defp balances(

apps/indexer/lib/indexer/block/uncle/fetcher.ex

@@ -139,7 +139,7 @@ defmodule Indexer.Block.Uncle.Fetcher do
     end
   end
 
-  @ignored_options ~w(address_hash_to_fetched_balance_block_number transaction_hash_to_block_number)a
+  @ignored_options ~w(address_hash_to_fetched_balance_block_number)a
 
   @impl Block.Fetcher
   def import(_, options) when is_map(options) do

apps/indexer/test/indexer/block/catchup/fetcher_test.exs

@@ -105,8 +105,7 @@ defmodule Indexer.Block.Catchup.FetcherTest do
                  transactions: %{
                    params: [],
                    on_conflict: :nothing
-                 },
+                 }
-                 transaction_hash_to_block_number: %{}
                })
 
       assert_receive {:uncles, [^uncle_hash]}

apps/indexer/test/indexer/block/fetcher_test.exs

@@ -459,17 +459,27 @@ defmodule Indexer.Block.FetcherTest do
                     ],
                     logs: [],
                     transactions: [
-                      %Explorer.Chain.Hash{
+                      %Transaction{
-                        byte_count: 32,
+                        block_number: block_number,
-                        bytes:
+                        index: 0,
-                          <<76, 188, 236, 37, 153, 153, 224, 115, 252, 79, 176, 224, 228, 166, 18, 66, 94, 61, 115, 57,
+                        hash: %Explorer.Chain.Hash{
-                            47, 162, 37, 255, 36, 96, 161, 238, 171, 66, 99, 10>>
+                          byte_count: 32,
+                          bytes:
+                            <<76, 188, 236, 37, 153, 153, 224, 115, 252, 79, 176, 224, 228, 166, 18, 66, 94, 61, 115,
+                              57, 47, 162, 37, 255, 36, 96, 161, 238, 171, 66, 99, 10>>
+                        },
+                        internal_transactions_indexed_at: nil
                       },
-                      %Explorer.Chain.Hash{
+                      %Transaction{
-                        byte_count: 32,
+                        block_number: block_number,
-                        bytes:
+                        index: 1,
-                          <<240, 237, 34, 44, 16, 174, 248, 135, 4, 196, 15, 198, 34, 220, 218, 174, 13, 208, 242, 122,
+                        hash: %Explorer.Chain.Hash{
-                            154, 143, 4, 28, 171, 95, 190, 255, 254, 174, 75, 182>>
+                          byte_count: 32,
+                          bytes:
+                            <<240, 237, 34, 44, 16, 174, 248, 135, 4, 196, 15, 198, 34, 220, 218, 174, 13, 208, 242,
+                              122, 154, 143, 4, 28, 171, 95, 190, 255, 254, 174, 75, 182>>
+                        },
+                        internal_transactions_indexed_at: nil
                       }
                     ]
                   }} = Fetcher.fetch_and_import_range(block_fetcher, block_number..block_number)
@@ -553,11 +563,16 @@ defmodule Indexer.Block.FetcherTest do
                         }
                       ],
                       transactions: [
-                        %Explorer.Chain.Hash{
+                        %Transaction{
-                          byte_count: 32,
+                          block_number: block_number,
-                          bytes:
+                          index: 0,
-                            <<83, 189, 136, 72, 114, 222, 62, 72, 134, 146, 136, 27, 174, 236, 38, 46, 123, 149, 35, 77,
+                          hash: %Explorer.Chain.Hash{
-                              57, 101, 36, 140, 57, 254, 153, 47, 255, 212, 51, 229>>
+                            byte_count: 32,
+                            bytes:
+                              <<83, 189, 136, 72, 114, 222, 62, 72, 134, 146, 136, 27, 174, 236, 38, 46, 123, 149, 35,
+                                77, 57, 101, 36, 140, 57, 254, 153, 47, 255, 212, 51, 229>>
+                          },
+                          internal_transactions_indexed_at: nil
                         }
                       ]
                     },

apps/indexer/test/indexer/block/realtime/fetcher_test.exs

@@ -5,7 +5,7 @@ defmodule Indexer.Block.Realtime.FetcherTest do
   import Mox
 
   alias Explorer.Chain
-  alias Explorer.Chain.Address
+  alias Explorer.Chain.{Address, Transaction}
   alias Indexer.{Sequence, Token, TokenBalance}
   alias Indexer.Block.{Realtime, Uncle}
 
@@ -410,7 +410,7 @@ defmodule Indexer.Block.Realtime.FetcherTest do
                     %{index: 4, transaction_hash: transaction_hash},
                     %{index: 5, transaction_hash: transaction_hash}
                   ],
-                  transactions: [transaction_hash]
+                  transactions: [%Transaction{hash: transaction_hash}]
                 },
                 errors: []
               }} = Indexer.Block.Fetcher.fetch_and_import_range(block_fetcher, 3_946_079..3_946_080)