Merge pull request #382 from poanetwork/bug-fix-multiple-create-internal-transactions-#367

Fix multiple results in Repo.One with multiple create type internal transactions

apps/explorer/lib/explorer/chain.ex

@@ -7,7 +7,6 @@ defmodule Explorer.Chain do
     only: [
       from: 2,
       join: 4,
-      join: 5,
       limit: 2,
       order_by: 2,
       order_by: 3,
@@ -147,7 +146,7 @@ defmodule Explorer.Chain do
             SELECT t0."hash" address
             FROM "transactions" AS t0
             LEFT OUTER JOIN "internal_transactions" AS i1 ON (i1."transaction_hash" = t0."hash") AND (i1."type" = 'create')
-            WHERE (i1."created_contract_address_hash" = $1)
+            WHERE (i1."created_contract_address_hash" = $1 AND t0."to_address_hash" IS NULL)
 
             UNION
 
@@ -459,7 +458,7 @@ defmodule Explorer.Chain do
     |> Keyword.get(:paging_options, @default_paging_options)
     |> fetch_transactions()
     |> join(:inner, [transaction], block in assoc(transaction, :block))
-    |> where([_, _, block], block.hash == ^block_hash)
+    |> where([_, block], block.hash == ^block_hash)
     |> join_associations(necessity_by_association)
     |> Repo.all()
   end
@@ -2101,7 +2100,7 @@ defmodule Explorer.Chain do
     |> where_transaction_has_multiple_internal_transactions()
     |> page_internal_transaction(paging_options)
     |> limit(^paging_options.page_size)
-    |> order_by([internal_transaction], desc: internal_transaction.index)
+    |> order_by([internal_transaction], asc: internal_transaction.index)
     |> Repo.all()
   end
 
@@ -2252,9 +2251,20 @@ defmodule Explorer.Chain do
 
   defp fetch_transactions(paging_options \\ nil) do
     Transaction
-    |> load_contract_creation()
-    |> select_merge([_, internal_transaction], %{
-      created_contract_address_hash: internal_transaction.created_contract_address_hash
+    |> select_merge([transaction], %{
+      created_contract_address_hash:
+        type(
+          fragment(
+            ~s[
+              (SELECT i."created_contract_address_hash"
+              FROM "internal_transactions" AS i
+              WHERE (i."transaction_hash" = ?) AND (i."type" = 'create')
+              LIMIT 1)
+              ],
+            transaction.hash
+          ),
+          Explorer.Chain.Hash.Truncated
+        )
     })
     |> order_by([transaction], desc: transaction.block_number, desc: transaction.index)
     |> handle_paging_options(paging_options)
@@ -2501,16 +2511,6 @@ defmodule Explorer.Chain do
     end)
   end
 
-  defp load_contract_creation(query) do
-    join(
-      query,
-      :left,
-      [transaction],
-      internal_transaction in assoc(transaction, :internal_transactions),
-      internal_transaction.type == ^:create
-    )
-  end
-
   defp page_blocks(query, %PagingOptions{key: nil}), do: query
 
   defp page_blocks(query, %PagingOptions{key: {block_number}}) do
@@ -2531,7 +2531,7 @@ defmodule Explorer.Chain do
   end
 
   defp page_internal_transaction(query, %PagingOptions{key: {index}}) do
-    where(query, [internal_transaction], internal_transaction.index < ^index)
+    where(query, [internal_transaction], internal_transaction.index > ^index)
   end
 
   defp page_logs(query, %PagingOptions{key: nil}), do: query
@@ -2698,9 +2698,18 @@ defmodule Explorer.Chain do
   defp where_address_fields_match(%Ecto.Query{from: {_table, Transaction}} = query, address_hash, nil) do
     where(
       query,
-      [t, it],
+      [t],
       t.to_address_hash == ^address_hash or t.from_address_hash == ^address_hash or
-        it.created_contract_address_hash == ^address_hash
+        (is_nil(t.to_address_hash) and
+           ^address_hash.bytes in fragment(
+             ~s[
+            (SELECT i."created_contract_address_hash"
+            FROM "internal_transactions" AS i
+            WHERE (i."transaction_hash" = ?) AND (i."type" = 'create')
+            LIMIT 1)
+          ],
+             t.hash
+           ))
     )
   end
 

apps/explorer/test/explorer/chain_test.exs

@@ -23,18 +23,17 @@ defmodule Explorer.ChainTest do
       assert Chain.address_to_transaction_count(address) == 2
     end
 
-    test "with transactions and contract creation address" do
-      %Transaction{from_address: address} = insert(:transaction) |> Repo.preload(:from_address)
-      insert(:transaction, to_address: address)
+    test "with contract creation transactions the contract address is counted" do
+      address = insert(:address)
 
       insert(
         :internal_transaction_create,
         created_contract_address: address,
         index: 0,
-        transaction: insert(:transaction)
+        transaction: insert(:transaction, to_address: nil)
       )
 
-      assert Chain.address_to_transaction_count(address) == 3
+      assert Chain.address_to_transaction_count(address) == 1
     end
 
     test "doesn't double count addresses when to_address = from_address" do
@@ -43,6 +42,19 @@ defmodule Explorer.ChainTest do
 
       assert Chain.address_to_transaction_count(address) == 2
     end
+
+    test "does not count non-contract-creation parent transactions" do
+      transaction_with_to_address =
+        %Transaction{} =
+        :transaction
+        |> insert()
+        |> with_block()
+
+      %InternalTransaction{created_contract_address: address} =
+        insert(:internal_transaction_create, transaction: transaction_with_to_address, index: 0)
+
+      assert Chain.address_to_transaction_count(address) == 0
+    end
   end
 
   describe "address_to_transactions/2" do
@@ -104,6 +116,19 @@ defmodule Explorer.ChainTest do
                Chain.address_to_transactions(address) |> Repo.preload([:block, :to_address, :from_address])
     end
 
+    test "does not include non-contract-creation parent transactions" do
+      transaction =
+        %Transaction{} =
+        :transaction
+        |> insert()
+        |> with_block()
+
+      %InternalTransaction{created_contract_address: address} =
+        insert(:internal_transaction_create, transaction: transaction, index: 0)
+
+      assert [] == Chain.address_to_transactions(address)
+    end
+
     test "with transactions can be paginated" do
       address = insert(:address)
 
@@ -335,6 +360,22 @@ defmodule Explorer.ChainTest do
                  necessity_by_association: %{block: :required}
                )
     end
+
+    test "transaction with multiple create internal transactions is returned" do
+      transaction =
+        %Transaction{hash: hash_with_block} =
+        :transaction
+        |> insert()
+        |> with_block()
+
+      insert(:internal_transaction, transaction: transaction, index: 0)
+
+      Enum.each(1..3, fn index ->
+        insert(:internal_transaction_create, transaction: transaction, index: index)
+      end)
+
+      assert {:ok, %Transaction{hash: ^hash_with_block}} = Chain.hash_to_transaction(hash_with_block)
+    end
   end
 
   describe "list_blocks/2" do
@@ -660,7 +701,7 @@ defmodule Explorer.ChainTest do
       assert actual.id == expected.id
     end
 
-    test "returns the internal transactions in descending index order" do
+    test "returns the internal transactions in ascending index order" do
       transaction =
         :transaction
         |> insert()
@@ -674,7 +715,7 @@ defmodule Explorer.ChainTest do
         |> Chain.transaction_to_internal_transactions()
         |> Enum.map(& &1.id)
 
-      assert [second_id, first_id] == result
+      assert [first_id, second_id] == result
     end
   end
 

apps/explorer_web/lib/explorer_web/templates/transaction_internal_transaction/index.html.eex

@@ -68,7 +68,7 @@
 
     <%= if @next_page_params do %>
       <%= link(
-        gettext("Older"),
+        gettext("Newer"),
         class: "button button--secondary button--sm u-float-right mt-3",
         to: transaction_internal_transaction_path(
           @conn,

apps/explorer_web/priv/gettext/default.pot

@@ -494,7 +494,6 @@ msgstr ""
 #: lib/explorer_web/templates/block_transaction/index.html.eex:198
 #: lib/explorer_web/templates/pending_transaction/index.html.eex:73
 #: lib/explorer_web/templates/transaction/index.html.eex:90
-#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71
 msgid "Older"
 msgstr ""
 
@@ -525,6 +524,7 @@ msgid "Last Updated In Block"
 msgstr ""
 
 #, elixir-format
+#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71
 #: lib/explorer_web/templates/transaction_log/index.html.eex:73
 msgid "Newer"
 msgstr ""

apps/explorer_web/priv/gettext/en/LC_MESSAGES/default.po

@@ -506,7 +506,6 @@ msgstr ""
 #: lib/explorer_web/templates/block_transaction/index.html.eex:198
 #: lib/explorer_web/templates/pending_transaction/index.html.eex:73
 #: lib/explorer_web/templates/transaction/index.html.eex:90
-#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71
 msgid "Older"
 msgstr ""
 
@@ -537,6 +536,7 @@ msgid "Last Updated In Block"
 msgstr ""
 
 #, elixir-format
+#: lib/explorer_web/templates/transaction_internal_transaction/index.html.eex:71
 #: lib/explorer_web/templates/transaction_log/index.html.eex:73
 msgid "Newer"
 msgstr ""

apps/explorer_web/test/explorer_web/controllers/transaction_internal_transaction_controller_test.exs

@@ -88,13 +88,13 @@ defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do
         |> insert()
         |> with_block()
 
+      %InternalTransaction{index: index} = insert(:internal_transaction, transaction: transaction, index: 0)
+
       second_page_indexes =
         1..50
         |> Enum.map(fn index -> insert(:internal_transaction, transaction: transaction, index: index) end)
         |> Enum.map(& &1.index)
 
-      %InternalTransaction{index: index} = insert(:internal_transaction, transaction: transaction, index: 51)
-
       conn =
         get(conn, transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, transaction.hash), %{
           "index" => Integer.to_string(index)
@@ -103,7 +103,6 @@ defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do
       actual_indexes =
         conn.assigns.internal_transactions
         |> Enum.map(& &1.index)
-        |> Enum.reverse()
 
       assert second_page_indexes == actual_indexes
     end
@@ -128,7 +127,7 @@ defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do
 
       conn = get(conn, transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, transaction.hash))
 
-      assert %{"block_number" => ^number, "index" => 11, "transaction_index" => ^transaction_index} =
+      assert %{"block_number" => ^number, "index" => 50, "transaction_index" => ^transaction_index} =
                conn.assigns.next_page_params
     end