mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-06-19 13:21:37 +00:00
fix(core/txpool/legacypool): prevent uint256 overflow panic in executable tx filtering, fix #2134 (#2168)
A runtime panic was triggered in promoteExecutables/demoteUnexecutables when account balance was converted with uint256.MustFromBig(...): panic: overflow ... legacypool.go:1637 Root cause: - pool.currentState.GetBalance(addr) can exceed uint256 range in this code path. - uint256.MustFromBig(balance) panics on overflow, crashing the reorg loop. What this commit changes: - remove uint256.MustFromBig(balance) from executable/non-executable filtering paths - change list.Filter costLimit from *uint256.Int to *big.Int, and compare costs using big.Int directly - keep overflow-safe totalcost accounting for replacements (subtract old cost first, then add new) - return txpool.ErrSpecialTxCostOverflow for special-tx cost/totalcost overflow instead of returning (false, nil) - avoid partial pending-state mutation by attaching a new pending list only after overflow-safe totalcost calculation succeeds Tests: - add regression coverage for special-tx overflow rejection returning non-nil error - verify no pending/lookup/nonce mutation on overflow rejection - cover replacement paths to ensure no intermediate-overflow regressions in list.Add/promoteSpecialTx
This commit is contained in:
parent
5b5c39d849
commit
bd355d7b3f
5 changed files with 221 additions and 25 deletions
|
|
@ -71,5 +71,7 @@ var (
|
|||
|
||||
ErrDuplicateSpecialTransaction = errors.New("duplicate a special transaction")
|
||||
|
||||
ErrSpecialTxCostOverflow = errors.New("special transaction cost overflow")
|
||||
|
||||
ErrMinDeploySMC = errors.New("smart contract creation cost is under allowance")
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1072,17 +1072,38 @@ func (pool *LegacyPool) promoteTx(addr common.Address, hash common.Hash, tx *typ
|
|||
}
|
||||
|
||||
func (pool *LegacyPool) promoteSpecialTx(addr common.Address, tx *types.Transaction, isLocal bool) (bool, error) {
|
||||
// Try to insert the transaction into the pending queue
|
||||
if pool.pending[addr] == nil {
|
||||
pool.pending[addr] = newList(true)
|
||||
list := pool.pending[addr]
|
||||
newPending := list == nil
|
||||
if newPending {
|
||||
list = newList(true)
|
||||
}
|
||||
|
||||
base := new(uint256.Int).Set(list.totalcost)
|
||||
old := list.txs.Get(tx.Nonce())
|
||||
if old != nil {
|
||||
if old.IsSpecialTransaction() {
|
||||
return false, txpool.ErrDuplicateSpecialTransaction
|
||||
}
|
||||
// Old is being replaced, subtract old cost
|
||||
if _, underflow := base.SubOverflow(base, uint256.MustFromBig(old.Cost())); underflow {
|
||||
panic("totalcost underflow")
|
||||
}
|
||||
}
|
||||
// Keep overflow behavior aligned with list.Add.
|
||||
cost, overflow := uint256.FromBig(tx.Cost())
|
||||
if overflow {
|
||||
return false, txpool.ErrSpecialTxCostOverflow
|
||||
}
|
||||
total, overflow := new(uint256.Int).AddOverflow(base, cost)
|
||||
if overflow {
|
||||
return false, txpool.ErrSpecialTxCostOverflow
|
||||
}
|
||||
list.totalcost = total
|
||||
if newPending {
|
||||
pool.pending[addr] = list
|
||||
pendingAddrsGauge.Inc(1)
|
||||
}
|
||||
list := pool.pending[addr]
|
||||
|
||||
old := list.txs.Get(tx.Nonce())
|
||||
if old.IsSpecialTransaction() {
|
||||
return false, txpool.ErrDuplicateSpecialTransaction
|
||||
}
|
||||
// Otherwise discard any previous transaction and mark this
|
||||
if old != nil {
|
||||
pool.all.Remove(old.Hash())
|
||||
|
|
@ -1093,7 +1114,7 @@ func (pool *LegacyPool) promoteSpecialTx(addr common.Address, tx *types.Transact
|
|||
pendingGauge.Inc(1)
|
||||
}
|
||||
list.txs.Put(tx)
|
||||
if cost := uint256.MustFromBig(tx.Cost()); list.costcap.Cmp(cost) < 0 {
|
||||
if cost := tx.Cost(); list.costcap.Cmp(cost) < 0 {
|
||||
list.costcap = cost
|
||||
}
|
||||
if gas := tx.Gas(); list.gascap < gas {
|
||||
|
|
@ -1648,7 +1669,7 @@ func (pool *LegacyPool) promoteExecutables(accounts []common.Address) []*types.T
|
|||
if pool.chain.CurrentHeader() != nil {
|
||||
number = pool.chain.CurrentHeader().Number
|
||||
}
|
||||
drops, _ := list.Filter(uint256.MustFromBig(pool.currentState.GetBalance(addr)), gasLimit, pool.trc21FeeCapacity, number)
|
||||
drops, _ := list.Filter(pool.currentState.GetBalance(addr), gasLimit, pool.trc21FeeCapacity, number)
|
||||
for _, tx := range drops {
|
||||
pool.all.Remove(tx.Hash())
|
||||
}
|
||||
|
|
@ -1853,7 +1874,7 @@ func (pool *LegacyPool) demoteUnexecutables() {
|
|||
if pool.chain.CurrentHeader() != nil {
|
||||
number = pool.chain.CurrentHeader().Number
|
||||
}
|
||||
drops, invalids := list.Filter(uint256.MustFromBig(pool.currentState.GetBalance(addr)), gasLimit, pool.trc21FeeCapacity, number)
|
||||
drops, invalids := list.Filter(pool.currentState.GetBalance(addr), gasLimit, pool.trc21FeeCapacity, number)
|
||||
for _, tx := range drops {
|
||||
hash := tx.Hash()
|
||||
pool.all.Remove(hash)
|
||||
|
|
|
|||
|
|
@ -272,6 +272,173 @@ func deriveSender(tx *types.Transaction) (common.Address, error) {
|
|||
return types.Sender(types.HomesteadSigner{}, tx)
|
||||
}
|
||||
|
||||
func TestPromoteSpecialTxUpdatesTotalCost(t *testing.T) {
|
||||
pool, key := setupPool()
|
||||
defer pool.Close()
|
||||
|
||||
normal := transaction(0, 21000, key)
|
||||
addr, err := deriveSender(normal)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to derive sender: %v", err)
|
||||
}
|
||||
special := setCodeTx(0, key, nil)
|
||||
|
||||
pool.mu.Lock()
|
||||
defer pool.mu.Unlock()
|
||||
|
||||
pool.pending[addr] = newList(true)
|
||||
list := pool.pending[addr]
|
||||
|
||||
inserted, _ := list.Add(normal, pool.config.PriceBump)
|
||||
if !inserted {
|
||||
t.Fatal("failed to insert baseline transaction")
|
||||
}
|
||||
if _, err := pool.promoteSpecialTx(addr, special, false); err != nil {
|
||||
t.Fatalf("promoteSpecialTx failed: %v", err)
|
||||
}
|
||||
want, overflow := uint256.FromBig(special.Cost())
|
||||
if overflow {
|
||||
t.Fatal("special tx cost overflowed uint256 in test setup")
|
||||
}
|
||||
if list.totalcost.Cmp(want) != 0 {
|
||||
t.Fatalf("totalcost mismatch after special promotion: have %v want %v", list.totalcost, want)
|
||||
}
|
||||
|
||||
// Removing the pending tx should not underflow totalcost.
|
||||
list.Forward(1)
|
||||
if list.totalcost.Sign() != 0 {
|
||||
t.Fatalf("totalcost should be zero after removal, have %v", list.totalcost)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListAddReplacementAvoidsIntermediateOverflow(t *testing.T) {
|
||||
key, err := crypto.GenerateKey()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to generate key: %v", err)
|
||||
}
|
||||
max := new(big.Int).Sub(new(big.Int).Lsh(common.Big1, 256), common.Big1)
|
||||
oldPrice := new(big.Int).Sub(new(big.Int).Rsh(new(big.Int).Set(max), 1), big.NewInt(100))
|
||||
newPrice := new(big.Int).Add(oldPrice, common.Big1)
|
||||
|
||||
oldTx, err := types.SignTx(types.NewTransaction(0, common.Address{}, common.Big0, 1, oldPrice, nil), types.HomesteadSigner{}, key)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to sign old tx: %v", err)
|
||||
}
|
||||
newTx, err := types.SignTx(types.NewTransaction(0, common.Address{}, common.Big0, 1, newPrice, nil), types.HomesteadSigner{}, key)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to sign replacement tx: %v", err)
|
||||
}
|
||||
|
||||
list := newList(true)
|
||||
inserted, _ := list.Add(oldTx, 0)
|
||||
if !inserted {
|
||||
t.Fatal("failed to insert baseline transaction")
|
||||
}
|
||||
inserted, replaced := list.Add(newTx, 0)
|
||||
if !inserted {
|
||||
t.Fatal("replacement transaction should not overflow after subtracting old cost")
|
||||
}
|
||||
if replaced == nil || replaced.Hash() != oldTx.Hash() {
|
||||
t.Fatal("expected old transaction to be replaced")
|
||||
}
|
||||
want, overflow := uint256.FromBig(newTx.Cost())
|
||||
if overflow {
|
||||
t.Fatal("replacement tx cost overflowed uint256 in test setup")
|
||||
}
|
||||
if list.totalcost.Cmp(want) != 0 {
|
||||
t.Fatalf("totalcost mismatch after replacement: have %v want %v", list.totalcost, want)
|
||||
}
|
||||
if tx := list.txs.Get(newTx.Nonce()); tx == nil || tx.Hash() != newTx.Hash() {
|
||||
t.Fatal("replacement transaction was not stored in list")
|
||||
}
|
||||
list.Forward(1)
|
||||
if list.totalcost.Sign() != 0 {
|
||||
t.Fatalf("totalcost should be zero after removal, have %v", list.totalcost)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromoteSpecialTxReplacementAvoidsIntermediateOverflow(t *testing.T) {
|
||||
pool, key := setupPool()
|
||||
defer pool.Close()
|
||||
|
||||
max := new(big.Int).Sub(new(big.Int).Lsh(common.Big1, 256), common.Big1)
|
||||
oldPrice := new(big.Int).Sub(new(big.Int).Rsh(new(big.Int).Set(max), 1), big.NewInt(100))
|
||||
newPrice := new(big.Int).Add(oldPrice, common.Big1)
|
||||
oldTx, err := types.SignTx(types.NewTransaction(0, common.Address{}, common.Big0, 1, oldPrice, nil), types.HomesteadSigner{}, key)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to sign old tx: %v", err)
|
||||
}
|
||||
addr, err := deriveSender(oldTx)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to derive sender: %v", err)
|
||||
}
|
||||
special := pricedSetCodeTx(0, 1, uint256.MustFromBig(newPrice), uint256.NewInt(1), key, nil)
|
||||
|
||||
pool.mu.Lock()
|
||||
defer pool.mu.Unlock()
|
||||
|
||||
pool.pending[addr] = newList(true)
|
||||
list := pool.pending[addr]
|
||||
inserted, _ := list.Add(oldTx, 0)
|
||||
if !inserted {
|
||||
t.Fatal("failed to insert baseline transaction")
|
||||
}
|
||||
inserted, err = pool.promoteSpecialTx(addr, special, false)
|
||||
if err != nil {
|
||||
t.Fatalf("promoteSpecialTx failed: %v", err)
|
||||
}
|
||||
if !inserted {
|
||||
t.Fatal("special replacement should not overflow after subtracting old cost")
|
||||
}
|
||||
want, overflow := uint256.FromBig(special.Cost())
|
||||
if overflow {
|
||||
t.Fatal("special tx cost overflowed uint256 in test setup")
|
||||
}
|
||||
if list.totalcost.Cmp(want) != 0 {
|
||||
t.Fatalf("totalcost mismatch after special replacement: have %v want %v", list.totalcost, want)
|
||||
}
|
||||
if tx := list.txs.Get(special.Nonce()); tx == nil || tx.Hash() != special.Hash() {
|
||||
t.Fatal("special replacement transaction was not stored in list")
|
||||
}
|
||||
list.Forward(1)
|
||||
if list.totalcost.Sign() != 0 {
|
||||
t.Fatalf("totalcost should be zero after removal, have %v", list.totalcost)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromoteSpecialTxOverflowReturnsErrorWithoutMutation(t *testing.T) {
|
||||
pool, key := setupPool()
|
||||
defer pool.Close()
|
||||
|
||||
normal := transaction(0, 21000, key)
|
||||
addr, err := deriveSender(normal)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to derive sender: %v", err)
|
||||
}
|
||||
max := new(big.Int).Sub(new(big.Int).Lsh(common.Big1, 256), common.Big1)
|
||||
special := pricedSetCodeTx(0, 2, uint256.MustFromBig(max), uint256.NewInt(1), key, nil)
|
||||
|
||||
pool.mu.Lock()
|
||||
defer pool.mu.Unlock()
|
||||
|
||||
inserted, err := pool.promoteSpecialTx(addr, special, false)
|
||||
if inserted {
|
||||
t.Fatal("overflowing special tx must not be inserted")
|
||||
}
|
||||
if !errors.Is(err, txpool.ErrSpecialTxCostOverflow) {
|
||||
t.Fatalf("wrong error: have %v, want %v", err, txpool.ErrSpecialTxCostOverflow)
|
||||
}
|
||||
if _, ok := pool.pending[addr]; ok {
|
||||
t.Fatal("pending list created for rejected special tx")
|
||||
}
|
||||
if pool.all.Get(special.Hash()) != nil {
|
||||
t.Fatal("rejected special tx should not be tracked in lookup")
|
||||
}
|
||||
if pool.pendingNonces.get(addr) != 0 {
|
||||
t.Fatalf("pending nonce changed for rejected special tx: have %d want 0", pool.pendingNonces.get(addr))
|
||||
}
|
||||
}
|
||||
|
||||
type testChain struct {
|
||||
*testBlockChain
|
||||
address common.Address
|
||||
|
|
|
|||
|
|
@ -274,7 +274,7 @@ type list struct {
|
|||
strict bool // Whether nonces are strictly continuous or not
|
||||
txs *sortedMap // Heap indexed sorted hash map of the transactions
|
||||
|
||||
costcap *uint256.Int // Price of the highest costing transaction (reset only if exceeds balance)
|
||||
costcap *big.Int // Price of the highest costing transaction (reset only if exceeds balance)
|
||||
gascap uint64 // Gas limit of the highest spending transaction (reset only if exceeds block limit)
|
||||
totalcost *uint256.Int // Total cost of all transactions in the list
|
||||
}
|
||||
|
|
@ -285,7 +285,7 @@ func newList(strict bool) *list {
|
|||
return &list{
|
||||
strict: strict,
|
||||
txs: newSortedMap(),
|
||||
costcap: new(uint256.Int),
|
||||
costcap: new(big.Int),
|
||||
totalcost: new(uint256.Int),
|
||||
}
|
||||
}
|
||||
|
|
@ -302,12 +302,13 @@ func (l *list) Contains(nonce uint64) bool {
|
|||
// If the new transaction is accepted into the list, the lists' cost and gas
|
||||
// thresholds are also potentially updated.
|
||||
func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transaction) {
|
||||
base := new(uint256.Int).Set(l.totalcost)
|
||||
// If there's an older better transaction, abort
|
||||
old := l.txs.Get(tx.Nonce())
|
||||
if old.IsSpecialTransaction() {
|
||||
return false, nil
|
||||
}
|
||||
if old != nil {
|
||||
if old.IsSpecialTransaction() {
|
||||
return false, nil
|
||||
}
|
||||
if old.GasFeeCapCmp(tx) >= 0 || old.GasTipCapCmp(tx) >= 0 {
|
||||
return false, nil
|
||||
}
|
||||
|
|
@ -328,18 +329,24 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transa
|
|||
return false, nil
|
||||
}
|
||||
// Old is being replaced, subtract old cost
|
||||
l.subTotalCost([]*types.Transaction{old})
|
||||
if _, underflow := base.SubOverflow(base, uint256.MustFromBig(old.Cost())); underflow {
|
||||
panic("totalcost underflow")
|
||||
}
|
||||
}
|
||||
// Add new tx cost to totalcost
|
||||
cost, overflow := uint256.FromBig(tx.Cost())
|
||||
if overflow {
|
||||
return false, nil
|
||||
}
|
||||
l.totalcost.Add(l.totalcost, cost)
|
||||
total, overflow := new(uint256.Int).AddOverflow(base, cost)
|
||||
if overflow {
|
||||
return false, nil
|
||||
}
|
||||
l.totalcost = total
|
||||
|
||||
// Otherwise overwrite the old transaction with the current one
|
||||
l.txs.Put(tx)
|
||||
if l.costcap.Cmp(cost) < 0 {
|
||||
if cost := tx.Cost(); l.costcap.Cmp(cost) < 0 {
|
||||
l.costcap = cost
|
||||
}
|
||||
if gas := tx.Gas(); l.gascap < gas {
|
||||
|
|
@ -366,12 +373,12 @@ func (l *list) Forward(threshold uint64) types.Transactions {
|
|||
// a point in calculating all the costs or if the balance covers all. If the threshold
|
||||
// is lower than the costgas cap, the caps will be reset to a new high after removing
|
||||
// the newly invalidated transactions.
|
||||
func (l *list) Filter(costLimit *uint256.Int, gasLimit uint64, trc21Issuers map[common.Address]*big.Int, number *big.Int) (types.Transactions, types.Transactions) {
|
||||
func (l *list) Filter(costLimit *big.Int, gasLimit uint64, trc21Issuers map[common.Address]*big.Int, number *big.Int) (types.Transactions, types.Transactions) {
|
||||
// If all transactions are below the threshold, short circuit
|
||||
if l.costcap.Cmp(costLimit) <= 0 && l.gascap <= gasLimit {
|
||||
return nil, nil
|
||||
}
|
||||
l.costcap = new(uint256.Int).Set(costLimit) // Lower the caps to the thresholds
|
||||
l.costcap = new(big.Int).Set(costLimit) // Lower the caps to the thresholds
|
||||
l.gascap = gasLimit
|
||||
|
||||
// Filter out all the transactions above the account's funds
|
||||
|
|
@ -379,10 +386,10 @@ func (l *list) Filter(costLimit *uint256.Int, gasLimit uint64, trc21Issuers map[
|
|||
maximum := costLimit
|
||||
if tx.To() != nil {
|
||||
if feeCapacity, ok := trc21Issuers[*tx.To()]; ok {
|
||||
return tx.Gas() > gasLimit || new(big.Int).Add(costLimit.ToBig(), feeCapacity).Cmp(tx.TxCost(number)) < 0
|
||||
return tx.Gas() > gasLimit || new(big.Int).Add(costLimit, feeCapacity).Cmp(tx.TxCost(number)) < 0
|
||||
}
|
||||
}
|
||||
return tx.Gas() > gasLimit || tx.Cost().Cmp(maximum.ToBig()) > 0
|
||||
return tx.Gas() > gasLimit || tx.Cost().Cmp(maximum) > 0
|
||||
})
|
||||
|
||||
if len(removed) == 0 {
|
||||
|
|
|
|||
|
|
@ -24,7 +24,6 @@ import (
|
|||
"github.com/XinFinOrg/XDPoSChain/common"
|
||||
"github.com/XinFinOrg/XDPoSChain/core/types"
|
||||
"github.com/XinFinOrg/XDPoSChain/crypto"
|
||||
"github.com/holiman/uint256"
|
||||
)
|
||||
|
||||
// Tests that transactions can be added to strict lists and list contents and
|
||||
|
|
@ -78,7 +77,7 @@ func BenchmarkListAdd(t *testing.B) {
|
|||
}
|
||||
// Insert the transactions in a random order
|
||||
list := newList(true)
|
||||
priceLimit := uint256.NewInt(DefaultConfig.PriceLimit)
|
||||
priceLimit := big.NewInt(int64(DefaultConfig.PriceLimit))
|
||||
t.ResetTimer()
|
||||
for _, v := range rand.Perm(len(txs)) {
|
||||
list.Add(txs[v], DefaultConfig.PriceBump)
|
||||
|
|
|
|||
Loading…
Reference in a new issue