Skip to content

Commit 247dfcd

Browse files
committed
fix(core/txpool,core/txpool/locals): hold back local resubmits below the gas price floor
A gas schedule fork raises the floor above transactions that were admitted under the previous tier, and the pool sweeps them out. The local tracker kept resubmitting them every minute: recheck ignored the result of pool.Add, and the transactions never went stale because their nonce never advanced, so they stayed tracked and journalled forever. Hold back any tracked transaction priced below the current floor, resolved for the block pending on top of the head exactly as admission validation resolves it. They stay tracked, so a reorg or a set-head rollback that lowers the floor picks them up again on the next recheck: the pool does not bring back what it swept, so the tracker is the only thing that can. Special transactions stay exempt, as they are during admission. Resolve the floor through a new TxPool.MinGasPrice, which goes through the same pendingBlockNumber helper as admission validation and the sweep, so the tracker and the pool cannot price the same transaction at different heights. Report the held back transactions through txpool/local/belowfloor, since they are still counted by txpool/local. Pin that ErrUnderMinGasPrice is not a temporary reject: the floor only rises as the chain advances, so retrying cannot help and AddLocal must not track the transaction, but the error must not be used to drop a tracked one either. Cover the floor with tests that rewind the head with SetHead across every tier boundary and cover a head without a block number, the hold-back at exactly the floor and one wei below it, resumption once the floor drops, and the exemption special transactions keep.
1 parent 6f8ae03 commit 247dfcd

8 files changed

Lines changed: 401 additions & 8 deletions

File tree

core/txpool/locals/errors.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ import (
2626
// IsTemporaryReject determines whether the given error indicates a temporary
2727
// reason to reject a transaction from being included in the txpool. The result
2828
// may change if the txpool's state changes later.
29+
//
30+
// ErrUnderMinGasPrice must stay out of this set: the floor only rises as the
31+
// chain advances, so retrying the same transaction cannot help while it stays
32+
// where it is. It must not be used to drop a tracked transaction either -- a
33+
// rollback or a reorg past the fork lowers the floor again, and the local
34+
// tracker is the only thing that can bring those transactions back.
2935
func IsTemporaryReject(err error) bool {
3036
switch {
3137
case errors.Is(err, legacypool.ErrOutOfOrderTxFromDelegated):

core/txpool/locals/errors_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright 2025 The go-ethereum Authors
2+
// This file is part of the go-ethereum library.
3+
//
4+
// The go-ethereum library is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Lesser General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// The go-ethereum library is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Lesser General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Lesser General Public License
15+
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
16+
17+
package locals
18+
19+
import (
20+
"errors"
21+
"testing"
22+
23+
"github.com/XinFinOrg/XDPoSChain/core/txpool"
24+
)
25+
26+
// TestIsTemporaryRejectExcludesMinGasPrice pins that a transaction priced below
27+
// the gas schedule floor is not a temporary reject: the floor only rises as the
28+
// chain advances, so retrying it cannot help and AddLocal must not track it.
29+
func TestIsTemporaryRejectExcludesMinGasPrice(t *testing.T) {
30+
if IsTemporaryReject(txpool.ErrUnderMinGasPrice) {
31+
t.Error("ErrUnderMinGasPrice must not be a temporary reject")
32+
}
33+
// The pool's own price limit is a node local setting that can change, so it
34+
// stays retryable. The contrast is what the assertion above is about.
35+
if !IsTemporaryReject(txpool.ErrUnderpriced) {
36+
t.Error("ErrUnderpriced must remain a temporary reject")
37+
}
38+
if IsTemporaryReject(errors.New("unrelated failure")) {
39+
t.Error("an unrelated error must not be a temporary reject")
40+
}
41+
}

core/txpool/locals/tx_tracker.go

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ import (
3535
var (
3636
recheckInterval = time.Minute
3737
localGauge = metrics.GetOrRegisterGauge("txpool/local", nil)
38+
39+
// Tracked transactions a gas schedule fork priced out of the pool. They stay
40+
// tracked so a rollback past the fork can pick them up again, but recheck
41+
// holds them back from resubmission, so they are broken out here.
42+
//
43+
// Only transactions missing from the pool are counted: a tracked transaction
44+
// the pool still has is counted as ok instead. local minus belowfloor is
45+
// therefore the tracked transactions the pool holds plus the ones recheck
46+
// resubmits this round, not just the latter.
47+
belowFloorGauge = metrics.GetOrRegisterGauge("txpool/local/belowfloor", nil)
3848
)
3949

4050
// TxTracker is a struct used to track priority transactions; it will check from
@@ -152,10 +162,15 @@ func (tracker *TxTracker) recheck(journalCheck bool) []*types.Transaction {
152162
defer tracker.mu.Unlock()
153163

154164
var (
155-
numStales = 0
156-
numOk = 0
157-
resubmits []*types.Transaction
165+
numStales = 0
166+
numOk = 0
167+
numBelowFloor = 0
168+
resubmits []*types.Transaction
158169
)
170+
// Resolved once: it can only change on a chain head update, and a change
171+
// mid-recheck would make the counters below inconsistent.
172+
floor := tracker.pool.MinGasPrice()
173+
159174
for sender, txs := range tracker.byAddr {
160175
// Wipe the stales
161176
stales := txs.Forward(tracker.pool.Nonce(sender))
@@ -170,6 +185,17 @@ func (tracker *TxTracker) recheck(journalCheck bool) []*types.Transaction {
170185
numOk++
171186
continue
172187
}
188+
// A gas schedule fork can raise the floor above transactions that
189+
// were admitted under the previous tier. Re-submitting them only
190+
// yields ErrUnderMinGasPrice, so hold them back until the floor
191+
// drops again: a reorg or a set-head rollback past the fork
192+
// reinstates them, and the pool does not bring back what it swept.
193+
// They stay tracked, so recheck picks them up again on its own.
194+
// Special transactions are exempt, exactly as during admission.
195+
if !tx.IsSpecialTransaction() && tx.GasPriceIntCmp(floor) < 0 {
196+
numBelowFloor++
197+
continue
198+
}
173199
resubmits = append(resubmits, tx)
174200
}
175201
}
@@ -197,7 +223,9 @@ func (tracker *TxTracker) recheck(journalCheck bool) []*types.Transaction {
197223
}
198224
}
199225
localGauge.Update(int64(len(tracker.all)))
200-
log.Debug("Tx tracker status", "need-resubmit", len(resubmits), "stale", numStales, "ok", numOk)
226+
belowFloorGauge.Update(int64(numBelowFloor))
227+
log.Debug("Tx tracker status", "need-resubmit", len(resubmits), "stale", numStales,
228+
"ok", numOk, "below-floor", numBelowFloor)
201229
return resubmits
202230
}
203231

core/txpool/locals/tx_tracker_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,3 +517,103 @@ func TestJournalLoadKeepsLastEntryPerNonce(t *testing.T) {
517517
t.Fatalf("the entry later in the file must survive the load: %v", replaced.Hash())
518518
}
519519
}
520+
521+
func TestRecheckHoldsBackBelowFloorTransactions(t *testing.T) {
522+
// Push the gas tier fork past the test chain so the floor resolves to the
523+
// baseline tier: params.TestChainConfig schedules Gas50x at block 0, which
524+
// would put the floor at 50x from genesis on. The field cannot be cleared
525+
// instead, CheckConfigForkOrder requires it. Gas2500xBlock stays nil, which
526+
// CheckConfigForkOrder accepts; the assignment states it explicitly.
527+
cfg := *params.TestChainConfig
528+
cfg.Gas50xBlock = big.NewInt(1000)
529+
cfg.Gas2500xBlock = nil
530+
531+
env := newTestEnvWithConfig(t, 1, 0, "", &cfg)
532+
defer env.close()
533+
534+
floor := big.NewInt(common.DefaultMinGasPrice)
535+
nonce := env.nonce()
536+
mk := func(n uint64, gasPrice *big.Int) *types.Transaction {
537+
tx, _ := types.SignTx(types.NewTransaction(n, common.Address{0x00}, big.NewInt(1000), params.TxGas, gasPrice, nil), env.signer, key)
538+
return tx
539+
}
540+
// Priced at the floor and one wei below it: the comparison is
541+
// GasPriceIntCmp(floor) < 0, so only the latter is held back.
542+
atFloor := mk(nonce, floor)
543+
belowFloor := mk(nonce+1, new(big.Int).Sub(floor, big.NewInt(1)))
544+
545+
env.tracker.TrackAll([]*types.Transaction{atFloor, belowFloor})
546+
547+
resubmits := env.tracker.recheck(false)
548+
if len(resubmits) != 1 || resubmits[0].Hash() != atFloor.Hash() {
549+
t.Fatalf("unexpected transactions to resubmit: %v", resubmits)
550+
}
551+
// Held back, not dropped: it stays tracked so a lower floor picks it up.
552+
if len(env.tracker.all) != 2 {
553+
t.Fatalf("below-floor transaction must stay tracked, got %d", len(env.tracker.all))
554+
}
555+
}
556+
557+
func TestRecheckResumesAfterFloorDrops(t *testing.T) {
558+
cfg := *params.TestChainConfig
559+
cfg.Gas50xBlock = big.NewInt(5)
560+
561+
env := newTestEnvWithConfig(t, 10, 0, "", &cfg)
562+
defer env.close()
563+
564+
// head=10, so the floor resolves at block 11: the 50x tier is active and a
565+
// baseline-priced transaction is held back.
566+
tx, _ := types.SignTx(types.NewTransaction(
567+
env.nonce(), common.Address{0x00}, big.NewInt(1000), params.TxGas,
568+
big.NewInt(common.DefaultMinGasPrice), nil), env.signer, key)
569+
570+
env.tracker.Track(tx)
571+
if resubmits := env.tracker.recheck(false); len(resubmits) != 0 {
572+
t.Fatalf("transaction below the floor must not be resubmitted: %v", resubmits)
573+
}
574+
if len(env.tracker.all) != 1 {
575+
t.Fatalf("transaction below the floor must stay tracked, got %d", len(env.tracker.all))
576+
}
577+
578+
// Roll the head back before the fork: the floor drops to the baseline tier,
579+
// which is exactly the transaction's price, so it is let through. recheck
580+
// only filters on Forward(pool.Nonce(sender)), which leaves nonce 10 alone
581+
// now that the state nonce has fallen back to 3.
582+
if err := env.chain.SetHead(3); err != nil {
583+
t.Fatalf("failed to roll back the chain: %v", err)
584+
}
585+
if err := env.pool.Sync(); err != nil {
586+
t.Fatalf("failed to sync the txpool: %v", err)
587+
}
588+
589+
resubmits := env.tracker.recheck(false)
590+
if len(resubmits) != 1 || resubmits[0].Hash() != tx.Hash() {
591+
t.Fatalf("transaction must be resubmitted once the floor drops: %v", resubmits)
592+
}
593+
}
594+
595+
// TestRecheckResubmitsSpecialTransactionBelowFloor pins the exemption that keeps
596+
// recheck aligned with admission: special transactions are not priced against
597+
// the floor there either, so a gas tier fork must not strand the ones submitted
598+
// locally, which is what the tracker exists to guard.
599+
func TestRecheckResubmitsSpecialTransactionBelowFloor(t *testing.T) {
600+
cfg := *params.TestChainConfig
601+
cfg.Gas50xBlock = big.NewInt(5)
602+
603+
env := newTestEnvWithConfig(t, 10, 0, "", &cfg)
604+
defer env.close()
605+
606+
// head=10, so the floor resolves at block 11 on the 50x tier. The
607+
// transaction is priced at the baseline tier, which a plain transaction
608+
// would be held back for.
609+
tx, _ := types.SignTx(types.NewTransaction(
610+
env.nonce(), common.BlockSignersBinary, big.NewInt(0), params.TxGas,
611+
big.NewInt(common.DefaultMinGasPrice), nil), env.signer, key)
612+
613+
env.tracker.Track(tx)
614+
615+
resubmits := env.tracker.recheck(false)
616+
if len(resubmits) != 1 || resubmits[0].Hash() != tx.Hash() {
617+
t.Fatalf("special transaction below the floor must still be resubmitted: %v", resubmits)
618+
}
619+
}

core/txpool/txpool.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,24 @@ func (p *TxPool) Nonce(addr common.Address) uint64 {
437437
return p.state.GetNonce(addr)
438438
}
439439

440+
// MinGasPrice returns the gas price floor the pool enforces on the transactions
441+
// it admits. A pooled transaction can only be included from the next block
442+
// onwards, so the floor is resolved at the height of the block pending on top
443+
// of the current head, the height admission validation prices a transaction at.
444+
//
445+
// The head here is the chain's, which advances on block insertion, while
446+
// admission resolves it against the subpool's, which follows on head events.
447+
// The two drift for as long as a head event is in flight, so a floor read in
448+
// that window can differ from the one admission applies. It self-corrects on
449+
// the next recheck, whose period is orders of magnitude longer than the window.
450+
func (p *TxPool) MinGasPrice() *big.Int {
451+
var number *big.Int
452+
if head := p.chain.CurrentBlock(); head != nil {
453+
number = head.Number
454+
}
455+
return params.GetMinGasPrice(pendingBlockNumber(number), p.chain.Config())
456+
}
457+
440458
// Stats retrieves the current pool stats, namely the number of pending and the
441459
// number of queued (non-executable) transactions.
442460
func (p *TxPool) Stats() (int, int) {

core/txpool/txpool_local_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,3 +337,36 @@ func TestAddLocalTemporaryRejectWithoutTrackerReturnsError(t *testing.T) {
337337
t.Fatalf("unexpected call order: have %v", events)
338338
}
339339
}
340+
341+
// TestAddLocalDoesNotTrackMinGasPriceRejectedTx checks that a local transaction
342+
// the pool rejects for being priced below the gas schedule floor is not tracked:
343+
// the floor only rises as the chain advances, so re-submitting the very same
344+
// transaction could never succeed. The error is still surfaced to the caller.
345+
func TestAddLocalDoesNotTrackMinGasPriceRejectedTx(t *testing.T) {
346+
events := []string{}
347+
tracker := &testLocalTracker{events: &events}
348+
subpool := &testSubPool{
349+
events: &events,
350+
addErrs: []error{ErrUnderMinGasPrice},
351+
}
352+
353+
pool, err := New(0, testChain{}, []SubPool{subpool})
354+
if err != nil {
355+
t.Fatalf("failed to create txpool: %v", err)
356+
}
357+
defer pool.Close()
358+
359+
pool.SetLocalTracker(tracker)
360+
361+
tx := types.NewTransaction(0, common.Address{0x1}, big.NewInt(1), 21000, big.NewInt(1), nil)
362+
err = pool.AddLocal(tx, true)
363+
if !errors.Is(err, ErrUnderMinGasPrice) {
364+
t.Fatalf("unexpected error: have %v, want %v", err, ErrUnderMinGasPrice)
365+
}
366+
if len(tracker.tracked) != 0 {
367+
t.Fatalf("tracker must not receive a tx rejected by the gas price floor: %v", tracker.tracked)
368+
}
369+
if !reflect.DeepEqual(events, []string{"add"}) {
370+
t.Fatalf("unexpected call order: have %v", events)
371+
}
372+
}

0 commit comments

Comments
 (0)