core: improve contextual information on core errors (#21869)
A lot of times when we hit 'core' errors, example: invalid tx, the information provided is insufficient. We miss several pieces of information: what account has nonce too high, and what transaction in that block was offending? This PR adds that information, using the new type of wrapped errors. It also adds a testcase which (partly) verifies the output from the errors. The first commit changes all usage of direct equality-checks on core errors, into using errors.Is. The second commit adds contextual information. This wraps most of the core errors with more information, and also wraps it one more time in stateprocessor, to further provide tx index and tx hash, if such a tx is encoutered in a block. The third commit uses the chainmaker to try to generate chains with such errors in them, thus triggering the errors and checking that the generated string meets expectations.
This commit is contained in:
parent
62cedb3aab
commit
7770e41cb5
@ -479,7 +479,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs
|
|||||||
b.pendingState.RevertToSnapshot(snapshot)
|
b.pendingState.RevertToSnapshot(snapshot)
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if err == core.ErrIntrinsicGas {
|
if errors.Is(err, core.ErrIntrinsicGas) {
|
||||||
return true, nil, nil // Special case, raise gas limit
|
return true, nil, nil // Special case, raise gas limit
|
||||||
}
|
}
|
||||||
return true, nil, err // Bail out
|
return true, nil, err // Bail out
|
||||||
|
@ -17,6 +17,7 @@
|
|||||||
package core
|
package core
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io/ioutil"
|
"io/ioutil"
|
||||||
"math/big"
|
"math/big"
|
||||||
@ -468,7 +469,7 @@ func testBadHashes(t *testing.T, full bool) {
|
|||||||
|
|
||||||
_, err = blockchain.InsertHeaderChain(headers, 1)
|
_, err = blockchain.InsertHeaderChain(headers, 1)
|
||||||
}
|
}
|
||||||
if err != ErrBlacklistedHash {
|
if !errors.Is(err, ErrBlacklistedHash) {
|
||||||
t.Errorf("error mismatch: have: %v, want: %v", err, ErrBlacklistedHash)
|
t.Errorf("error mismatch: have: %v, want: %v", err, ErrBlacklistedHash)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -17,6 +17,8 @@
|
|||||||
package core
|
package core
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
|
|
||||||
"github.com/ethereum/go-ethereum/common"
|
"github.com/ethereum/go-ethereum/common"
|
||||||
"github.com/ethereum/go-ethereum/consensus"
|
"github.com/ethereum/go-ethereum/consensus"
|
||||||
"github.com/ethereum/go-ethereum/consensus/misc"
|
"github.com/ethereum/go-ethereum/consensus/misc"
|
||||||
@ -76,7 +78,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
|
|||||||
statedb.Prepare(tx.Hash(), block.Hash(), i)
|
statedb.Prepare(tx.Hash(), block.Hash(), i)
|
||||||
receipt, err := applyTransaction(msg, p.config, p.bc, nil, gp, statedb, header, tx, usedGas, vmenv)
|
receipt, err := applyTransaction(msg, p.config, p.bc, nil, gp, statedb, header, tx, usedGas, vmenv)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, nil, 0, err
|
return nil, nil, 0, fmt.Errorf("could not apply tx %d [%v]: %w", i, tx.Hash().Hex(), err)
|
||||||
}
|
}
|
||||||
receipts = append(receipts, receipt)
|
receipts = append(receipts, receipt)
|
||||||
allLogs = append(allLogs, receipt.Logs...)
|
allLogs = append(allLogs, receipt.Logs...)
|
||||||
|
152
core/state_processor_test.go
Normal file
152
core/state_processor_test.go
Normal file
@ -0,0 +1,152 @@
|
|||||||
|
// Copyright 2020 The go-ethereum Authors
|
||||||
|
// This file is part of the go-ethereum library.
|
||||||
|
//
|
||||||
|
// The go-ethereum library is free software: you can redistribute it and/or modify
|
||||||
|
// it under the terms of the GNU Lesser General Public License as published by
|
||||||
|
// the Free Software Foundation, either version 3 of the License, or
|
||||||
|
// (at your option) any later version.
|
||||||
|
//
|
||||||
|
// The go-ethereum library is distributed in the hope that it will be useful,
|
||||||
|
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||||
|
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||||
|
// GNU Lesser General Public License for more details.
|
||||||
|
//
|
||||||
|
// You should have received a copy of the GNU Lesser General Public License
|
||||||
|
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
|
||||||
|
package core
|
||||||
|
|
||||||
|
import (
|
||||||
|
"math/big"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/ethereum/go-ethereum/common"
|
||||||
|
"github.com/ethereum/go-ethereum/consensus"
|
||||||
|
"github.com/ethereum/go-ethereum/consensus/ethash"
|
||||||
|
"github.com/ethereum/go-ethereum/core/rawdb"
|
||||||
|
"github.com/ethereum/go-ethereum/core/types"
|
||||||
|
"github.com/ethereum/go-ethereum/core/vm"
|
||||||
|
"github.com/ethereum/go-ethereum/crypto"
|
||||||
|
"github.com/ethereum/go-ethereum/params"
|
||||||
|
"github.com/ethereum/go-ethereum/trie"
|
||||||
|
"golang.org/x/crypto/sha3"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestStateProcessorErrors tests the output from the 'core' errors
|
||||||
|
// as defined in core/error.go. These errors are generated when the
|
||||||
|
// blockchain imports bad blocks, meaning blocks which have valid headers but
|
||||||
|
// contain invalid transactions
|
||||||
|
func TestStateProcessorErrors(t *testing.T) {
|
||||||
|
var (
|
||||||
|
signer = types.HomesteadSigner{}
|
||||||
|
testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
|
||||||
|
db = rawdb.NewMemoryDatabase()
|
||||||
|
gspec = &Genesis{
|
||||||
|
Config: params.TestChainConfig,
|
||||||
|
}
|
||||||
|
genesis = gspec.MustCommit(db)
|
||||||
|
blockchain, _ = NewBlockChain(db, nil, gspec.Config, ethash.NewFaker(), vm.Config{}, nil, nil)
|
||||||
|
)
|
||||||
|
defer blockchain.Stop()
|
||||||
|
var makeTx = func(nonce uint64, to common.Address, amount *big.Int, gasLimit uint64, gasPrice *big.Int, data []byte) *types.Transaction {
|
||||||
|
tx, _ := types.SignTx(types.NewTransaction(nonce, to, amount, gasLimit, gasPrice, data), signer, testKey)
|
||||||
|
return tx
|
||||||
|
}
|
||||||
|
for i, tt := range []struct {
|
||||||
|
txs []*types.Transaction
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
txs: []*types.Transaction{
|
||||||
|
makeTx(0, common.Address{}, big.NewInt(0), params.TxGas, nil, nil),
|
||||||
|
makeTx(0, common.Address{}, big.NewInt(0), params.TxGas, nil, nil),
|
||||||
|
},
|
||||||
|
want: "could not apply tx 1 [0x36bfa6d14f1cd35a1be8cc2322982a595fabc0e799f09c1de3bad7bd5b1f7626]: nonce too low: address 0x71562b71999873DB5b286dF957af199Ec94617F7, tx: 0 state: 1",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
txs: []*types.Transaction{
|
||||||
|
makeTx(100, common.Address{}, big.NewInt(0), params.TxGas, nil, nil),
|
||||||
|
},
|
||||||
|
want: "could not apply tx 0 [0x51cd272d41ef6011d8138e18bf4043797aca9b713c7d39a97563f9bbe6bdbe6f]: nonce too high: address 0x71562b71999873DB5b286dF957af199Ec94617F7, tx: 100 state: 0",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
txs: []*types.Transaction{
|
||||||
|
makeTx(0, common.Address{}, big.NewInt(0), 21000000, nil, nil),
|
||||||
|
},
|
||||||
|
want: "could not apply tx 0 [0x54c58b530824b0bb84b7a98183f08913b5d74e1cebc368515ef3c65edf8eb56a]: gas limit reached",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
txs: []*types.Transaction{
|
||||||
|
makeTx(0, common.Address{}, big.NewInt(1), params.TxGas, nil, nil),
|
||||||
|
},
|
||||||
|
want: "could not apply tx 0 [0x3094b17498940d92b13baccf356ce8bfd6f221e926abc903d642fa1466c5b50e]: insufficient funds for transfer: address 0x71562b71999873DB5b286dF957af199Ec94617F7",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
txs: []*types.Transaction{
|
||||||
|
makeTx(0, common.Address{}, big.NewInt(0), params.TxGas, big.NewInt(0xffffff), nil),
|
||||||
|
},
|
||||||
|
want: "could not apply tx 0 [0xaa3f7d86802b1f364576d9071bf231e31d61b392d306831ac9cf706ff5371ce0]: insufficient funds for gas * price + value: address 0x71562b71999873DB5b286dF957af199Ec94617F7 have 0 want 352321515000",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
txs: []*types.Transaction{
|
||||||
|
makeTx(0, common.Address{}, big.NewInt(0), params.TxGas, nil, nil),
|
||||||
|
makeTx(1, common.Address{}, big.NewInt(0), params.TxGas, nil, nil),
|
||||||
|
makeTx(2, common.Address{}, big.NewInt(0), params.TxGas, nil, nil),
|
||||||
|
makeTx(3, common.Address{}, big.NewInt(0), params.TxGas-1000, big.NewInt(0), nil),
|
||||||
|
},
|
||||||
|
want: "could not apply tx 3 [0x836fab5882205362680e49b311a20646de03b630920f18ec6ee3b111a2cf6835]: intrinsic gas too low: have 20000, want 21000",
|
||||||
|
},
|
||||||
|
// The last 'core' error is ErrGasUintOverflow: "gas uint64 overflow", but in order to
|
||||||
|
// trigger that one, we'd have to allocate a _huge_ chunk of data, such that the
|
||||||
|
// multiplication len(data) +gas_per_byte overflows uint64. Not testable at the moment
|
||||||
|
} {
|
||||||
|
block := GenerateBadBlock(genesis, ethash.NewFaker(), tt.txs)
|
||||||
|
_, err := blockchain.InsertChain(types.Blocks{block})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("block imported without errors")
|
||||||
|
}
|
||||||
|
if have, want := err.Error(), tt.want; have != want {
|
||||||
|
t.Errorf("test %d:\nhave \"%v\"\nwant \"%v\"\n", i, have, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// GenerateBadBlock constructs a "block" which contains the transactions. The transactions are not expected to be
|
||||||
|
// valid, and no proper post-state can be made. But from the perspective of the blockchain, the block is sufficiently
|
||||||
|
// valid to be considered for import:
|
||||||
|
// - valid pow (fake), ancestry, difficulty, gaslimit etc
|
||||||
|
func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Transactions) *types.Block {
|
||||||
|
header := &types.Header{
|
||||||
|
ParentHash: parent.Hash(),
|
||||||
|
Coinbase: parent.Coinbase(),
|
||||||
|
Difficulty: engine.CalcDifficulty(&fakeChainReader{params.TestChainConfig}, parent.Time()+10, &types.Header{
|
||||||
|
Number: parent.Number(),
|
||||||
|
Time: parent.Time(),
|
||||||
|
Difficulty: parent.Difficulty(),
|
||||||
|
UncleHash: parent.UncleHash(),
|
||||||
|
}),
|
||||||
|
GasLimit: CalcGasLimit(parent, parent.GasLimit(), parent.GasLimit()),
|
||||||
|
Number: new(big.Int).Add(parent.Number(), common.Big1),
|
||||||
|
Time: parent.Time() + 10,
|
||||||
|
UncleHash: types.EmptyUncleHash,
|
||||||
|
}
|
||||||
|
var receipts []*types.Receipt
|
||||||
|
|
||||||
|
// The post-state result doesn't need to be correct (this is a bad block), but we do need something there
|
||||||
|
// Preferably something unique. So let's use a combo of blocknum + txhash
|
||||||
|
hasher := sha3.NewLegacyKeccak256()
|
||||||
|
hasher.Write(header.Number.Bytes())
|
||||||
|
var cumulativeGas uint64
|
||||||
|
for _, tx := range txs {
|
||||||
|
txh := tx.Hash()
|
||||||
|
hasher.Write(txh[:])
|
||||||
|
receipt := types.NewReceipt(nil, false, cumulativeGas+tx.Gas())
|
||||||
|
receipt.TxHash = tx.Hash()
|
||||||
|
receipt.GasUsed = tx.Gas()
|
||||||
|
receipts = append(receipts, receipt)
|
||||||
|
cumulativeGas += tx.Gas()
|
||||||
|
}
|
||||||
|
header.Root = common.BytesToHash(hasher.Sum(nil))
|
||||||
|
// Assemble and return the final block for sealing
|
||||||
|
return types.NewBlock(header, txs, nil, receipts, new(trie.Trie))
|
||||||
|
}
|
@ -17,6 +17,7 @@
|
|||||||
package core
|
package core
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
"math"
|
"math"
|
||||||
"math/big"
|
"math/big"
|
||||||
|
|
||||||
@ -174,8 +175,8 @@ func (st *StateTransition) to() common.Address {
|
|||||||
|
|
||||||
func (st *StateTransition) buyGas() error {
|
func (st *StateTransition) buyGas() error {
|
||||||
mgval := new(big.Int).Mul(new(big.Int).SetUint64(st.msg.Gas()), st.gasPrice)
|
mgval := new(big.Int).Mul(new(big.Int).SetUint64(st.msg.Gas()), st.gasPrice)
|
||||||
if st.state.GetBalance(st.msg.From()).Cmp(mgval) < 0 {
|
if have, want := st.state.GetBalance(st.msg.From()), mgval; have.Cmp(want) < 0 {
|
||||||
return ErrInsufficientFunds
|
return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From().Hex(), have, want)
|
||||||
}
|
}
|
||||||
if err := st.gp.SubGas(st.msg.Gas()); err != nil {
|
if err := st.gp.SubGas(st.msg.Gas()); err != nil {
|
||||||
return err
|
return err
|
||||||
@ -190,11 +191,13 @@ func (st *StateTransition) buyGas() error {
|
|||||||
func (st *StateTransition) preCheck() error {
|
func (st *StateTransition) preCheck() error {
|
||||||
// Make sure this transaction's nonce is correct.
|
// Make sure this transaction's nonce is correct.
|
||||||
if st.msg.CheckNonce() {
|
if st.msg.CheckNonce() {
|
||||||
nonce := st.state.GetNonce(st.msg.From())
|
stNonce := st.state.GetNonce(st.msg.From())
|
||||||
if nonce < st.msg.Nonce() {
|
if msgNonce := st.msg.Nonce(); stNonce < msgNonce {
|
||||||
return ErrNonceTooHigh
|
return fmt.Errorf("%w: address %v, tx: %d state: %d", ErrNonceTooHigh,
|
||||||
} else if nonce > st.msg.Nonce() {
|
st.msg.From().Hex(), msgNonce, stNonce)
|
||||||
return ErrNonceTooLow
|
} else if stNonce > msgNonce {
|
||||||
|
return fmt.Errorf("%w: address %v, tx: %d state: %d", ErrNonceTooLow,
|
||||||
|
st.msg.From().Hex(), msgNonce, stNonce)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return st.buyGas()
|
return st.buyGas()
|
||||||
@ -240,13 +243,13 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if st.gas < gas {
|
if st.gas < gas {
|
||||||
return nil, ErrIntrinsicGas
|
return nil, fmt.Errorf("%w: have %d, want %d", ErrIntrinsicGas, st.gas, gas)
|
||||||
}
|
}
|
||||||
st.gas -= gas
|
st.gas -= gas
|
||||||
|
|
||||||
// Check clause 6
|
// Check clause 6
|
||||||
if msg.Value().Sign() > 0 && !st.evm.Context.CanTransfer(st.state, msg.From(), msg.Value()) {
|
if msg.Value().Sign() > 0 && !st.evm.Context.CanTransfer(st.state, msg.From(), msg.Value()) {
|
||||||
return nil, ErrInsufficientFundsForTransfer
|
return nil, fmt.Errorf("%w: address %v", ErrInsufficientFundsForTransfer, msg.From().Hex())
|
||||||
}
|
}
|
||||||
var (
|
var (
|
||||||
ret []byte
|
ret []byte
|
||||||
|
@ -242,7 +242,7 @@ func TestInvalidTransactions(t *testing.T) {
|
|||||||
from, _ := deriveSender(tx)
|
from, _ := deriveSender(tx)
|
||||||
|
|
||||||
pool.currentState.AddBalance(from, big.NewInt(1))
|
pool.currentState.AddBalance(from, big.NewInt(1))
|
||||||
if err := pool.AddRemote(tx); err != ErrInsufficientFunds {
|
if err := pool.AddRemote(tx); !errors.Is(err, ErrInsufficientFunds) {
|
||||||
t.Error("expected", ErrInsufficientFunds)
|
t.Error("expected", ErrInsufficientFunds)
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -255,7 +255,7 @@ func TestInvalidTransactions(t *testing.T) {
|
|||||||
pool.currentState.SetNonce(from, 1)
|
pool.currentState.SetNonce(from, 1)
|
||||||
pool.currentState.AddBalance(from, big.NewInt(0xffffffffffffff))
|
pool.currentState.AddBalance(from, big.NewInt(0xffffffffffffff))
|
||||||
tx = transaction(0, 100000, key)
|
tx = transaction(0, 100000, key)
|
||||||
if err := pool.AddRemote(tx); err != ErrNonceTooLow {
|
if err := pool.AddRemote(tx); !errors.Is(err, ErrNonceTooLow) {
|
||||||
t.Error("expected", ErrNonceTooLow)
|
t.Error("expected", ErrNonceTooLow)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -18,6 +18,7 @@ package light
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"math/big"
|
"math/big"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@ -321,7 +322,7 @@ func TestBadHeaderHashes(t *testing.T) {
|
|||||||
var err error
|
var err error
|
||||||
headers := makeHeaderChainWithDiff(bc.genesisBlock, []int{1, 2, 4}, 10)
|
headers := makeHeaderChainWithDiff(bc.genesisBlock, []int{1, 2, 4}, 10)
|
||||||
core.BadHashes[headers[2].Hash()] = true
|
core.BadHashes[headers[2].Hash()] = true
|
||||||
if _, err = bc.InsertHeaderChain(headers, 1); err != core.ErrBlacklistedHash {
|
if _, err = bc.InsertHeaderChain(headers, 1); !errors.Is(err, core.ErrBlacklistedHash) {
|
||||||
t.Errorf("error mismatch: have: %v, want %v", err, core.ErrBlacklistedHash)
|
t.Errorf("error mismatch: have: %v, want %v", err, core.ErrBlacklistedHash)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -793,23 +793,23 @@ func (w *worker) commitTransactions(txs *types.TransactionsByPriceAndNonce, coin
|
|||||||
w.current.state.Prepare(tx.Hash(), common.Hash{}, w.current.tcount)
|
w.current.state.Prepare(tx.Hash(), common.Hash{}, w.current.tcount)
|
||||||
|
|
||||||
logs, err := w.commitTransaction(tx, coinbase)
|
logs, err := w.commitTransaction(tx, coinbase)
|
||||||
switch err {
|
switch {
|
||||||
case core.ErrGasLimitReached:
|
case errors.Is(err, core.ErrGasLimitReached):
|
||||||
// Pop the current out-of-gas transaction without shifting in the next from the account
|
// Pop the current out-of-gas transaction without shifting in the next from the account
|
||||||
log.Trace("Gas limit exceeded for current block", "sender", from)
|
log.Trace("Gas limit exceeded for current block", "sender", from)
|
||||||
txs.Pop()
|
txs.Pop()
|
||||||
|
|
||||||
case core.ErrNonceTooLow:
|
case errors.Is(err, core.ErrNonceTooLow):
|
||||||
// New head notification data race between the transaction pool and miner, shift
|
// New head notification data race between the transaction pool and miner, shift
|
||||||
log.Trace("Skipping transaction with low nonce", "sender", from, "nonce", tx.Nonce())
|
log.Trace("Skipping transaction with low nonce", "sender", from, "nonce", tx.Nonce())
|
||||||
txs.Shift()
|
txs.Shift()
|
||||||
|
|
||||||
case core.ErrNonceTooHigh:
|
case errors.Is(err, core.ErrNonceTooHigh):
|
||||||
// Reorg notification data race between the transaction pool and miner, skip account =
|
// Reorg notification data race between the transaction pool and miner, skip account =
|
||||||
log.Trace("Skipping account with hight nonce", "sender", from, "nonce", tx.Nonce())
|
log.Trace("Skipping account with hight nonce", "sender", from, "nonce", tx.Nonce())
|
||||||
txs.Pop()
|
txs.Pop()
|
||||||
|
|
||||||
case nil:
|
case errors.Is(err, nil):
|
||||||
// Everything ok, collect the logs and shift in the next transaction from the same account
|
// Everything ok, collect the logs and shift in the next transaction from the same account
|
||||||
coalescedLogs = append(coalescedLogs, logs...)
|
coalescedLogs = append(coalescedLogs, logs...)
|
||||||
w.current.tcount++
|
w.current.tcount++
|
||||||
|
Loading…
Reference in New Issue
Block a user