core/txpool: declare `address already reserved` error #29095 #1920

gzliudan · 2026-01-06T02:08:37Z

Proposed changes

Ref: ethereum#29095

Types of changes

What types of changes does your code introduce to XDC network?
Put an ✅ in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)
Regular KTLO or any of the maintaince work. e.g code style
CICD Improvement

Impacted Components

Which part of the codebase this PR will touch base on,

Put an ✅ in the boxes that apply

Checklist

Put an ✅ in the boxes once you have confirmed below actions (or provide reasons on not doing so) that

This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage
Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet.
Tested the backwards compatibility.
Tested with XDC nodes running this version co-exist with those running the previous version.
Relevant documentation has been updated as part of this PR
N/A

…m#27471

coderabbitai · 2026-01-06T02:08:44Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR introduces an address reservation mechanism for the transaction pool system to prevent concurrent management of the same account across different subpools. The changes include declaring new errors (ErrAlreadyReserved and ErrAccountLimitExceeded), introducing a LazyTransaction type for deferred transaction loading, and relocating transaction ordering logic from the core/types package to the miner package.

Key Changes:

Adds address reservation functionality with AddressReserver callback to ensure exclusive subpool access to accounts
Introduces LazyTransaction wrapper type for efficient transaction handling with deferred resolution
Moves transaction ordering logic (transactionsByPriceAndNonce) from core/types to miner package to support LazyTransaction

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
core/txpool/errors.go	Declares new error types `ErrAlreadyReserved` and `ErrAccountLimitExceeded`
core/txpool/txpool.go	Implements address reservation tracking with `reserver` method and reservation map
core/txpool/subpool.go	Defines `LazyTransaction` type and `AddressReserver` function signature
core/txpool/validation.go	Adds account slot limit validation using `UsedAndLeftSlots` callback
core/txpool/legacypool/legacypool.go	Integrates reservation mechanism into add/remove operations and implements slot tracking
miner/ordering.go	New file containing transaction ordering logic adapted for `LazyTransaction`
miner/ordering_test.go	New file with comprehensive tests for transaction ordering including EIP-1559 support
miner/worker.go	Updates to use `LazyTransaction` and new ordering functions
eth/protocol.go	Updates `txPool` interface to return `LazyTransaction` from `Pending`
eth/sync.go	Resolves lazy transactions when syncing
eth/api_backend.go	Resolves lazy transactions when fetching pool transactions
eth/helper_test.go	Updates test helper to return `LazyTransaction` from `Pending`
core/types/transaction.go	Removes old ordering logic, adds `SetTime` and `Time` methods
core/types/transaction_test.go	Removes tests moved to `miner/ordering_test.go`
core/txpool/legacypool/legacypool_test.go	Updates all test `Init` calls to include address reserver
core/txpool/legacypool/legacypool2_test.go	Updates test `Init` calls to include address reserver

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T02:12:55Z

core/txpool/validation.go

 			return fmt.Errorf("%w: balance %v, queued cost %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, balance, spent, cost, new(big.Int).Sub(need, newBalance))
 		}
+		// Transaction takes a new nonce value out of the pool. Ensure it doesn't
+		// overflow the number of permitted transactions from a single accoun


The comment contains a typo: "accoun" should be "account".

Suggested change

// overflow the number of permitted transactions from a single accoun

// overflow the number of permitted transactions from a single account

Copilot · 2026-01-06T02:12:55Z

miner/ordering.go

+// price sorted transactions in a nonce-honouring way.
+//
+// Note, the input map is reowned so the caller should not interact any more with
+// if after providing it to the constructor.


The comment has a grammatical error: "if after providing it to the constructor" should be "it after providing it to the constructor".

Suggested change

// if after providing it to the constructor.

// it after providing it to the constructor.

Copilot · 2026-01-06T02:12:56Z

miner/ordering.go

+			if tx := lazyTx.Resolve(); tx.Tx.IsSpecialTransaction() {
+				specialTxs = append(specialTxs, tx.Tx)
+			} else {
+				normalTxs = append(normalTxs, lazyTx)
+			}


There's a potential nil pointer dereference. If lazyTx.Resolve() returns nil, the code will panic when accessing tx.Tx.IsSpecialTransaction(). A nil check should be added before accessing tx.Tx.

Copilot · 2026-01-06T02:12:56Z

miner/ordering.go

+	i_price := s.txs[i].fees
+	if tx := s.txs[i].tx.Resolve(); tx != nil && tx.Tx.To() != nil {
+		if _, ok := s.payersSwap[*tx.Tx.To()]; ok {
+			i_price = common.TRC21GasPrice
+		}
+	}
+
+	j_price := s.txs[j].fees
+	if tx := s.txs[j].tx.Resolve(); tx != nil && tx.Tx.To() != nil {
+		if _, ok := s.payersSwap[*tx.Tx.To()]; ok {
+			j_price = common.TRC21GasPrice
+		}
+	}
+
+	// If the prices are equal, use the time the transaction was first seen for
+	// deterministic sorting
+	cmp := i_price.Cmp(j_price)


Variable naming uses snake_case (i_price, j_price) which is inconsistent with Go naming conventions. These should be named using camelCase (iPrice, jPrice) to follow Go style guidelines.

Suggested change

i_price := s.txs[i].fees

if tx := s.txs[i].tx.Resolve(); tx != nil && tx.Tx.To() != nil {

if _, ok := s.payersSwap[*tx.Tx.To()]; ok {

i_price = common.TRC21GasPrice

}

}

j_price := s.txs[j].fees

if tx := s.txs[j].tx.Resolve(); tx != nil && tx.Tx.To() != nil {

if _, ok := s.payersSwap[*tx.Tx.To()]; ok {

j_price = common.TRC21GasPrice

}

}

// If the prices are equal, use the time the transaction was first seen for

// deterministic sorting

cmp := i_price.Cmp(j_price)

iPrice := s.txs[i].fees

if tx := s.txs[i].tx.Resolve(); tx != nil && tx.Tx.To() != nil {

if _, ok := s.payersSwap[*tx.Tx.To()]; ok {

iPrice = common.TRC21GasPrice

}

}

jPrice := s.txs[j].fees

if tx := s.txs[j].tx.Resolve(); tx != nil && tx.Tx.To() != nil {

if _, ok := s.payersSwap[*tx.Tx.To()]; ok {

jPrice = common.TRC21GasPrice

}

}

// If the prices are equal, use the time the transaction was first seen for

// deterministic sorting

cmp := iPrice.Cmp(jPrice)

Copilot · 2026-01-06T02:12:56Z

core/txpool/legacypool/legacypool.go

 	if tx.IsSpecialTransaction() && pool.IsSigner(from) && pool.pendingNonces.get(from) == tx.Nonce() {
 		return pool.promoteSpecialTx(from, tx, isLocal)


The promoteSpecialTx function is called as an early return at line 722, bypassing the reservation logic that starts at line 731. This means special transactions from new addresses won't reserve the address, potentially causing issues when the account later adds normal transactions. The reservation logic should be moved before the special transaction check, or the special transaction path should also handle reservations.

Copilot · 2026-01-06T02:12:56Z

core/txpool/errors.go

+	// ErrAlreadyReserved is returned if the sender address has a pending transaction
+	// in a different subpool. For example, this error is returned in response to any
+	// input transaction of non-blob type when a blob transaction from this sender
+	// remains pending (and vice-versa).
+	ErrAlreadyReserved = errors.New("address already reserved")


The new ErrAlreadyReserved error and address reservation mechanism lack test coverage. No tests verify the behavior when an address is already reserved by a different subpool, or when concurrent reservations are attempted. Tests should be added to verify this critical synchronization mechanism works correctly.

Copilot · 2026-01-06T02:12:57Z

miner/ordering_test.go

+
+	txs := types.Transactions{}
+	for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
+		txs = append(txs, tx.Tx.Tx)


The test is directly accessing tx.Tx.Tx at line 104 without checking if tx.Tx is nil. While the test sets up the LazyTransaction with Tx already populated, this doesn't match the actual usage pattern where LazyTransaction.Resolve() should be called to safely retrieve the underlying transaction. The test should call tx.Resolve() instead of directly accessing tx.Tx.

Suggested change

txs = append(txs, tx.Tx.Tx)

resolvedTx, err := tx.Resolve()

if err != nil {

t.Fatalf("failed to resolve lazy transaction: %v", err)

}

txs = append(txs, resolvedTx)

Copilot · 2026-01-06T02:12:57Z

core/txpool/legacypool/legacypool.go

 // removeTx removes a single transaction from the queue, moving all subsequent
 // transactions back to the future queue.
+//
+// In unreserve is false, the account will not be relinquished to the main txpool


The comment has inconsistent capitalization: "In unreserve is false" should be "If unreserve is false". Additionally, this appears to be a grammatical error where "In" should be "If".

Suggested change

// In unreserve is false, the account will not be relinquished to the main txpool

// If unreserve is false, the account will not be relinquished to the main txpool

Copilot · 2026-01-06T02:12:57Z

core/txpool/legacypool/legacypool.go

 	addr, _ := types.Sender(pool.signer, tx) // already validated during insertion

+	// If after deletion there are no more transactions belonging to this account,
+	// relinquish the address reservation. It's a bit convoluted do this, via a


The comment has a grammatical error: "It's a bit convoluted do this" should be "It's a bit convoluted to do this".

Suggested change

// relinquish the address reservation. It's a bit convoluted do this, via a

// relinquish the address reservation. It's a bit convoluted to do this, via a

Copilot · 2026-01-06T02:12:57Z

core/txpool/legacypool/legacypool.go

+			if list := pool.queue[addr]; list != nil {
+				have += list.Len()
+			}
+			return have, math.MaxInt


The UsedAndLeftSlots callback always returns math.MaxInt for the left slots, which means the account limit check in validation.go line 223 will never trigger. This makes the ErrAccountLimitExceeded error unreachable in the legacy pool implementation. If account limits should be enforced, this should return a proper limit value; otherwise, the validation check may be unnecessary for this pool type.

Suggested change

return have, math.MaxInt

// Enforce per-account slot limit using the shared txpool configuration.

accountLimit := int(txpool.DefaultAccountSlots)

if have >= accountLimit {

return have, 0

}

return have, accountLimit - have

gzliudan added 3 commits January 5, 2026 18:37

core/txpool/legacypool: handle missing head in reset ethereum#27479

5ffd91b

core/txpool/legacypool: narrow down the scope of the variable ethereu…

4d62cb0

…m#27471

core, eth, miner: 4844 blob transaction pool ethereum#26940

681699f

gzliudan requested review from AnilChinchawale, anunay-xin, benjamin202410, Copilot, liam-lai and wanwiset25 January 6, 2026 02:08

Copilot started reviewing on behalf of gzliudan January 6, 2026 02:09 View session

core/txpool: declare address already reserved error ethereum#29095

f7163ab

gzliudan force-pushed the txpool-err branch from c0381ee to f7163ab Compare January 6, 2026 02:09

gzliudan changed the title ~~core/txpool: declare 'already reserved' error #29095~~ core/txpool: declare address already reserved error #29095 Jan 6, 2026

Copilot AI reviewed Jan 6, 2026

View reviewed changes

anunay-xin approved these changes Jan 8, 2026

View reviewed changes

	// overflow the number of permitted transactions from a single accoun
	// overflow the number of permitted transactions from a single account

	// if after providing it to the constructor.
	// it after providing it to the constructor.

		if tx.IsSpecialTransaction() && pool.IsSigner(from) && pool.pendingNonces.get(from) == tx.Nonce() {
		return pool.promoteSpecialTx(from, tx, isLocal)

-		txs = append(txs, tx.Tx.Tx)
+		resolvedTx, err := tx.Resolve()
+		if err != nil {
+			t.Fatalf("failed to resolve lazy transaction: %v", err)
+		}
+		txs = append(txs, resolvedTx)

	// In unreserve is false, the account will not be relinquished to the main txpool
	// If unreserve is false, the account will not be relinquished to the main txpool

	// relinquish the address reservation. It's a bit convoluted do this, via a
	// relinquish the address reservation. It's a bit convoluted to do this, via a

-			return have, math.MaxInt
+			// Enforce per-account slot limit using the shared txpool configuration.
+			accountLimit := int(txpool.DefaultAccountSlots)
+			if have >= accountLimit {
+				return have, 0
+			}
+			return have, accountLimit - have

core/txpool: declare address already reserved error #29095 #1920

Are you sure you want to change the base?

core/txpool: declare address already reserved error #29095 #1920

Uh oh!

Conversation

gzliudan commented Jan 6, 2026

Proposed changes

Types of changes

Impacted Components

Checklist

Uh oh!

coderabbitai bot commented Jan 6, 2026

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

core/txpool: declare `address already reserved` error #29095 #1920

core/txpool: declare `address already reserved` error #29095 #1920