Merge pull request #2198 from poanetwork/reduce-transaction-status-constraint

Reduce transaction status and error constraint
pull/2275/head
Victor Baranov 5 years ago committed by GitHub
commit 618ff5eb5a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 1
      CHANGELOG.md
  2. 105
      apps/explorer/lib/explorer/chain/transaction.ex
  3. 132
      apps/explorer/priv/repo/migrations/20190619154943_reduce_transaction_status_constraint.exs
  4. 2
      apps/explorer/test/explorer/chain/import_test.exs

@ -54,6 +54,7 @@
- [#2173](https://github.com/poanetwork/blockscout/pull/2173) - handle correctly empty transactions
- [#2174](https://github.com/poanetwork/blockscout/pull/2174) - fix reward channel joining
- [#2186](https://github.com/poanetwork/blockscout/pull/2186) - fix net version test
- [#2198](https://github.com/poanetwork/blockscout/pull/2198) - reduce transaction status and error constraint
- [#2167](https://github.com/poanetwork/blockscout/pull/2167) - feat: document eth rpc api mimicking endpoints
- [#2225](https://github.com/poanetwork/blockscout/pull/2225) - fix metadata decoding in Solidity 0.5.9 smart contract verification
- [#2204](https://github.com/poanetwork/blockscout/pull/2204) - fix large contract verification

@ -247,7 +247,7 @@ defmodule Explorer.Chain.Transaction do
end
@doc """
A pending transaction has neither `block_hash` nor an `index`
A pending transaction does not have a `block_hash`
iex> changeset = Explorer.Chain.Transaction.changeset(
...> %Transaction{},
@ -267,42 +267,6 @@ defmodule Explorer.Chain.Transaction do
iex> changeset.valid?
true
A pending transaction (which is indicated by not having a `block_hash`) can't have `block_number`,
`cumulative_gas_used`, `gas_used`, or `index`.
iex> changeset = Explorer.Chain.Transaction.changeset(
...> %Transaction{},
...> %{
...> from_address_hash: "0xe8ddc5c7a2d2f0d7a9798459c0104fdf5e987aca",
...> block_number: 34,
...> cumulative_gas_used: 0,
...> gas: 4700000,
...> gas_price: 100000000000,
...> gas_used: 4600000,
...> hash: "0x3a3eb134e6792ce9403ea4188e5e79693de9e4c94e499db132be086400da79e6",
...> index: 0,
...> input: "0x6060604052341561000f57600080fd5b336000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055506102db8061005e6000396000f300606060405260043610610062576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff1680630900f01014610067578063445df0ac146100a05780638da5cb5b146100c9578063fdacd5761461011e575b600080fd5b341561007257600080fd5b61009e600480803573ffffffffffffffffffffffffffffffffffffffff16906020019091905050610141565b005b34156100ab57600080fd5b6100b3610224565b6040518082815260200191505060405180910390f35b34156100d457600080fd5b6100dc61022a565b604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b341561012957600080fd5b61013f600480803590602001909190505061024f565b005b60008060009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff161415610220578190508073ffffffffffffffffffffffffffffffffffffffff1663fdacd5766001546040518263ffffffff167c010000000000000000000000000000000000000000000000000000000002815260040180828152602001915050600060405180830381600087803b151561020b57600080fd5b6102c65a03f1151561021c57600080fd5b5050505b5050565b60015481565b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff1614156102ac57806001819055505b505600a165627a7a72305820a9c628775efbfbc17477a472413c01ee9b33881f550c59d21bee9928835c854b0029",
...> nonce: 0,
...> r: 0xAD3733DF250C87556335FFE46C23E34DBAFFDE93097EF92F52C88632A40F0C75,
...> s: 0x72caddc0371451a58de2ca6ab64e0f586ccdb9465ff54e1c82564940e89291e3,
...> status: :ok,
...> v: 0x8d,
...> value: 0
...> }
...> )
iex> changeset.valid?
false
iex> Keyword.get_values(changeset.errors, :block_number)
[{"can't be set when the transaction is pending", []}]
iex> Keyword.get_values(changeset.errors, :cumulative_gas_used)
[{"can't be set when the transaction is pending", []}]
iex> Keyword.get_values(changeset.errors, :gas_used)
[{"can't be set when the transaction is pending", []}]
iex> Keyword.get_values(changeset.errors, :index)
[{"can't be set when the transaction is pending", []}]
iex> Keyword.get_values(changeset.errors, :status)
[{"can't be set when the transaction is pending", []}]
A collated transaction MUST have an `index` so its position in the `block` is known and the `cumulative_gas_used` ane
`gas_used` to know its fees.
@ -359,35 +323,6 @@ defmodule Explorer.Chain.Transaction do
iex> changeset.valid?
true
Once the `internal_transactions_indexed_at` is set, both pre- and post-Byzantium transactions will be able to know
their status, so if `internal_transaction_indexed_at` is set, `status` is required.
iex> changeset = Explorer.Chain.Transaction.changeset(
...> %Transaction{},
...> %{
...> block_hash: "0xe52d77084cab13a4e724162bcd8c6028e5ecfaa04d091ee476e96b9958ed6b47",
...> block_number: 34,
...> cumulative_gas_used: 0,
...> from_address_hash: "0xe8ddc5c7a2d2f0d7a9798459c0104fdf5e987aca",
...> gas: 4700000,
...> gas_price: 100000000000,
...> gas_used: 4600000,
...> hash: "0x3a3eb134e6792ce9403ea4188e5e79693de9e4c94e499db132be086400da79e6",
...> index: 0,
...> input: "0x6060604052341561000f57600080fd5b336000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055506102db8061005e6000396000f300606060405260043610610062576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff1680630900f01014610067578063445df0ac146100a05780638da5cb5b146100c9578063fdacd5761461011e575b600080fd5b341561007257600080fd5b61009e600480803573ffffffffffffffffffffffffffffffffffffffff16906020019091905050610141565b005b34156100ab57600080fd5b6100b3610224565b6040518082815260200191505060405180910390f35b34156100d457600080fd5b6100dc61022a565b604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b341561012957600080fd5b61013f600480803590602001909190505061024f565b005b60008060009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff161415610220578190508073ffffffffffffffffffffffffffffffffffffffff1663fdacd5766001546040518263ffffffff167c010000000000000000000000000000000000000000000000000000000002815260040180828152602001915050600060405180830381600087803b151561020b57600080fd5b6102c65a03f1151561021c57600080fd5b5050505b5050565b60015481565b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff1614156102ac57806001819055505b505600a165627a7a72305820a9c628775efbfbc17477a472413c01ee9b33881f550c59d21bee9928835c854b0029",
...> internal_transactions_indexed_at: DateTime.utc_now(),
...> nonce: 0,
...> r: 0xAD3733DF250C87556335FFE46C23E34DBAFFDE93097EF92F52C88632A40F0C75,
...> s: 0x72caddc0371451a58de2ca6ab64e0f586ccdb9465ff54e1c82564940e89291e3,
...> v: 0x8d,
...> value: 0
...> }
...> )
iex> changeset.valid?
false
iex> Keyword.get_values(changeset.errors, :status)
[{"can't be blank when the internal transactions have been fetched", []}]
The `error` can only be set with a specific error message when `status` is `:error`
iex> changeset = Explorer.Chain.Transaction.changeset(
@ -445,10 +380,9 @@ defmodule Explorer.Chain.Transaction do
transaction
|> cast(attrs, @required_attrs ++ @optional_attrs)
|> validate_required(@required_attrs)
|> validate_collated_or_pending()
|> validate_collated()
|> validate_error()
|> validate_status()
|> check_pending()
|> check_collated()
|> check_error()
|> check_status()
@ -592,29 +526,17 @@ defmodule Explorer.Chain.Transaction do
{collated_field, :"collated_#{collated_field}}"}
end)
@pending_fields_with_check @collated_fields
@pending_fields_with_validation @collated_fields ++ ~w(internal_transaction_indexed_at status)a
@pending_message "can't be set when the transaction is pending"
@pending_field_to_check Enum.into(@pending_fields_with_check, %{}, fn pending_field ->
{pending_field, :"pending_#{pending_field}}"}
end)
defp check_collated(%Changeset{} = changeset) do
check_constraints(changeset, @collated_field_to_check, @collated_message)
end
defp check_pending(%Changeset{} = changeset) do
check_constraints(changeset, @pending_field_to_check, @pending_message)
end
@error_message "can't be set when status is not :error"
defp check_error(%Changeset{} = changeset) do
check_constraint(changeset, :error, message: @error_message, name: :error)
changeset
end
@status_message "can't be blank when the internal transactions have been fetched"
@status_message "can't be set when the block_hash is unknown"
defp check_status(%Changeset{} = changeset) do
check_constraint(changeset, :status, message: @status_message, name: :status)
@ -632,22 +554,10 @@ defmodule Explorer.Chain.Transaction do
end)
end
defp validate_collated_or_pending(%Changeset{} = changeset) do
defp validate_collated(%Changeset{} = changeset) do
case Changeset.get_field(changeset, :block_hash) do
nil -> validate_collated_or_pending(changeset, @pending_fields_with_validation, &validate_pending/2)
%Hash{} -> validate_collated_or_pending(changeset, @collated_fields, &validate_collated/2)
end
end
defp validate_collated_or_pending(%Changeset{} = changeset, fields, field_validator)
when is_list(fields) and is_function(field_validator, 2) do
Enum.reduce(fields, changeset, field_validator)
end
defp validate_pending(field, %Changeset{} = changeset) when is_atom(field) do
case Changeset.get_field(changeset, field) do
%Hash{} -> Enum.reduce(@collated_fields, changeset, &validate_collated/2)
nil -> changeset
_ -> Changeset.add_error(changeset, field, @pending_message)
end
end
@ -667,9 +577,8 @@ defmodule Explorer.Chain.Transaction do
end
defp validate_status(%Changeset{} = changeset) do
# all other errors on status are handled by validate_pending
if Changeset.get_field(changeset, :internal_transactions_indexed_at) != nil and
Changeset.get_field(changeset, :status) == nil do
if Changeset.get_field(changeset, :block_hash) == nil and
Changeset.get_field(changeset, :status) != nil do
Changeset.add_error(changeset, :status, @status_message)
else
changeset

@ -0,0 +1,132 @@
defmodule Explorer.Repo.Migrations.ReduceTransactionStatusConstraint do
use Ecto.Migration
def up do
drop(
constraint(
:transactions,
:status
)
)
create(
constraint(
:transactions,
:status,
# NOTE: all checks on status are lifted except those regarding block_hash
# This is because of block invalidation, that causes transactions to be
# refetched while previous internal transactions still exist
check: """
(block_hash IS NULL AND status IS NULL) OR
(block_hash IS NOT NULL) OR
(status = 0 and error = 'dropped/replaced')
"""
)
)
drop(
constraint(
:transactions,
:error
)
)
create(
constraint(
:transactions,
:error,
# NOTE: all checks on error are lifted except when status is not 0, for
# the same reasons as above
check: """
(status = 0) OR
(status != 0 AND error IS NULL)
"""
)
)
end
def down do
drop(
constraint(
:transactions,
:status
)
)
create(
constraint(
:transactions,
:status,
# 0 - NULL
# 1 - NOT NULL
#
# | block_hash | internal_transactions_indexed_at | status | OK | description
# |------------|----------------------------------|--------|----|------------
# | 0 | 0 | 0 | 1 | pending
# | 0 | 0 | 1 | 0 | pending with status
# | 0 | 1 | 0 | 0 | pending with internal transactions
# | 0 | 1 | 1 | 0 | pending with internal transactions and status
# | 1 | 0 | 0 | 1 | pre-byzantium collated transaction without internal transactions
# | 1 | 0 | 1 | 1 | post-byzantium collated transaction without internal transactions
# | 1 | 1 | 0 | 0 | pre-byzantium collated transaction with internal transaction without status
# | 1 | 1 | 1 | 1 | pre- or post-byzantium collated transaction with internal transactions and status
#
# [Karnaugh map](https://en.wikipedia.org/wiki/Karnaugh_map)
# b \ is | 00 | 01 | 11 | 10 |
# -------|----|----|----|----|
# 0 | 1 | 0 | 0 | 0 |
# 1 | 1 | 1 | 1 | 0 |
#
# Simplification: ¬i·¬s + b·¬i + b·s
check: """
(internal_transactions_indexed_at IS NULL AND status IS NULL) OR
(block_hash IS NOT NULL AND internal_transactions_indexed_at IS NULL) OR
(block_hash IS NOT NULL AND status IS NOT NULL) OR
(status = 0 and error = 'dropped/replaced')
"""
)
)
drop(
constraint(
:transactions,
:error
)
)
create(
constraint(
:transactions,
:error,
# | status | internal_transactions_indexed_at | error | OK | description
# |--------|----------------------------------|----------|------------|------------
# | NULL | NULL | NULL | TRUE | pending or pre-byzantium collated
# | NULL | NULL | NOT NULL | FALSE | error cannot be known before internal transactions are indexed
# | NULL | NOT NULL | NULL | DON'T CARE | handled by `status` check
# | NULL | NOT NULL | NOT NULL | FALSE | error cannot be set unless status is known to be error (`0`)
# | 0 | NULL | NULL | TRUE | post-byzantium before internal transactions indexed
# | 0 | NULL | NOT NULL | FALSE | error cannot be set unless internal transactions are indexed
# | 0 | NOT NULL | NULL | FALSE | error MUST be set when status is error
# | 0 | NOT NULL | NOT NULL | TRUE | error is set when status is error
# | 1 | NULL | NULL | TRUE | post-byzantium before internal transactions indexed
# | 1 | NULL | NOT NULL | FALSE | error cannot be set when status is ok
# | 1 | NOT NULL | NULL | TRUE | error is not set when status is ok
# | 1 | NOT NULL | NOT NULL | FALSE | error cannot be set when status is ok
#
# Karnaugh map
# s \ ie | NULL, NULL | NULL, NOT NULL | NOT NULL, NOT NULL | NOT NULL, NULL |
# -------|------------|----------------|--------------------|----------------|
# NULL | TRUE | FALSE | FALSE | DON'T CARE |
# 0 | TRUE | FALSE | TRUE | FALSE |
# 1 | TRUE | FALSE | FALSE | TRUE |
#
check: """
(internal_transactions_indexed_at IS NULL AND error IS NULL) OR
(status = 0 AND internal_transactions_indexed_at IS NOT NULL AND error IS NOT NULL) OR
(status != 0 AND internal_transactions_indexed_at IS NOT NULL AND error IS NULL) OR
(status = 0 and error = 'dropped/replaced')
"""
)
)
end
end

@ -573,7 +573,7 @@ defmodule Explorer.Chain.ImportTest do
transaction =
:transaction
|> insert(error: nil, internal_transactions_indexed_at: nil, status: nil, from_address: from_address, status: 0)
|> insert(error: nil, internal_transactions_indexed_at: nil, status: nil, from_address: from_address)
|> with_block(block, status: :error)
internal_transacton =

Loading…
Cancel
Save