From 32a7c2b324eb1dc309e52c5a062f30d173824bee Mon Sep 17 00:00:00 2001 From: Csaba Kiraly Date: Fri, 30 Jan 2026 12:31:27 +0100 Subject: [PATCH] core/txpool/blobpool: drop underpriced before adding to pool Underpriced transactions were first added then dropped. This created various issues: - dropping is neither FIFO nor LIFO in a heap, so we had undefined behavior between equal priced transactions. We now only add a new transaction if it is strictly better than the worst in the pool. - adding and removing created extra disk writes - adding resulted in sending announcements to peers Signed-off-by: Csaba Kiraly --- core/txpool/blobpool/blobpool.go | 101 ++++++++++++++++++++++-------- core/txpool/blobpool/evictheap.go | 20 ++++++ 2 files changed, 96 insertions(+), 25 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index a1d9d9b820..a44da3952b 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -135,28 +135,38 @@ type blobTxMeta struct { evictionBlobFeeJumps float64 // Worse blob fee (converted to fee jumps) across all previous nonces } +// newStoredBlobTxMeta retrieves the indexed metadata fields from a blob transaction, +// adds storage specific fields (ID and size), and assembles a helper struct to track in memory. +// Requires the transaction to have a sidecar (or that we introduce a special version tag for no-sidecar). +func newStoredBlobTxMeta(tx *types.Transaction, id uint64, storageSize uint32) *blobTxMeta { + meta := newBlobTxMeta(tx) + meta.id = id + meta.storageSize = storageSize + return meta +} + // newBlobTxMeta retrieves the indexed metadata fields from a blob transaction // and assembles a helper struct to track in memory. -// Requires the transaction to have a sidecar (or that we introduce a special version tag for no-sidecar). -func newBlobTxMeta(id uint64, size uint64, storageSize uint32, tx *types.Transaction) *blobTxMeta { +// newBlobTxMeta leaves storage specific fields empty. Use newStoredBlobTxMeta +// to also populate those. +// Requires the transaction to have a sidecar. +func newBlobTxMeta(tx *types.Transaction) *blobTxMeta { if tx.BlobTxSidecar() == nil { // This should never happen, as the pool only admits blob transactions with a sidecar panic("missing blob tx sidecar") } meta := &blobTxMeta{ - hash: tx.Hash(), - vhashes: tx.BlobHashes(), - version: tx.BlobTxSidecar().Version, - id: id, - storageSize: storageSize, - size: size, - nonce: tx.Nonce(), - costCap: uint256.MustFromBig(tx.Cost()), - execTipCap: uint256.MustFromBig(tx.GasTipCap()), - execFeeCap: uint256.MustFromBig(tx.GasFeeCap()), - blobFeeCap: uint256.MustFromBig(tx.BlobGasFeeCap()), - execGas: tx.Gas(), - blobGas: tx.BlobGas(), + hash: tx.Hash(), + vhashes: tx.BlobHashes(), + version: tx.BlobTxSidecar().Version, + size: tx.Size(), + nonce: tx.Nonce(), + costCap: uint256.MustFromBig(tx.Cost()), + execTipCap: uint256.MustFromBig(tx.GasTipCap()), + execFeeCap: uint256.MustFromBig(tx.GasFeeCap()), + blobFeeCap: uint256.MustFromBig(tx.BlobGasFeeCap()), + execGas: tx.Gas(), + blobGas: tx.BlobGas(), } meta.basefeeJumps = dynamicFeeJumps(meta.execFeeCap) meta.blobfeeJumps = dynamicFeeJumps(meta.blobFeeCap) @@ -531,7 +541,7 @@ func (p *BlobPool) parseTransaction(id uint64, size uint32, blob []byte) error { return errors.New("missing blob sidecar") } - meta := newBlobTxMeta(id, tx.Size(), size, tx) + meta := newStoredBlobTxMeta(tx, id, size) if p.lookup.exists(meta.hash) { // This path is only possible after a crash, where deleted items are not // removed via the normal shutdown-startup procedure and thus may get @@ -1071,7 +1081,7 @@ func (p *BlobPool) reinject(addr common.Address, txhash common.Hash) error { } // Update the indices and metrics - meta := newBlobTxMeta(id, tx.Size(), p.store.Size(id), tx) + meta := newStoredBlobTxMeta(tx, id, p.store.Size(id)) if _, ok := p.index[addr]; !ok { if err := p.reserver.Hold(addr); err != nil { log.Warn("Failed to reserve account for blob pool", "tx", tx.Hash(), "from", addr, "err", err) @@ -1248,6 +1258,7 @@ func (p *BlobPool) validateTx(tx *types.Transaction) error { if err := p.checkDelegationLimit(tx); err != nil { return err } + // If the transaction replaces an existing one, ensure that price bumps are // adhered to. var ( @@ -1551,9 +1562,47 @@ func (p *BlobPool) addLocked(tx *types.Transaction, checkGapped bool) (err error } return err } + + // Create meta, in preparation of adding to the pool. + // Having the meta simplifies the check below for underpriced transactions. + meta := newBlobTxMeta(tx) + + // Calculate the eviction parameters for the transaction + var ( + from, _ = types.Sender(p.signer, tx) // already validated above + next = p.state.GetNonce(from) + offset = int(meta.nonce - next) + ) + meta.evictionExecTip = meta.execTipCap + meta.evictionExecFeeJumps = meta.basefeeJumps + meta.evictionBlobFeeJumps = meta.blobfeeJumps + if meta.nonce > next && len(p.index[from]) >= offset { + prev := p.index[from][int(meta.nonce-next-1)] + if meta.evictionExecTip.Cmp(meta.execTipCap) < 0 { + meta.evictionExecTip = prev.evictionExecTip + } + if meta.evictionExecFeeJumps < meta.basefeeJumps { + meta.evictionExecFeeJumps = prev.evictionExecFeeJumps + } + if meta.evictionBlobFeeJumps < meta.blobfeeJumps { + meta.evictionBlobFeeJumps = prev.evictionBlobFeeJumps + } + } + + // Check pool size limits before inserting the transaction + // If at limit, check whether it is underpriced. + // Note: we do not have the exact storage size yet, so we try to guess + // Note: equal priority to worse of pool is still considered underpriced. + // This is to prevent constant replacement when the pool is full. + if p.stored+meta.size > p.config.Datacap { + if p.evict.Underpriced(meta) { + log.Warn("Dropping underpriced blob transaction", "tx", tx.Hash(), "feecap", tx.GasFeeCap(), "tipcap", tx.GasTipCap(), "blobfeecap", tx.BlobGasFeeCap()) + return txpool.ErrUnderpriced + } + } + // If the address is not yet known, request exclusivity to track the account // only by this subpool until all transactions are evicted - from, _ := types.Sender(p.signer, tx) // already validated above if _, ok := p.index[from]; !ok { if err := p.reserver.Hold(from); err != nil { addNonExclusiveMeter.Mark(1) @@ -1582,14 +1631,15 @@ func (p *BlobPool) addLocked(tx *types.Transaction, checkGapped bool) (err error if err != nil { return err } - meta := newBlobTxMeta(id, tx.Size(), p.store.Size(id), tx) + // Finalize the meta with storage information + meta.id = id + meta.storageSize = p.store.Size(id) var ( - next = p.state.GetNonce(from) - offset = int(tx.Nonce() - next) - newacc = false + newacc = false + oldEvictionExecFeeJumps float64 + oldEvictionBlobFeeJumps float64 ) - var oldEvictionExecFeeJumps, oldEvictionBlobFeeJumps float64 if txs, ok := p.index[from]; ok { oldEvictionExecFeeJumps = txs[len(txs)-1].evictionExecFeeJumps oldEvictionBlobFeeJumps = txs[len(txs)-1].evictionBlobFeeJumps @@ -1677,9 +1727,10 @@ func (p *BlobPool) addLocked(tx *types.Transaction, checkGapped bool) (err error p.updateStorageMetrics() // If we've just dropped the added transaction, it was clearly underpriced. - // We could also try to check for this earlier, but it is compex because - // of the rolling fee caculations. + // We've already checked for this with approximate size, but do a final + // check in case it was dropped with the exact size. if !p.lookup.exists(tx.Hash()) { + log.Warn("Added blob transaction was dropped immediately, indicating underpricing", "hash", tx.Hash()) addUnderpricedMeter.Mark(1) return txpool.ErrUnderpriced } diff --git a/core/txpool/blobpool/evictheap.go b/core/txpool/blobpool/evictheap.go index 722a71bc9b..0a2aedad83 100644 --- a/core/txpool/blobpool/evictheap.go +++ b/core/txpool/blobpool/evictheap.go @@ -94,6 +94,12 @@ func (h *evictHeap) Less(i, j int) bool { lastI := txsI[len(txsI)-1] lastJ := txsJ[len(txsJ)-1] + return h.txPrioLt(lastI, lastJ) +} + +// LessTx compares two blobTxMeta entries and returns whether the first has a lower +// eviction priority than the second. +func (h *evictHeap) txPrioLt(lastI, lastJ *blobTxMeta) bool { prioI := evictionPriority(h.basefeeJumps, lastI.evictionExecFeeJumps, h.blobfeeJumps, lastI.evictionBlobFeeJumps) if prioI > 0 { prioI = 0 @@ -123,6 +129,20 @@ func (h *evictHeap) Push(x any) { h.addrs = append(h.addrs, x.(common.Address)) } +// Underpriced checks whether the given transaction is underpriced compared to the +// cheapest transaction in the heap. +// If a transaction has the same priority as the cheapest, it is still considered +// underpriced. +func (h *evictHeap) Underpriced(meta *blobTxMeta) bool { + if len(h.addrs) == 0 { + return false + } + cheapestTxs := h.metas[h.addrs[0]] + cheapestTx := cheapestTxs[len(cheapestTxs)-1] + + return !h.txPrioLt(cheapestTx, meta) +} + // Pop implements heap.Interface, removing and returning the last element of the // heap. //