diff --git a/cl/merkle_tree/list.go b/cl/merkle_tree/list.go index d953b968458..9522a6123fc 100644 --- a/cl/merkle_tree/list.go +++ b/cl/merkle_tree/list.go @@ -110,6 +110,10 @@ func packBitsInto(dst [][32]byte, bytes []byte) [][32]byte { } func parseBitlist(dst, buf []byte) ([]byte, uint64) { + // A bitlist without its sentinel bit has no recoverable length. + if len(buf) == 0 || buf[len(buf)-1] == 0 { + return dst, 0 + } msb := uint8(bits.Len8(buf[len(buf)-1])) - 1 size := uint64(8*(len(buf)-1) + int(msb)) diff --git a/cl/merkle_tree/merkle_root_test.go b/cl/merkle_tree/merkle_root_test.go index 5ac9d4ce840..82eea0ff644 100644 --- a/cl/merkle_tree/merkle_root_test.go +++ b/cl/merkle_tree/merkle_root_test.go @@ -79,3 +79,20 @@ func TestProgressiveContainerProofRejectsOversizedSchema(t *testing.T) { _, err := merkle_tree.ProgressiveContainerProofAll(0, schema...) require.Error(t, err) } + +func TestBitlistRootWithLimitNoSentinel(t *testing.T) { + const limit = 2048 + + empty, err := merkle_tree.BitlistRootWithLimit([]byte{}, limit) + require.NoError(t, err) + + for _, malformed := range [][]byte{{0x00}, {0xff, 0x00}} { + root, err := merkle_tree.BitlistRootWithLimit(malformed, limit) + require.NoError(t, err) + require.Equal(t, empty, root) + } + + wellFormed, err := merkle_tree.BitlistRootWithLimit([]byte{0x03}, limit) + require.NoError(t, err) + require.NotEqual(t, empty, wellFormed) +} diff --git a/cl/phase1/network/backward_beacon_downloader.go b/cl/phase1/network/backward_beacon_downloader.go index ef7cfad7315..f30a9f12b0e 100644 --- a/cl/phase1/network/backward_beacon_downloader.go +++ b/cl/phase1/network/backward_beacon_downloader.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "io" - "math" "net/http" "slices" "strings" @@ -38,6 +37,7 @@ import ( "github.com/erigontech/erigon/cl/sentinel/peers" "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/common/math" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/snapshotsync/freezeblocks" ) @@ -199,8 +199,8 @@ func (b *BackwardBeaconDownloader) RequestMore(ctx context.Context) error { // Falls back to the beacon API when P2P is unavailable and an HTTP URL is configured. func (b *BackwardBeaconDownloader) fetchBlockRange(ctx context.Context) ([]*cltypes.SignedBeaconBlock, error) { const count = uint64(64) - start := b.slotToDownload.Load() - count + 1 - if start > b.slotToDownload.Load() { // overflow check + start, underflow := math.SafeSub(b.slotToDownload.Load(), count-1) + if underflow { start = 0 } diff --git a/common/math/integer.go b/common/math/integer.go index 201f7f3f8f1..95ca56f359d 100644 --- a/common/math/integer.go +++ b/common/math/integer.go @@ -125,6 +125,12 @@ func SafeAdd(x, y uint64) (uint64, bool) { return sum, carryOut != 0 } +// SafeSub returns x-y and checks for underflow. +func SafeSub(x, y uint64) (uint64, bool) { + diff, borrowOut := bits.Sub64(x, y, 0) + return diff, borrowOut != 0 +} + // NextPowerOfTwo returns the least power of two at or above n, and 1 for // n == 0; n above 1<<63 wraps to 0. func NextPowerOfTwo(n uint64) uint64 { diff --git a/common/math/integer_test.go b/common/math/integer_test.go index ba4f9831e06..523e91bebb3 100644 --- a/common/math/integer_test.go +++ b/common/math/integer_test.go @@ -99,3 +99,20 @@ func TestNextPowerOfTwo(t *testing.T) { assert.Equal(t, tc.want, NextPowerOfTwo(tc.in), "n=%d", tc.in) } } + +func TestSafeSub(t *testing.T) { + for _, tc := range []struct { + x, y, want uint64 + underflow bool + }{ + {9, 4, 5, false}, + {4, 4, 0, false}, + {0, 1, ^uint64(0), true}, + {4, 9, ^uint64(0) - 4, true}, + {^uint64(0), ^uint64(0), 0, false}, + } { + got, underflow := SafeSub(tc.x, tc.y) + assert.Equal(t, tc.want, got, "%d-%d", tc.x, tc.y) + assert.Equal(t, tc.underflow, underflow, "%d-%d", tc.x, tc.y) + } +} diff --git a/execution/execmodule/notification_dispatcher.go b/execution/execmodule/notification_dispatcher.go index 03fe89e3ad4..9f751a79b8f 100644 --- a/execution/execmodule/notification_dispatcher.go +++ b/execution/execmodule/notification_dispatcher.go @@ -21,6 +21,7 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/common/math" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/rawdb" "github.com/erigontech/erigon/db/state/execctx" @@ -109,9 +110,13 @@ func (d *Dispatcher) Dispatch( // Genesis (block 0): notify from block 0. notifyFrom = 0 default: - heightSpan := min(finishProgressAfter-finishProgressBefore, 1024) - notifyFrom = finishProgressAfter - heightSpan - notifyFrom++ + // finishProgressAfter can trail finishProgressBefore, and a wrapped + // span would clamp to 1024 and notify over a range that never ran. + span, underflow := math.SafeSub(finishProgressAfter, finishProgressBefore) + if underflow { + span = 0 + } + notifyFrom = finishProgressAfter - min(span, 1024) + 1 } notifyTo := finishProgressAfter + 1 //[from, to) diff --git a/execution/protocol/aa/aa_exec.go b/execution/protocol/aa/aa_exec.go index bd3c2381d22..e9f9c92ca92 100644 --- a/execution/protocol/aa/aa_exec.go +++ b/execution/protocol/aa/aa_exec.go @@ -64,7 +64,7 @@ func ValidateAATransaction( } validationGasUsed = preTxCost - if err := chargeGas(header, tx, gasPool, ibs, preTxCost); err != nil { + if err := chargeGas(header, tx, gasPool, ibs); err != nil { return nil, 0, err } @@ -317,7 +317,11 @@ func ExecuteAATransaction( return 0, 0, err } - gasPool.AddGas(params.TxAAGas + tx.ValidationGasLimit + tx.PaymasterValidationGasLimit + tx.GasLimit + tx.PostOpGasLimit - gasUsed) + totalGasLimit, ok := tx.TotalGasLimit(params.TxAAGas) + if !ok { + return 0, 0, fmt.Errorf("%w: RIP-7560 gas limits sum overflows uint64", protocol.ErrGasLimitReached) + } + gasPool.AddGas(totalGasLimit - gasUsed) return executionStatus, gasUsed, nil } diff --git a/execution/protocol/aa/aa_gas.go b/execution/protocol/aa/aa_gas.go index 6f053abc1e4..890eb20619c 100644 --- a/execution/protocol/aa/aa_gas.go +++ b/execution/protocol/aa/aa_gas.go @@ -20,13 +20,19 @@ func chargeGas( tx *types.AccountAbstractionTransaction, gasPool *protocol.GasPool, ibs *state.IntraBlockState, - preTxCost uint64, ) error { baseFee := header.BaseFee effectiveGasTip := tx.GetEffectiveGasTip(baseFee) effectiveGasPrice := new(uint256.Int).Add(baseFee, &effectiveGasTip) - totalGasLimit := preTxCost + tx.ValidationGasLimit + tx.PaymasterValidationGasLimit + tx.GasLimit + tx.PostOpGasLimit + // RIP-7560 maxPossibleGasCost is AA_BASE_GAS_COST plus the four declared + // limits. preTxCost carries the dynamic calldata charges too, which the + // validation frame already takes out of ValidationGasLimit — charging it + // here would precharge more than refundGas and the gas pool ever return. + totalGasLimit, ok := tx.TotalGasLimit(params.TxAAGas) + if !ok { + return fmt.Errorf("%w: RIP-7560 gas limits sum overflows uint64", protocol.ErrGasLimitReached) + } preCharge := new(uint256.Int).SetUint64(totalGasLimit) preCharge = preCharge.Mul(preCharge, effectiveGasPrice) @@ -62,7 +68,10 @@ func refundGas( effectiveGasPrice := new(uint256.Int).Add(baseFee, &effectiveGasTip) actualGasCost := new(uint256.Int).Mul(effectiveGasPrice, new(uint256.Int).SetUint64(gasUsed)) - totalGasLimit := params.TxAAGas + tx.ValidationGasLimit + tx.PaymasterValidationGasLimit + tx.GasLimit + tx.PostOpGasLimit + totalGasLimit, ok := tx.TotalGasLimit(params.TxAAGas) + if !ok { + return fmt.Errorf("%w: RIP-7560 gas limits sum overflows uint64", protocol.ErrGasLimitReached) + } preCharge := new(uint256.Int).SetUint64(totalGasLimit) preCharge = preCharge.Mul(preCharge, effectiveGasPrice) diff --git a/execution/protocol/txn_executor.go b/execution/protocol/txn_executor.go index b2d485ee680..2a8168107aa 100644 --- a/execution/protocol/txn_executor.go +++ b/execution/protocol/txn_executor.go @@ -29,6 +29,7 @@ import ( "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/dbg" "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/common/math" "github.com/erigontech/erigon/common/u256" "github.com/erigontech/erigon/execution/protocol/mdgas" "github.com/erigontech/erigon/execution/protocol/params" @@ -304,7 +305,7 @@ func (st *TxnExecutor) preCheck(gasBailout bool, intrinsicGasResult mdgas.Intrin return upfrontTxnFees{}, &nonceError{err: ErrNonceTooHigh, from: from, txNonce: msgNonce, stateNonce: stNonce} } else if stNonce > msgNonce { return upfrontTxnFees{}, &nonceError{err: ErrNonceTooLow, from: from, txNonce: msgNonce, stateNonce: stNonce} - } else if stNonce+1 < stNonce { + } else if _, overflow := math.SafeAdd(stNonce, 1); overflow { return upfrontTxnFees{}, fmt.Errorf("%w: address %v, nonce: %d", ErrNonceMax, from, stNonce) } diff --git a/execution/types/aa_transaction.go b/execution/types/aa_transaction.go index d0c51ec8cda..73dc945c873 100644 --- a/execution/types/aa_transaction.go +++ b/execution/types/aa_transaction.go @@ -9,6 +9,7 @@ import ( "github.com/holiman/uint256" "github.com/erigontech/erigon/common" + "github.com/erigontech/erigon/common/math" "github.com/erigontech/erigon/execution/abi" "github.com/erigontech/erigon/execution/chain" "github.com/erigontech/erigon/execution/protocol/mdgas" @@ -114,8 +115,26 @@ func (tx *AccountAbstractionTransaction) GetFeeCap() *uint256.Int { return tx.FeeCap } +// TotalGasLimit reports whether base plus the declared gas limits fits in a uint64. +func (tx *AccountAbstractionTransaction) TotalGasLimit(base uint64) (uint64, bool) { + total := base + for _, gas := range [...]uint64{tx.ValidationGasLimit, tx.PaymasterValidationGasLimit, tx.GasLimit, tx.PostOpGasLimit} { + sum, overflow := math.SafeAdd(total, gas) + if overflow { + return 0, false + } + total = sum + } + return total, true +} + func (tx *AccountAbstractionTransaction) GetGasLimit() uint64 { - return params.TxAAGas + tx.ValidationGasLimit + tx.PaymasterValidationGasLimit + tx.GasLimit + tx.PostOpGasLimit + // Saturate: the interface cannot report overflow, and a wrapped-small total would pass gas checks. + total, ok := tx.TotalGasLimit(params.TxAAGas) + if !ok { + return math.MaxUint64 + } + return total } func (tx *AccountAbstractionTransaction) GetTipCap() *uint256.Int { diff --git a/execution/types/transaction_test.go b/execution/types/transaction_test.go index 4375677f41c..2e0b3bf8952 100644 --- a/execution/types/transaction_test.go +++ b/execution/types/transaction_test.go @@ -876,3 +876,18 @@ func TestTypedTxEmptyToErrorMessage(t *testing.T) { }) } } + +func TestAATotalGasLimitOverflow(t *testing.T) { + t.Parallel() + + tx := &AccountAbstractionTransaction{ValidationGasLimit: 1, PaymasterValidationGasLimit: 2, PostOpGasLimit: 4} + tx.GasLimit = 8 + total, ok := tx.TotalGasLimit(16) + assert.True(t, ok) + assert.Equal(t, uint64(31), total) + + overflowing := &AccountAbstractionTransaction{ValidationGasLimit: ^uint64(0), PaymasterValidationGasLimit: 1} + _, ok = overflowing.TotalGasLimit(params.TxAAGas) + assert.False(t, ok) + assert.Equal(t, ^uint64(0), overflowing.GetGasLimit()) +} diff --git a/node/privateapi/ethbackend.go b/node/privateapi/ethbackend.go index 7a80043a8d3..b5e6a38cdd6 100644 --- a/node/privateapi/ethbackend.go +++ b/node/privateapi/ethbackend.go @@ -538,7 +538,11 @@ func (s *EthBackendServer) AAValidation(ctx context.Context, req *remoteproto.AA return nil, err } - totalGasLimit := preTxCost + aaTxn.ValidationGasLimit + aaTxn.PaymasterValidationGasLimit + aaTxn.GasLimit + aaTxn.PostOpGasLimit + totalGasLimit, ok := aaTxn.TotalGasLimit(preTxCost) + if !ok { + log.Info("RIP-7560 validation err", "err", "gas limits sum overflows uint64") + return &remoteproto.AAValidationReply{Valid: false}, nil + } _, _, err = aa.ValidateAATransaction(aaTxn, ibs, new(protocol.GasPool).AddGas(totalGasLimit), header, evm, s.chainConfig) if err != nil { log.Info("RIP-7560 validation err", "err", err.Error())