Merge pull request #1124 from poanetwork/frg-fix-token-transfers-performance-issue

Fix performance issue at token transfers tab on Token's page

apps/block_scout_web/lib/block_scout_web/chain.ex

@@ -130,9 +130,6 @@ defmodule BlockScoutWeb.Chain do
     end
   end
 
-  def paging_options(%{"inserted_at" => inserted_at}),
-    do: [paging_options: %{@default_paging_options | key: inserted_at}]
-
   def paging_options(%{"token_name" => name, "token_type" => type, "token_inserted_at" => inserted_at}),
     do: [paging_options: %{@default_paging_options | key: {name, type, inserted_at}}]
 
@@ -180,13 +177,8 @@ defmodule BlockScoutWeb.Chain do
     %{"block_number" => block_number, "index" => index}
   end
 
-  defp paging_params(%TokenTransfer{inserted_at: inserted_at}) do
+  defp paging_params(%TokenTransfer{block_number: block_number, log_index: index}) do
-    inserted_at_datetime =
+    %{"block_number" => block_number, "index" => index}
-      inserted_at
-      |> DateTime.from_naive!("Etc/UTC")
-      |> DateTime.to_iso8601()
-
-    %{"inserted_at" => inserted_at_datetime}
   end
 
   defp paging_params(%Address.Token{name: name, type: type, inserted_at: inserted_at}) do

apps/block_scout_web/lib/block_scout_web/templates/tokens/transfer/_token_transfer.html.eex

@@ -43,7 +43,7 @@
           <%= link(
             gettext(
               "Block #%{number}",
-              number: @transfer.transaction.block_number
+              number: @transfer.block_number
             ),
             class: "mr-2 mr-sm-0 text-muted",
             to: block_path(BlockScoutWeb.Endpoint, :show, @transfer.transaction.block_number)

apps/block_scout_web/test/block_scout_web/controllers/transaction_token_transfer_controller_test.exs

@@ -82,25 +82,21 @@ defmodule BlockScoutWeb.TransactionTokenTransferControllerTest do
         |> insert()
         |> with_block()
 
-      {:ok, first_transfer_time} = NaiveDateTime.new(2000, 1, 1, 0, 0, 5)
+      token_transfer = insert(:token_transfer, transaction: transaction, block_number: 1000, log_index: 1)
-      {:ok, remaining_transfers_time} = NaiveDateTime.new(1999, 1, 1, 0, 0, 0)
-      insert(:token_transfer, transaction: transaction, inserted_at: first_transfer_time)
 
-      1..5
+      Enum.each(2..5, fn item ->
-      |> Enum.each(fn log_index ->
+        insert(:token_transfer, transaction: transaction, block_number: item + 1001, log_index: item + 1)
-        insert(:token_transfer, transaction: transaction, inserted_at: remaining_transfers_time, log_index: log_index)
       end)
 
       conn =
         get(conn, transaction_token_transfer_path(BlockScoutWeb.Endpoint, :index, transaction.hash), %{
-          "inserted_at" => first_transfer_time |> DateTime.from_naive!("Etc/UTC") |> DateTime.to_iso8601()
+          "block_number" => "1000",
+          "index" => "1"
         })
 
-      actual_times =
+      actual_log_indexes = Enum.map(conn.assigns.token_transfers, & &1.log_index)
-        conn.assigns.token_transfers
-        |> Enum.map(& &1.inserted_at)
 
-      refute Enum.any?(actual_times, fn time -> first_transfer_time == time end)
+      refute Enum.any?(actual_log_indexes, fn log_index -> log_index == token_transfer.log_index end)
     end
 
     test "next_page_params exist if not on last page", %{conn: conn} do

apps/explorer/lib/explorer/chain/token_transfer.ex

@@ -24,15 +24,17 @@ defmodule Explorer.Chain.TokenTransfer do
 
   use Ecto.Schema
 
-  import Ecto.{Changeset, Query}
+  import Ecto.Changeset
+  import Ecto.Query, only: [from: 2, dynamic: 2, limit: 2, where: 3]
 
-  alias Explorer.Chain.{Address, Block, Hash, Token, TokenTransfer, Transaction}
+  alias Explorer.Chain.{Address, Hash, Token, TokenTransfer, Transaction}
   alias Explorer.{PagingOptions, Repo}
 
   @default_paging_options %PagingOptions{page_size: 50}
 
   @typedoc """
   * `:amount` - The token transferred amount
+  * `:block_number` - The block number that the transfer took place.
   * `:from_address` - The `t:Explorer.Chain.Address.t/0` that sent the tokens
   * `:from_address_hash` - Address hash foreign key
   * `:to_address` - The `t:Explorer.Chain.Address.t/0` that received the tokens
@@ -46,6 +48,7 @@ defmodule Explorer.Chain.TokenTransfer do
   """
   @type t :: %TokenTransfer{
           amount: Decimal.t(),
+          block_number: non_neg_integer() | nil,
           from_address: %Ecto.Association.NotLoaded{} | Address.t(),
           from_address_hash: Hash.Address.t(),
           to_address: %Ecto.Association.NotLoaded{} | Address.t(),
@@ -65,6 +68,7 @@ defmodule Explorer.Chain.TokenTransfer do
   @primary_key false
   schema "token_transfers" do
     field(:amount, :decimal)
+    field(:block_number, :integer)
     field(:log_index, :integer, primary_key: true)
     field(:token_id, :decimal)
 
@@ -91,7 +95,7 @@ defmodule Explorer.Chain.TokenTransfer do
     timestamps()
   end
 
-  @required_attrs ~w(log_index from_address_hash to_address_hash token_contract_address_hash transaction_hash)a
+  @required_attrs ~w(block_number log_index from_address_hash to_address_hash token_contract_address_hash transaction_hash)a
   @optional_attrs ~w(amount token_id)a
 
   @doc false
@@ -118,13 +122,9 @@ defmodule Explorer.Chain.TokenTransfer do
     query =
       from(
         tt in TokenTransfer,
-        join: t in Transaction,
-        on: tt.transaction_hash == t.hash,
-        join: b in Block,
-        on: t.block_hash == b.hash,
         where: tt.token_contract_address_hash == ^token_address_hash,
         preload: [{:transaction, :block}, :token, :from_address, :to_address],
-        order_by: [desc: b.timestamp]
+        order_by: [desc: tt.block_number, desc: tt.log_index]
       )
 
     query
@@ -135,19 +135,11 @@ defmodule Explorer.Chain.TokenTransfer do
 
   def page_token_transfer(query, %PagingOptions{key: nil}), do: query
 
-  def page_token_transfer(query, %PagingOptions{key: {token_id}}) do
+  def page_token_transfer(query, %PagingOptions{key: {block_number, log_index}}) do
     where(
       query,
-      [token_transfer],
+      [tt],
-      token_transfer.token_id > ^token_id
+      tt.block_number < ^block_number or (tt.block_number == ^block_number and tt.log_index < ^log_index)
-    )
-  end
-
-  def page_token_transfer(query, %PagingOptions{key: inserted_at}) do
-    where(
-      query,
-      [token_transfer],
-      token_transfer.inserted_at < ^inserted_at
     )
   end
 

apps/explorer/priv/repo/migrations/20181121170616_add_block_number_to_token_transfers.exs

@@ -0,0 +1,18 @@
+defmodule Explorer.Repo.Migrations.AddBlockNumberToTokenTransfers do
+  @moduledoc """
+  Use `priv/repo/migrations/scripts/20181121170616_token_transfers_update_block_number_in_batches.sql` to migrate data.
+
+  ```sh
+  mix ecto.migrate
+  psql -d $DATABASE -a -f priv/repo/migrations/scripts/20181121170616_token_transfers_update_block_number_in_batches.sql
+  ```
+  """
+
+  use Ecto.Migration
+
+  def change do
+    alter table(:token_transfers) do
+      add(:block_number, :integer)
+    end
+  end
+end

apps/explorer/priv/repo/migrations/scripts/20181121170616_token_transfers_update_block_number_in_batches.sql

@@ -0,0 +1,39 @@
+DO $$
+DECLARE
+   row_count integer;
+   batch_size  integer := 100000; -- HOW MANY ITEMS WILL BE UPDATED AT TIME
+   affected integer;
+BEGIN
+  RAISE NOTICE 'Counting items to be updated';
+
+  row_count := (SELECT COUNT(*) FROM token_transfers WHERE block_number IS NULL);
+
+  RAISE NOTICE '% items', row_count;
+
+  WHILE row_count > 0 LOOP
+    WITH cte AS (
+     SELECT
+       t.hash,
+       t.block_number
+     FROM token_transfers AS tt
+     INNER JOIN transactions AS t ON t.hash = tt.transaction_hash
+	   WHERE tt.block_number IS NULL
+     LIMIT batch_size
+    )
+    UPDATE token_transfers
+    SET
+      block_number = cte.block_number
+    FROM cte
+    WHERE token_transfers.transaction_hash = cte.hash;
+
+    GET DIAGNOSTICS affected = ROW_COUNT;
+    RAISE NOTICE '-> % token transfers updated!', affected;
+
+    -- UPDATES THE COUNTER SO IT DOESN'T TURN INTO AN INFINITE LOOP
+    row_count := row_count - batch_size;
+
+    RAISE NOTICE '-> % items missing to update', row_count;
+
+    CHECKPOINT; -- COMMITS THE BATCH UPDATES
+  END LOOP;
+END $$;

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

@@ -1517,7 +1517,9 @@ defmodule Explorer.Chain.ImportTest do
                  },
                  token_transfers: %{
                    params: [
-                     params_for(:token_transfer,
+                     params_for(
+                       :token_transfer,
+                       block_number: 35,
                        from_address_hash: from_address_hash,
                        to_address_hash: to_address_hash,
                        token_contract_address_hash: token_contract_address_hash,

apps/explorer/test/explorer/chain/token_transfer_test.exs

@@ -8,7 +8,7 @@ defmodule Explorer.Chain.TokenTransferTest do
 
   doctest Explorer.Chain.TokenTransfer
 
-  describe "fetch_token_transfers/2" do
+  describe "fetch_token_transfers_from_token_hash/2" do
     test "returns token transfers for the given address" do
       token_contract_address = insert(:contract_address)
 
@@ -87,6 +87,7 @@ defmodule Explorer.Chain.TokenTransferTest do
       second_page =
         insert(
           :token_transfer,
+          block_number: 999,
           to_address: build(:address),
           transaction: transaction,
           token_contract_address: token_contract_address,
@@ -96,23 +97,53 @@ defmodule Explorer.Chain.TokenTransferTest do
       first_page =
         insert(
           :token_transfer,
+          block_number: 1000,
           to_address: build(:address),
           transaction: transaction,
           token_contract_address: token_contract_address,
           token: token
         )
 
-      paging_options = %PagingOptions{key: first_page.inserted_at, page_size: 1}
+      paging_options = %PagingOptions{key: {first_page.block_number, first_page.log_index}, page_size: 1}
 
       token_transfers_primary_keys_paginated =
-        TokenTransfer.fetch_token_transfers_from_token_hash(
+        token_contract_address.hash
-          token_contract_address.hash,
+        |> TokenTransfer.fetch_token_transfers_from_token_hash(paging_options: paging_options)
-          paging_options: paging_options
-        )
         |> Enum.map(&{&1.transaction_hash, &1.log_index})
 
       assert token_transfers_primary_keys_paginated == [{second_page.transaction_hash, second_page.log_index}]
     end
+
+    test "paginates considering the log_index when there are repeated block numbers" do
+      token_contract_address = insert(:contract_address)
+
+      transaction =
+        :transaction
+        |> insert()
+        |> with_block()
+
+      token = insert(:token)
+
+      token_transfer =
+        insert(
+          :token_transfer,
+          block_number: 1000,
+          log_index: 0,
+          to_address: build(:address),
+          transaction: transaction,
+          token_contract_address: token_contract_address,
+          token: token
+        )
+
+      paging_options = %PagingOptions{key: {token_transfer.block_number, token_transfer.log_index + 1}, page_size: 1}
+
+      token_transfers_primary_keys_paginated =
+        token_contract_address.hash
+        |> TokenTransfer.fetch_token_transfers_from_token_hash(paging_options: paging_options)
+        |> Enum.map(&{&1.transaction_hash, &1.log_index})
+
+      assert token_transfers_primary_keys_paginated == [{token_transfer.transaction_hash, token_transfer.log_index}]
+    end
   end
 
   describe "count_token_transfers/0" do

apps/explorer/test/explorer/chain_test.exs

@@ -3112,6 +3112,7 @@ defmodule Explorer.ChainTest do
       first_page =
         insert(
           :token_transfer,
+          block_number: 1000,
           to_address: build(:address),
           transaction: transaction,
           token_contract_address: token_contract_address,
@@ -3122,6 +3123,7 @@ defmodule Explorer.ChainTest do
       second_page =
         insert(
           :token_transfer,
+          block_number: 999,
           to_address: build(:address),
           transaction: transaction,
           token_contract_address: token_contract_address,
@@ -3129,13 +3131,11 @@ defmodule Explorer.ChainTest do
           token_id: 29
         )
 
-      paging_options = %PagingOptions{key: {first_page.token_id}, page_size: 1}
+      paging_options = %PagingOptions{key: {first_page.block_number, first_page.log_index}, page_size: 1}
 
       unique_tokens_ids_paginated =
-        Chain.address_to_unique_tokens(
+        token_contract_address.hash
-          token_contract_address.hash,
+        |> Chain.address_to_unique_tokens(paging_options: paging_options)
-          paging_options: paging_options
-        )
         |> Enum.map(& &1.token_id)
 
       assert unique_tokens_ids_paginated == [second_page.token_id]

apps/explorer/test/support/factory.ex

@@ -387,6 +387,7 @@ defmodule Explorer.Factory do
 
     %TokenTransfer{
       amount: Decimal.new(1),
+      block_number: block_number(),
       from_address: from_address,
       to_address: to_address,
       token_contract_address: token_address,