mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-07-18 02:40:45 +00:00
- Fix "invalid transaction v, r, s values" error when calling debug_traceCall on BlockSigners contract (0x89) - Fix "nonce too low" error by respecting Message.SkipNonceChecks flag - ApplySignTransaction now accepts *Message and uses msg.From directly - Add fallback to signature recovery for real transactions - Skip nonce validation when SkipNonceChecks=true (for traceCall) - Add comprehensive unit tests for both scenarios Root cause: BlockSigners uses special fast-path that calls ApplySignTransaction directly, which previously attempted signature recovery on unsigned transactions from debug_traceCall. Fixes #1870
This commit is contained in:
parent
1a9935625f
commit
8c380e76f5
2 changed files with 195 additions and 10 deletions
|
|
@ -542,17 +542,32 @@ func ApplySignTransaction(msg *Message, config *params.ChainConfig, statedb *sta
|
||||||
} else {
|
} else {
|
||||||
root = statedb.IntermediateRoot(config.IsEIP158(blockNumber)).Bytes()
|
root = statedb.IntermediateRoot(config.IsEIP158(blockNumber)).Bytes()
|
||||||
}
|
}
|
||||||
from, err := types.Sender(types.MakeSigner(config, blockNumber), tx)
|
// Defensive fallback: msg.From should already be populated by the caller through one of these paths:
|
||||||
if err != nil {
|
// 1. Normal block processing: TransactionToMessage recovers from via signature (types.Sender)
|
||||||
return nil, 0, false, err
|
// 2. TraceCall/debug_traceCall: args.ToMessage directly uses the provided args.From parameter
|
||||||
|
// This zero-check should rarely execute. If it does, signature recovery is attempted as a last resort,
|
||||||
|
// which will fail if the transaction lacks a valid signature (e.g., unsigned simulation transactions).
|
||||||
|
from := msg.From
|
||||||
|
if from.IsZero() {
|
||||||
|
var err error
|
||||||
|
from, err = types.Sender(types.MakeSigner(config, blockNumber), tx)
|
||||||
|
if err != nil {
|
||||||
|
return nil, 0, false, err
|
||||||
|
}
|
||||||
}
|
}
|
||||||
nonce := statedb.GetNonce(from)
|
nonce := statedb.GetNonce(from)
|
||||||
if nonce < tx.Nonce() {
|
// For tracing/simulation calls (e.g., debug_traceCall), SkipNonceChecks is true,
|
||||||
return nil, 0, false, ErrNonceTooHigh
|
// so nonce checks and incrementing are skipped, allowing the transaction to be processed
|
||||||
} else if nonce > tx.Nonce() {
|
// regardless of the current account nonce. For regular transactions, nonce checks are enforced.
|
||||||
return nil, 0, false, ErrNonceTooLow
|
if !msg.SkipNonceChecks {
|
||||||
|
if nonce < tx.Nonce() {
|
||||||
|
return nil, 0, false, ErrNonceTooHigh
|
||||||
|
} else if nonce > tx.Nonce() {
|
||||||
|
return nil, 0, false, ErrNonceTooLow
|
||||||
|
}
|
||||||
|
// Only increment the nonce for real transactions.
|
||||||
|
statedb.SetNonce(from, nonce+1)
|
||||||
}
|
}
|
||||||
statedb.SetNonce(from, nonce+1)
|
|
||||||
// Create a new receipt for the transaction, storing the intermediate root and gas used by the tx
|
// Create a new receipt for the transaction, storing the intermediate root and gas used by the tx
|
||||||
// based on the eip phase, we're passing whether the root touch-delete accounts.
|
// based on the eip phase, we're passing whether the root touch-delete accounts.
|
||||||
receipt = types.NewReceipt(root, false, *usedGas)
|
receipt = types.NewReceipt(root, false, *usedGas)
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,7 @@ import (
|
||||||
"math/big"
|
"math/big"
|
||||||
"reflect"
|
"reflect"
|
||||||
"slices"
|
"slices"
|
||||||
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
@ -248,8 +249,6 @@ func TestTraceCall(t *testing.T) {
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
uintPtr := func(i int) *hexutil.Uint { x := hexutil.Uint(i); return &x }
|
|
||||||
|
|
||||||
defer backend.teardown()
|
defer backend.teardown()
|
||||||
api := NewAPI(backend)
|
api := NewAPI(backend)
|
||||||
var testSuite = []struct {
|
var testSuite = []struct {
|
||||||
|
|
@ -838,3 +837,174 @@ func TestTraceChain(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestTraceCallBlockSigners tests tracing calls to the BlockSignersBinary contract (0x89)
|
||||||
|
// This regression test ensures that debug_traceCall works for calls to system contracts
|
||||||
|
// that previously failed with "invalid transaction v, r, s values" error.
|
||||||
|
func TestTraceCallBlockSigners(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// Initialize test accounts
|
||||||
|
accounts := newAccounts(1)
|
||||||
|
config := *params.TestChainConfig
|
||||||
|
genesis := &core.Genesis{
|
||||||
|
Config: &config,
|
||||||
|
Alloc: types.GenesisAlloc{
|
||||||
|
accounts[0].addr: {Balance: big.NewInt(params.Ether)},
|
||||||
|
common.BlockSignersBinary: {Balance: big.NewInt(0)}, // System contract
|
||||||
|
},
|
||||||
|
}
|
||||||
|
backend := newTestBackend(t, 1, genesis, func(i int, b *core.BlockGen) {
|
||||||
|
// Just create an empty block
|
||||||
|
})
|
||||||
|
defer backend.teardown()
|
||||||
|
|
||||||
|
api := NewAPI(backend)
|
||||||
|
blockSignersAddr := common.BlockSignersBinary
|
||||||
|
|
||||||
|
// Test data: e341eaa4 is a function selector + some data (from the bug report)
|
||||||
|
testData := hexutil.MustDecode("0xe341eaa40000000000000000000000000000000000000000000000000000000005c9212eaa6f69addff0a2d21ec701940a81975992a67dc4b01aa89e039795852705edb1")
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
call ethapi.TransactionArgs
|
||||||
|
config *TraceCallConfig
|
||||||
|
expectErr bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "Call to BlockSignersBinary with default tracer",
|
||||||
|
call: ethapi.TransactionArgs{
|
||||||
|
From: &accounts[0].addr,
|
||||||
|
To: &blockSignersAddr,
|
||||||
|
Value: (*hexutil.Big)(big.NewInt(0)),
|
||||||
|
Gas: uint64Ptr(200000),
|
||||||
|
Data: (*hexutil.Bytes)(&testData),
|
||||||
|
},
|
||||||
|
config: nil,
|
||||||
|
expectErr: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
blockNum := rpc.BlockNumber(1)
|
||||||
|
result, err := api.TraceCall(context.Background(), tc.call, rpc.BlockNumberOrHash{BlockNumber: &blockNum}, tc.config)
|
||||||
|
|
||||||
|
if tc.expectErr {
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("expected error but got none")
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("unexpected error: %v", err)
|
||||||
|
} else if result == nil {
|
||||||
|
t.Errorf("expected result but got nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTraceCallBlockSignersNonceValidation tests that traceCall skips nonce validation
|
||||||
|
// This regression test ensures that debug_traceCall works even when the account nonce
|
||||||
|
// doesn't match the transaction nonce (which would fail with "nonce too low" in real execution).
|
||||||
|
func TestTraceCallBlockSignersNonceValidation(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// Initialize test accounts
|
||||||
|
accounts := newAccounts(1)
|
||||||
|
config := *params.TestChainConfig
|
||||||
|
genesis := &core.Genesis{
|
||||||
|
Config: &config,
|
||||||
|
Alloc: types.GenesisAlloc{
|
||||||
|
accounts[0].addr: {Balance: big.NewInt(params.Ether), Nonce: 5}, // Account has nonce 5
|
||||||
|
common.BlockSignersBinary: {Balance: big.NewInt(0)}, // System contract
|
||||||
|
},
|
||||||
|
}
|
||||||
|
backend := newTestBackend(t, 1, genesis, func(i int, b *core.BlockGen) {
|
||||||
|
// Just create an empty block
|
||||||
|
})
|
||||||
|
defer backend.teardown()
|
||||||
|
|
||||||
|
api := NewAPI(backend)
|
||||||
|
blockSignersAddr := common.BlockSignersBinary
|
||||||
|
testData := hexutil.MustDecode("0xe341eaa40000000000000000000000000000000000000000000000000000000005c9212eaa6f69addff0a2d21ec701940a81975992a67dc4b01aa89e039795852705edb1")
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
call ethapi.TransactionArgs
|
||||||
|
expectErr bool
|
||||||
|
errMsg string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "TraceCall with nonce=0 (lower than account nonce=5) should succeed",
|
||||||
|
call: ethapi.TransactionArgs{
|
||||||
|
From: &accounts[0].addr,
|
||||||
|
To: &blockSignersAddr,
|
||||||
|
Value: (*hexutil.Big)(big.NewInt(0)),
|
||||||
|
Gas: uint64Ptr(200000),
|
||||||
|
Nonce: uint64Ptr(0), // Nonce 0, but account has nonce 5
|
||||||
|
Data: (*hexutil.Bytes)(&testData),
|
||||||
|
},
|
||||||
|
expectErr: false,
|
||||||
|
errMsg: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "TraceCall with nonce=10 (higher than account nonce=5) should succeed",
|
||||||
|
call: ethapi.TransactionArgs{
|
||||||
|
From: &accounts[0].addr,
|
||||||
|
To: &blockSignersAddr,
|
||||||
|
Value: (*hexutil.Big)(big.NewInt(0)),
|
||||||
|
Gas: uint64Ptr(200000),
|
||||||
|
Nonce: uint64Ptr(10), // Nonce 10, but account has nonce 5
|
||||||
|
Data: (*hexutil.Bytes)(&testData),
|
||||||
|
},
|
||||||
|
expectErr: false,
|
||||||
|
errMsg: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "TraceCall without explicit nonce should succeed",
|
||||||
|
call: ethapi.TransactionArgs{
|
||||||
|
From: &accounts[0].addr,
|
||||||
|
To: &blockSignersAddr,
|
||||||
|
Value: (*hexutil.Big)(big.NewInt(0)),
|
||||||
|
Gas: uint64Ptr(200000),
|
||||||
|
// Nonce not specified - will use 0
|
||||||
|
Data: (*hexutil.Bytes)(&testData),
|
||||||
|
},
|
||||||
|
expectErr: false,
|
||||||
|
errMsg: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
blockNum := rpc.BlockNumber(1)
|
||||||
|
result, err := api.TraceCall(context.Background(), tc.call, rpc.BlockNumberOrHash{BlockNumber: &blockNum}, nil)
|
||||||
|
|
||||||
|
if tc.expectErr {
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("expected error but got none")
|
||||||
|
} else if tc.errMsg != "" && !strings.Contains(err.Error(), tc.errMsg) {
|
||||||
|
t.Errorf("expected error containing %q, got: %v", tc.errMsg, err)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("unexpected error: %v", err)
|
||||||
|
} else if result == nil {
|
||||||
|
t.Errorf("expected result but got nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func uintPtr(i int) *hexutil.Uint {
|
||||||
|
x := hexutil.Uint(i)
|
||||||
|
return &x
|
||||||
|
}
|
||||||
|
|
||||||
|
func uint64Ptr(u uint64) *hexutil.Uint64 {
|
||||||
|
ret := hexutil.Uint64(u)
|
||||||
|
return &ret
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue