Skip to content

Commit 7dd662e

Browse files
committed
fix(core,txpool,ethapi): reject overflowing tx values
Reject transaction values above uint256 max during txpool validation and execution. Map the overflow to an invalid params RPC error and add focused unit tests.
1 parent f5fe86c commit 7dd662e

6 files changed

Lines changed: 118 additions & 1 deletion

File tree

core/state_processor_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package core
1919
import (
2020
"crypto/ecdsa"
2121
"encoding/binary"
22+
"errors"
2223
"fmt"
2324
"math"
2425
"math/big"
@@ -598,6 +599,63 @@ func TestApplyTransactionWithEVMStateChangeHooks(t *testing.T) {
598599
}
599600
}
600601

602+
func TestApplyTransactionWithEVMRejectsValueOverflow(t *testing.T) {
603+
t.Parallel()
604+
605+
config := &params.ChainConfig{
606+
ChainID: big.NewInt(1),
607+
HomesteadBlock: big.NewInt(0),
608+
EIP155Block: big.NewInt(0),
609+
EIP158Block: big.NewInt(0),
610+
ByzantiumBlock: big.NewInt(0),
611+
Ethash: new(params.EthashConfig),
612+
}
613+
signer := types.LatestSigner(config)
614+
key, err := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
615+
if err != nil {
616+
t.Fatalf("Failed to create key: %v", err)
617+
}
618+
sender := crypto.PubkeyToAddress(key.PublicKey)
619+
recipient := common.HexToAddress("0x1234567890123456789012345678901234567890")
620+
tooBigValue := new(big.Int).Lsh(big.NewInt(1), 256)
621+
hugeBalance := new(big.Int).Lsh(big.NewInt(1), 300)
622+
623+
db := rawdb.NewMemoryDatabase()
624+
gspec := &Genesis{
625+
Config: config,
626+
Alloc: types.GenesisAlloc{
627+
sender: {
628+
Balance: hugeBalance,
629+
},
630+
},
631+
}
632+
genesis := gspec.MustCommit(db)
633+
blockchain, _ := NewBlockChain(db, nil, gspec, ethash.NewFaker(), vm.Config{})
634+
defer blockchain.Stop()
635+
636+
statedb, err := blockchain.State()
637+
if err != nil {
638+
t.Fatalf("Failed to get state: %v", err)
639+
}
640+
tx := types.NewTransaction(0, recipient, tooBigValue, 21000, big.NewInt(1), nil)
641+
signedTx, err := types.SignTx(tx, signer, key)
642+
if err != nil {
643+
t.Fatalf("Failed to sign tx: %v", err)
644+
}
645+
msg, err := TransactionToMessage(signedTx, signer, nil, big.NewInt(1), nil)
646+
if err != nil {
647+
t.Fatalf("Failed to build message: %v", err)
648+
}
649+
vmContext := NewEVMBlockContext(blockchain.CurrentBlock(), blockchain, nil)
650+
evmenv := vm.NewEVM(vmContext, statedb, nil, blockchain.Config(), vm.Config{})
651+
gasPool := new(GasPool).AddGas(1000000)
652+
var usedGas uint64
653+
_, _, _, err = ApplyTransactionWithEVM(msg, gasPool, statedb, big.NewInt(1), genesis.Hash(), signedTx, &usedGas, evmenv, nil, common.Address{})
654+
if !errors.Is(err, types.ErrUint256Overflow) {
655+
t.Fatalf("expected %v, got %v", types.ErrUint256Overflow, err)
656+
}
657+
}
658+
601659
func TestProcessParentBlockHash(t *testing.T) {
602660
var (
603661
chainConfig = params.MergedTestChainConfig

core/state_transition.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (*ExecutionResult,
433433
// Check clause 6
434434
value, overflow := uint256.FromBig(msg.Value)
435435
if overflow {
436-
return nil, fmt.Errorf("%w: address %v", ErrInsufficientFundsForTransfer, msg.From.Hex())
436+
return nil, fmt.Errorf("%w: address %v", types.ErrUint256Overflow, msg.From.Hex())
437437
}
438438
if !value.IsZero() && !st.evm.Context.CanTransfer(st.state, msg.From, value) {
439439
return nil, fmt.Errorf("%w: address %v", ErrInsufficientFundsForTransfer, msg.From.Hex())

core/txpool/legacypool/legacypool_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,19 @@ func TestNegativeValue(t *testing.T) {
733733
}
734734
}
735735

736+
func TestValueOverflow(t *testing.T) {
737+
t.Parallel()
738+
739+
pool, key := setupPool()
740+
defer pool.Close()
741+
742+
tooBigValue := new(big.Int).Lsh(big.NewInt(1), 256)
743+
tx, _ := types.SignTx(types.NewTransaction(0, common.Address{}, tooBigValue, 100, big.NewInt(1), nil), types.HomesteadSigner{}, key)
744+
if err := pool.ValidateTxBasics(tx); !errors.Is(err, types.ErrUint256Overflow) {
745+
t.Error("expected", types.ErrUint256Overflow, "got", err)
746+
}
747+
}
748+
736749
// TestValidateTransactionEIP2681 tests that the pool correctly validates transactions
737750
// according to EIP-2681, which limits the nonce to a maximum value of 2^64 - 1.
738751
func TestValidateTransactionEIP2681(t *testing.T) {

core/txpool/validation.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types
8080
if tx.Value().Sign() < 0 {
8181
return ErrNegativeValue
8282
}
83+
if tx.Value().BitLen() > 256 {
84+
return types.ErrUint256Overflow
85+
}
8386
// Ensure the transaction doesn't exceed the current block limit gas
8487
if head.GasLimit < tx.Gas() {
8588
return ErrGasLimit

internal/ethapi/api_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3180,6 +3180,9 @@ type txPoolBackendMock struct {
31803180

31813181
func (b *sendTxBackendMock) SendTx(ctx context.Context, signedTx *types.Transaction) error {
31823182
b.lastTx = signedTx
3183+
if signedTx.Value().BitLen() > 256 {
3184+
return types.ErrUint256Overflow
3185+
}
31833186
if b.sendErr != nil {
31843187
return b.sendErr
31853188
}
@@ -3444,6 +3447,43 @@ func TestSendRawTransactionFeeCapExceeded(t *testing.T) {
34443447
require.ErrorContains(t, err, "exceeds the configured cap")
34453448
}
34463449

3450+
func TestSendRawTransactionRejectsValueOverflow(t *testing.T) {
3451+
t.Parallel()
3452+
3453+
key, err := crypto.HexToECDSA("8a1f9a8f95be41cd7ccb6168179afb4504aefe388d1e14474d32c45c72ce7b7a")
3454+
require.NoError(t, err)
3455+
3456+
backend := &sendTxBackendMock{backendMock: newBackendMock()}
3457+
api := NewTransactionAPI(backend, nil)
3458+
3459+
to := common.Address{0x56}
3460+
overflowValue := new(big.Int).Lsh(big.NewInt(1), 256)
3461+
tx, err := types.SignNewTx(key, types.LatestSigner(backend.ChainConfig()), &types.LegacyTx{
3462+
Nonce: 11,
3463+
GasPrice: big.NewInt(2),
3464+
Gas: 21000,
3465+
To: &to,
3466+
Value: overflowValue,
3467+
})
3468+
require.NoError(t, err)
3469+
3470+
raw, err := tx.MarshalBinary()
3471+
require.NoError(t, err)
3472+
3473+
_, err = api.SendRawTransaction(context.Background(), raw)
3474+
require.ErrorIs(t, err, types.ErrUint256Overflow)
3475+
require.NotNil(t, backend.lastTx)
3476+
require.Equal(t, overflowValue, backend.lastTx.Value())
3477+
}
3478+
3479+
func TestTxValidationErrorUint256Overflow(t *testing.T) {
3480+
t.Parallel()
3481+
3482+
err := txValidationError(types.ErrUint256Overflow)
3483+
require.Equal(t, errCodeInvalidParams, err.ErrorCode())
3484+
require.ErrorContains(t, err, types.ErrUint256Overflow.Error())
3485+
}
3486+
34473487
func TestGetTransactionByHashBasic(t *testing.T) {
34483488
t.Parallel()
34493489

internal/ethapi/errors.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/XinFinOrg/XDPoSChain/accounts/abi"
2424
"github.com/XinFinOrg/XDPoSChain/common/hexutil"
2525
"github.com/XinFinOrg/XDPoSChain/core"
26+
coretypes "github.com/XinFinOrg/XDPoSChain/core/types"
2627
"github.com/XinFinOrg/XDPoSChain/core/vm"
2728
)
2829

@@ -107,6 +108,8 @@ func txValidationError(err error) *invalidTxError {
107108
return &invalidTxError{Message: err.Error(), Code: errCodeInvalidParams}
108109
case errors.Is(err, core.ErrFeeCapTooLow):
109110
return &invalidTxError{Message: err.Error(), Code: errCodeInvalidParams}
111+
case errors.Is(err, coretypes.ErrUint256Overflow):
112+
return &invalidTxError{Message: err.Error(), Code: errCodeInvalidParams}
110113
case errors.Is(err, core.ErrInsufficientFunds):
111114
return &invalidTxError{Message: err.Error(), Code: errCodeInsufficientFunds}
112115
case errors.Is(err, core.ErrIntrinsicGas):

0 commit comments

Comments
 (0)