Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: improve contextual information on core errors #21869

Merged
merged 3 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion accounts/abi/bind/backends/simulated.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs
b.pendingState.RevertToSnapshot(snapshot)

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, err // Bail out
Expand Down
3 changes: 2 additions & 1 deletion core/blockchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package core

import (
"errors"
"fmt"
"io/ioutil"
"math/big"
Expand Down Expand Up @@ -468,7 +469,7 @@ func testBadHashes(t *testing.T, full bool) {

_, err = blockchain.InsertHeaderChain(headers, 1)
}
if err != ErrBlacklistedHash {
if !errors.Is(err, ErrBlacklistedHash) {
t.Errorf("error mismatch: have: %v, want: %v", err, ErrBlacklistedHash)
}
}
Expand Down
4 changes: 3 additions & 1 deletion core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package core

import (
"fmt"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/consensus"
"github.com/ethereum/go-ethereum/consensus/misc"
Expand Down Expand Up @@ -76,7 +78,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
statedb.Prepare(tx.Hash(), block.Hash(), i)
receipt, err := applyTransaction(msg, p.config, p.bc, nil, gp, statedb, header, tx, usedGas, vmenv)
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)
allLogs = append(allLogs, receipt.Logs...)
Expand Down
152 changes: 152 additions & 0 deletions core/state_processor_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
21 changes: 12 additions & 9 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package core

import (
"fmt"
"math"
"math/big"

Expand Down Expand Up @@ -174,8 +175,8 @@ func (st *StateTransition) to() common.Address {

func (st *StateTransition) buyGas() error {
mgval := new(big.Int).Mul(new(big.Int).SetUint64(st.msg.Gas()), st.gasPrice)
if st.state.GetBalance(st.msg.From()).Cmp(mgval) < 0 {
return ErrInsufficientFunds
if have, want := st.state.GetBalance(st.msg.From()), mgval; have.Cmp(want) < 0 {
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 {
return err
Expand All @@ -190,11 +191,13 @@ func (st *StateTransition) buyGas() error {
func (st *StateTransition) preCheck() error {
// Make sure this transaction's nonce is correct.
if st.msg.CheckNonce() {
nonce := st.state.GetNonce(st.msg.From())
if nonce < st.msg.Nonce() {
return ErrNonceTooHigh
} else if nonce > st.msg.Nonce() {
return ErrNonceTooLow
stNonce := st.state.GetNonce(st.msg.From())
if msgNonce := st.msg.Nonce(); stNonce < msgNonce {
return fmt.Errorf("%w: address %v, tx: %d state: %d", ErrNonceTooHigh,
st.msg.From().Hex(), msgNonce, stNonce)
} else if stNonce > msgNonce {
return fmt.Errorf("%w: address %v, tx: %d state: %d", ErrNonceTooLow,
st.msg.From().Hex(), msgNonce, stNonce)
}
}
return st.buyGas()
Expand Down Expand Up @@ -240,13 +243,13 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
return nil, err
}
if st.gas < gas {
return nil, ErrIntrinsicGas
return nil, fmt.Errorf("%w: have %d, want %d", ErrIntrinsicGas, st.gas, gas)
}
st.gas -= gas

// Check clause 6
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 (
ret []byte
Expand Down
4 changes: 2 additions & 2 deletions core/tx_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func TestInvalidTransactions(t *testing.T) {
from, _ := deriveSender(tx)

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)
}

Expand All @@ -255,7 +255,7 @@ func TestInvalidTransactions(t *testing.T) {
pool.currentState.SetNonce(from, 1)
pool.currentState.AddBalance(from, big.NewInt(0xffffffffffffff))
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)
}

Expand Down
3 changes: 2 additions & 1 deletion light/lightchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package light

import (
"context"
"errors"
"math/big"
"testing"

Expand Down Expand Up @@ -321,7 +322,7 @@ func TestBadHeaderHashes(t *testing.T) {
var err error
headers := makeHeaderChainWithDiff(bc.genesisBlock, []int{1, 2, 4}, 10)
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)
}
}
Expand Down
10 changes: 5 additions & 5 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -793,23 +793,23 @@ func (w *worker) commitTransactions(txs *types.TransactionsByPriceAndNonce, coin
w.current.state.Prepare(tx.Hash(), common.Hash{}, w.current.tcount)

logs, err := w.commitTransaction(tx, coinbase)
switch err {
case core.ErrGasLimitReached:
switch {
case errors.Is(err, core.ErrGasLimitReached):
// 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)
txs.Pop()

case core.ErrNonceTooLow:
case errors.Is(err, core.ErrNonceTooLow):
// New head notification data race between the transaction pool and miner, shift
log.Trace("Skipping transaction with low nonce", "sender", from, "nonce", tx.Nonce())
txs.Shift()

case core.ErrNonceTooHigh:
case errors.Is(err, core.ErrNonceTooHigh):
// Reorg notification data race between the transaction pool and miner, skip account =
log.Trace("Skipping account with hight nonce", "sender", from, "nonce", tx.Nonce())
txs.Pop()

case nil:
case errors.Is(err, nil):
// Everything ok, collect the logs and shift in the next transaction from the same account
coalescedLogs = append(coalescedLogs, logs...)
w.current.tcount++
Expand Down