Skip to content

Commit

Permalink
Derive status from internal transaction at index 0
Browse files Browse the repository at this point in the history
Why:

* Issue #757 was created after we noticed a few constraint violations
for the `error` column in the `transactions` table. The constraint
violations came about as we attempted to update transactions with a
successful status but some value for the error field. The constraint
is violated in this scenario because we expect successful transactions
to not have an error. [EIP 658](ethereum/EIPs#658),
which apparently is also called [EIP 98](ethereum/EIPs#98 (comment))
, says that the status should be set to 1 (success) if the outermost
code execution succeeded (internal transaction with index 0), or it
should be set to 0 (failure) if the outermost code execution failed.
We're currently deriving the status from the last internal transaction
when in fact we should be deriving it from the first internal
transaction.
* Issue link: #757

This change addresses the need by:

* Editing `Explorer.Chain.Import.update_transactions/2` to set the error
for a transaction equal to the error, if any, of the internal
transaction with index 0.
* Editing `Explorer.Chain.Import.update_transactions/2` to set a
transaction's status as successful if it's internal transaction at index
0 doesn't have an error, and as failed if it does. This only applies to
pre-Byzantium and Ethereum Classic, for which transaction status is not
set from receipts.
  • Loading branch information
sabondano committed Sep 19, 2018
1 parent 23d7416 commit 6be5bc8
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions apps/explorer/lib/explorer/chain.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1384,12 +1384,12 @@ defmodule Explorer.Chain do
* `:pending` - the transaction has not be confirmed in a block yet.
* `:awaiting_internal_transactions` - the transaction happened in a pre-Byzantium block or on a chain like Ethereum
Classic (ETC) that never adopted [EIP-658](https://github.com/Arachnid/EIPs/blob/master/EIPS/eip-658.md), which
add transaction status to transaction receipts, so the stats can only be derived whether the last internal
add transaction status to transaction receipts, so the status can only be derived whether the first internal
transaction has an error.
* `:success` - the transaction has been confirmed in a block
* `{:error, :awaiting_internal_transactions}` - the transactions happened post-Byzantium, but the error message
requires the internal transactions.
* `{:error, reason}` - the transaction failed due to `reason` in its last internal transaction.
* `{:error, reason}` - the transaction failed due to `reason` in its first internal transaction.
"""
@spec transaction_to_status(Transaction.t()) ::
Expand Down
4 changes: 2 additions & 2 deletions apps/explorer/lib/explorer/chain/import.ex
Original file line number Diff line number Diff line change
Expand Up @@ -868,12 +868,12 @@ defmodule Explorer.Chain.Import do
),
error:
fragment(
"(SELECT it.error FROM internal_transactions AS it WHERE it.transaction_hash = ? ORDER BY it.index DESC LIMIT 1)",
"(SELECT it.error FROM internal_transactions AS it WHERE it.transaction_hash = ? ORDER BY it.index ASC LIMIT 1)",
t.hash
),
status:
fragment(
"COALESCE(?, CASE WHEN (SELECT it.error FROM internal_transactions AS it WHERE it.transaction_hash = ? ORDER BY it.index DESC LIMIT 1) IS NULL THEN ? ELSE ? END)",
"COALESCE(?, CASE WHEN (SELECT it.error FROM internal_transactions AS it WHERE it.transaction_hash = ? ORDER BY it.index ASC LIMIT 1) IS NULL THEN ? ELSE ? END)",
t.status,
t.hash,
type(^:ok, t.status),
Expand Down

0 comments on commit 6be5bc8

Please sign in to comment.