Merge pull request #8513 from blockscout/dont-override-transaction-status

Don't override transaction status

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### Fixes
 
+- [#8513](https://github.com/blockscout/blockscout/pull/8513) - Don't override transaction status
+
 ### Chore
 
 - [#8504](https://github.com/blockscout/blockscout/pull/8504) - Deploy new UI through Makefile

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

@@ -311,7 +311,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
       from(
         t in Transaction,
         where: t.block_hash in ^pending_block_hashes,
-        select: map(t, [:hash, :block_hash, :block_number, :cumulative_gas_used]),
+        select: map(t, [:hash, :block_hash, :block_number, :cumulative_gas_used, :status]),
         # Enforce Transaction ShareLocks order (see docs: sharelocks.md)
         order_by: [asc: t.hash],
         lock: "FOR NO KEY UPDATE"
@@ -514,6 +514,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
           timeout,
           timestamps,
           first_trace,
+          transaction_from_db,
           transaction_receipt_from_node
         )
 
@@ -525,7 +526,8 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
           transaction_hashes_iterator,
           timeout,
           timestamps,
-          first_trace
+          first_trace,
+          transaction_from_db
         )
 
       true ->
@@ -539,6 +541,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
           timeout,
           timestamps,
           first_trace,
+          transaction_from_db,
           transaction_receipt_from_node
         )
     end
@@ -579,6 +582,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
     end
   end
 
+  # credo:disable-for-next-line
   defp update_transactions_inner(
          repo,
          valid_internal_transactions,
@@ -587,6 +591,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
          timeout,
          timestamps,
          first_trace,
+         transaction_from_db,
          transaction_receipt_from_node \\ nil
        ) do
     valid_internal_transactions_count = Enum.count(valid_internal_transactions)
@@ -595,6 +600,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
     set =
       generate_transaction_set_to_update(
         first_trace,
+        transaction_from_db,
         transaction_receipt_from_node,
         timestamps,
         txs_with_error_in_internal_txs
@@ -628,19 +634,20 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
 
   def generate_transaction_set_to_update(
         first_trace,
+        transaction_from_db,
         transaction_receipt_from_node,
         timestamps,
         txs_with_error_in_internal_txs
       ) do
     default_set = [
       created_contract_address_hash: first_trace.created_contract_address_hash,
-      error: first_trace.error,
-      status: first_trace.status,
       updated_at: timestamps.updated_at
     ]
 
     set =
       default_set
+      |> put_status_in_update_set(first_trace, transaction_from_db)
+      |> put_error_in_update_set(first_trace, transaction_from_db, transaction_receipt_from_node)
       |> Keyword.put_new(:block_hash, first_trace.block_hash)
       |> Keyword.put_new(:block_number, first_trace.block_number)
       |> Keyword.put_new(:index, transaction_receipt_from_node && transaction_receipt_from_node.transaction_index)
@@ -665,6 +672,24 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactions do
     filtered_set
   end
 
+  defp put_status_in_update_set(update_set, first_trace, %{status: nil}),
+    do: Keyword.put_new(update_set, :status, first_trace.status)
+
+  defp put_status_in_update_set(update_set, _first_trace, _transaction_from_db), do: update_set
+
+  defp put_error_in_update_set(update_set, first_trace, _transaction_from_db, %{status: :error}),
+    do: Keyword.put_new(update_set, :error, first_trace.error)
+
+  defp put_error_in_update_set(update_set, first_trace, %{status: :error}, _transaction_receipt_from_node),
+    do: Keyword.put_new(update_set, :error, first_trace.error)
+
+  defp put_error_in_update_set(update_set, first_trace, _transaction_from_db, _transaction_receipt_from_node) do
+    case update_set[:status] do
+      :error -> Keyword.put_new(update_set, :error, first_trace.error)
+      _ -> update_set
+    end
+  end
+
   defp remove_consensus_of_invalid_blocks(repo, invalid_block_numbers) do
     minimal_block = EthereumJSONRPC.first_block_to_fetch(:trace_first_block)
 

apps/explorer/test/explorer/chain/import/runner/internal_transactions_test.exs

@@ -6,7 +6,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactionsTest do
   alias Explorer.Chain.Import.Runner.InternalTransactions
 
   describe "run/1" do
-    test "transaction's status becomes :error when its internal_transaction has an error" do
+    test "transaction's status doesn't become :error when its internal_transaction has an error" do
       transaction = insert(:transaction) |> with_block(status: :ok)
       insert(:pending_block_operation, block_hash: transaction.block_hash, block_number: transaction.block_number)
 
@@ -19,7 +19,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactionsTest do
 
       assert {:ok, _} = run_internal_transactions([internal_transaction_changes])
 
-      assert :error == Repo.get(Transaction, transaction.hash).status
+      assert :ok == Repo.get(Transaction, transaction.hash).status
     end
 
     test "transaction's has_error_in_internal_txs become true when its internal_transaction (where index != 0) has an error" do
@@ -61,7 +61,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactionsTest do
       assert {:ok, _} = run_internal_transactions([internal_transaction_changes])
       tx = Repo.get(Transaction, transaction.hash)
 
-      assert :error == tx.status
+      assert :ok == tx.status
       assert false == tx.has_error_in_internal_txs
     end
 
@@ -90,7 +90,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactionsTest do
       assert false == tx.has_error_in_internal_txs
     end
 
-    test "simple coin transfer's status becomes :error when its internal_transaction has an error" do
+    test "simple coin transfer's status doesn't become :error when its internal_transaction has an error" do
       transaction = insert(:transaction) |> with_block(status: :ok)
       insert(:pending_block_operation, block_hash: transaction.block_hash, block_number: transaction.block_number)
 
@@ -104,10 +104,10 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactionsTest do
 
       assert {:ok, _} = run_internal_transactions([internal_transaction_changes])
 
-      assert :error == Repo.get(Transaction, transaction.hash).status
+      assert :ok == Repo.get(Transaction, transaction.hash).status
     end
 
-    test "for block with 2 simple coin transfer's statuses become :error when its both internal_transactions has an error" do
+    test "for block with 2 simple coin transfer's statuses doesn't become :error even when its both internal_transactions has an error" do
       a_block = insert(:block, number: 1000)
       transaction1 = insert(:transaction) |> with_block(a_block, status: :ok)
       transaction2 = insert(:transaction) |> with_block(a_block, status: :ok)
@@ -128,31 +128,7 @@ defmodule Explorer.Chain.Import.Runner.InternalTransactionsTest do
 
       assert {:ok, _} = run_internal_transactions([internal_transaction_changes_1, internal_transaction_changes_2])
 
-      assert :error == Repo.get(Transaction, transaction1.hash).status
-      assert :error == Repo.get(Transaction, transaction2.hash).status
-    end
-
-    test "for block with 2 simple coin transfer's only status become :error for tx where internal_transactions has an error" do
-      a_block = insert(:block, number: 1000)
-      transaction1 = insert(:transaction) |> with_block(a_block, status: :ok)
-      transaction2 = insert(:transaction) |> with_block(a_block, status: :ok)
-      insert(:pending_block_operation, block_hash: a_block.hash, block_number: a_block.number)
-
-      assert :ok == transaction1.status
-      assert :ok == transaction2.status
-
-      index = 0
-      error = "Out of gas"
-
-      internal_transaction_changes_1 =
-        make_internal_transaction_changes_for_simple_coin_transfers(transaction1, index, error)
-
-      internal_transaction_changes_2 =
-        make_internal_transaction_changes_for_simple_coin_transfers(transaction2, index, nil)
-
-      assert {:ok, _} = run_internal_transactions([internal_transaction_changes_1, internal_transaction_changes_2])
-
-      assert :error == Repo.get(Transaction, transaction1.hash).status
+      assert :ok == Repo.get(Transaction, transaction1.hash).status
       assert :ok == Repo.get(Transaction, transaction2.hash).status
     end