EIP-8141: revalidate dependency-affected frame transactions on a new head

[Marchhill]

Changes

Closes the biggest correctness hole in the frame-tx mempool stack: a frame transaction was validated once, at submit, and never rechecked. FrameTxDependencySet was constructed by the payer resolver and never consumed — its doc comment promised "a later layer". This is that layer.

• FrameTxDependencyIndex — maps the chain-head accounts a pending prefix depends on (sender, resolved payer, and the expiry verifier when an expiry frame is present) back to the transaction. Maintained from the pool's Inserted / Removed events, so it can never outlive pool membership.

• Head-change revalidation — the new block's changed-account list is intersected with the index before it is disposed, and only that subset is re-resolved after included and expired transactions have left. A head whose change list does not describe everything that moved — a reorg, or a non-sequential block — falls back to every indexed prefix; that is the same completeness test the account cache uses, computed once so the two cannot drift. Revalidating the whole pool per head would be its own denial-of-service vector, which is the point of the index.

• Invalid-against-head eviction — a prefix that no longer resolves a payer, or whose payer can no longer cover the pool's exposure to it, is evicted immediately. That is the first tier of the spec's eviction order: such transactions never compete for pool space at all.

• Reservation follows the payer — a revalidation that resolves a different payer releases the old reservation and takes a new one against the new payer's balance, evicting if it does not fit.

• FrameTxSimulationResult.Indeterminate — a simulation rejected by a resource bound (busy, per-head budget spent, timed out) says nothing about validity. Admission still declines, but revalidation leaves the transaction pending; otherwise an exhausted budget would turn into a mass eviction.

• Near-expiry shedding — when the pool is full at a head boundary, the pending frame transactions whose deadline is within roughly two slots are shed, nearest deadline first and lowest effective priority fee first among equals. That is the spec's second and third eviction tiers applied where they carry real information: a transaction about to expire is worth little, so it yields its slot rather than displacing a live one through the pool's fee-ordered capacity eviction.

The EIP8141-GAP list in NotSupportedTxFilter loses the items now implemented and keeps canonical-paymaster reservation, the failed-APPROVE replay bound, and a deadline-ordered pool index.

Scope

• Account granularity, not slot. A conservative superset of the spec's wording: a write to a sender storage slot also changes that sender's account, so the account index subsumes the slot index.
• Helper contracts are not indexed. An opaque prefix may read the code of contracts it reaches through CALL*/EXTCODE*; those are not yet dependencies, so a code change at one does not trigger revalidation. Marked EIP8141-GAP.
• The deadline order is not applied across the whole pool. The pool's eviction order is a single IComparer<Transaction> over all transaction types. A frame-only expiry tiebreak that returns 0 for mixed pairs is not transitive and would corrupt the sorted pool; a total order would rank every expiring frame transaction below every other transaction, which is a policy change well beyond the spec. Doing it properly needs a deadline-ordered index inside TxDistinctSortedPool and an eviction-preference hook on SortedPool, so this PR ships the bounded form instead: expired transactions are dropped proactively on each head, and the near-expiry shed pass covers the ordering that matters under pressure. What remains unordered is the deadline ranking among frame transactions with real life left.

Types of changes

What types of changes does your code introduce?

• New feature (a non-breaking change that adds functionality)

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

Notes on testing

TxPoolTests covers a sponsored frame transaction evicted once its prefix stops validating against the new head, one retained when the simulation failed on a resource bound, one left untouched when the block's changed accounts miss its dependency set (asserting the simulator is not consulted), a reorg revalidating a transaction its change list never mentions, and a sponsor able to fund again after an eviction released its reservation.

Nethermind.TxPool.Test 726 passed / 1 skipped; Nethermind.Evm.Test frame suites 147 passed. dotnet format whitespace clean.

Documentation

Requires documentation update

• No

Requires explanation in Release Notes

• No

[claude]

Claude finished @Marchhill's task in 5m 29s —— View job

---

Review: EIP-8141 dependency revalidation

• Read the PR diff
• Read surrounding TxPool / simulator / exposure-cache context
• Analyze correctness, thread-safety, performance
• Post review (7 inline comments)

The design is right and the layering is clean — FrameTxDependencyIndex maintained purely off the pool's Inserted/Removed events is a good choice (it structurally cannot outlive pool membership), the Indeterminate split is a genuinely necessary distinction, and driving revalidation from the block's changed-account list rather than the whole pool is the correct answer to the obvious DoS. The EIP8141-GAP bookkeeping and the scope section in the description are honest about what's left.

That said, I found one issue I'd call blocking and several that should be resolved before merge. The headline one is that the new eviction path leaks payer reservations, and its effects compound with two of the others.

Findings

#	Severity	Where	Issue
1	Critical	TxPool.cs:609-613	Payer-exposure reservation permanently leaked on 4 of 5 eviction paths
2	High	TxPool.cs:578-582	Reorg / non-sequential fallback never fires — guard tests the wrong condition
3	High	TxPool.cs:644	Unbounded synchronous EVM simulation inside the _newHeadLock write lock
4	Medium	FrameTxDependencyIndex.cs:50-56	Empty-bucket pruning race silently drops live index entries
5	Medium	TxPool.cs:656-659	Index not refreshed when revalidation moves the payer
6	Medium	TxPool.cs:651-654	Same-payer check wipes a payer's entire pending set, not the marginal txs
7	Medium	FrameTxPrefixSimulator.cs:146-152	A thrown exception isn't a resource bound — Indeterminate retains it forever
8	Low	TxPool.cs:296-303	Double array allocation + duplicate HasExpiryDeadline parse per insert

Critical: the reservation leak (finding 1)

Worth expanding here because it's the merge-blocker. RevalidateFrameTransactions clears tx.PayerAddress unconditionally before RemoveTransaction, which makes ReleasePayerExposure (line 968, early-returns on a null payer) a no-op. But TryRevalidateFrameTransaction only released the old reservation on one of its five false returns:

TryCalculateMaxCost fails      -> no Subtract  -> leaked
NoPayer                        -> no Subtract  -> leaked
simulator rejected (definite)  -> no Subtract  -> leaked
same payer, over balance       -> no Subtract  -> leaked
payer moved, TryReserve fails  -> Subtract ran -> correct  <- the only case the comment describes

PayerExposureCache is monotonic apart from Subtract, so this is permanent. It's also remotely triggerable and self-amplifying: fill a paymaster's exposure headroom with sponsored frame txs, make the prefix stop resolving, and the eviction pass leaves that paymaster showing a full reservation with zero pending transactions — it can never sponsor again for the process lifetime. Finding 6 then guarantees the whole set goes in that first pass rather than the surplus, and because GetReserved never drops afterwards, every future submission for that payer is rejected at admission too.

The fix is small — move the PayerAddress = null into the one branch that earned it (suggested diff in the inline comment).

Testing

The three new tests are well-chosen for the paths they cover (definite rejection evicts, indeterminate retains, unrelated change list doesn't consult the simulator — the DidNotReceive assertion is the right shape). The gaps line up with the findings above:

• no assertion that an eviction releases the payer reservation (finding 1 — this is the test that would have caught it; asserting the same payer can sponsor again afterwards is enough);
• no reorg case, i.e. PreviousBlock is not null with a non-null AccountChanges (finding 2);
• no payer-move case at all — neither that the reservation follows the payer, which is a headline bullet in the description, nor that the index follows it (finding 5);
• no concurrent insert/remove exercise of FrameTxDependencyIndex (finding 4).

Smaller notes

• Simulate(tx) at line 644 doesn't pass a cancellation token, so a shutdown during a revalidation sweep waits it out. _cts.Token is right there and the interface already accepts one — the XML doc says the interpreter polls it.
• Metrics.FrameTxRevalidations counts every re-resolve including the native fast path, while the metric description says "revalidated because a new head touched their tracked dependencies". That reads fine, but a separate counter for simulator-backed revalidations would make the cost of finding 3 visible in production rather than inferred.
• RevalidateFrameTransactions returns early on the IsEip8141Enabled check without clearing _frameTxsToRevalidate. Harmless today since CollectFrameTxsToRevalidate clears on entry, but it leaves the field holding stale hashes between heads; clearing at the top of the guard would make the lifecycle local.
• NotSupportedTxFilter's trimmed EIP8141-GAP comment is accurate against what's implemented here, assuming findings 2 and 3 are resolved — as written, "dependency-set revalidation" is only partly delivered (reorgs missed, and the fallback path is unbounded).

Verdict

Not mergeable as-is: finding 1 is a remotely-triggerable permanent DoS on any paymaster, and findings 2/3 mean the feature both misses the reorg case it claims to cover and reintroduces the head-processing stall it sets out to avoid. Findings 1, 2 and 5 are each a few lines. Findings 3, 4, 6 and 7 are design calls that need either a fix or an explicit rationale in the code.

I have not built or run the test suite here (review only, no changes pushed).
· branch eip8141-dependency-revalidation

[Marchhill]

The eight table findings all had inline threads and are resolved there. Responding here to the "Smaller notes", which did not:

• Simulate(tx) without a cancellation token — fixed; the sweep now passes _cts.Token, so shutdown is not held behind it.
• _frameTxsToRevalidate not cleared on the early return — fixed; the guard clears before returning, so the field never holds stale hashes between heads.
• Metrics.FrameTxRevalidations counts the native fast path too — kept as one counter. The metric measures how much of the pool a head forced back through resolution, which is the number that says whether the index is doing its job; simulator-backed cost is already visible through FrameTxSimulations.
• EIP8141-GAP accuracy — the two findings it was contingent on (reorg fallback, head-lock cost) are resolved, so the trimmed text holds.

Separately, a summary-only note from #12624 applies here and is now fixed: an EIP-7702-delegated sender runs the delegate's code, so the delegate is a dependency of the prefix; the sender's own code hash only pins the designation. IndexFrameTxDependencies now indexes it, gated on the account carrying code so a codeless sender still costs one account read.

[Marchhill]

@claude review

[claude]

Claude finished @Marchhill's task in 7m 25s —— View job

---

Review: EIP-8141 dependency revalidation (round 2)

• Read the PR diff vs origin/eip8141-simulation-guards
• Verify round-1 findings are resolved
• Fresh pass: correctness, thread-safety, performance
• Post review (5 inline comments)

Round-1 findings — all resolved

#	Round-1 finding	Verdict
1	Reservation leaked on 4 of 5 eviction paths	Fixed. exposureReleased is now an out param and PayerAddress is cleared only on the branch where Subtract already ran. I walked all four previousPayer/payer null combinations — release happens exactly once in each.
2	Reorg fallback never fired	Fixed. changeListIsComplete is computed once and drives both _accountCache and CollectFrameTxsToRevalidate; they cannot drift. Reorg test added.
3	Unbounded simulation under the write lock	Acknowledged, and the rebuttal is correct. Admission takes the _newHeadLock read lock, head processing the write lock, so Monitor.TryEnter in the simulator is uncontended during a sweep — my O(affected) × 250 ms claim does not hold, and per-head work stays bounded by the budget. _cts.Token is now threaded through.
4	Empty-bucket pruning race	Fixed properly. Replacing the lock-free index with a Lock over Dictionary/HashSet removes the reference-equality trap entirely rather than patching it.
5	Index not refreshed on a payer move	Fixed for the payer — but see finding B below: the head commit added a second mutable dependency that the same code path still misses.
6	Same-payer check wiped the whole pending set	Fixed as a consequence of #1, with the policy stated in <remarks>.
7	Thrown exception classified Indeterminate	Fixed. Back to a definite Reject once the tracer exists, with the reasoning in the comment; env-level failures stay indeterminate.
8	Double array allocation	Fixed (single sized pass). The duplicate HasExpiryDeadline parse is knowingly deferred.

The "smaller notes" replies are all reasonable, including keeping FrameTxRevalidations as one counter.

New findings

#	Severity	Where	Issue
A	Medium	TxPool.cs:642	Blob-carrying frame txs are indexed but never revalidated
B	Medium	TxPool.cs:699	Re-index only on a payer move leaves the delegate dependency permanently stale
C	Medium	TxPool.cs:650	Evicted hash stays in the long-term _hashCache, so a reversible eviction is permanent
D	Low	TxPool.cs:322-332	DelegationTargetOf bypasses the account cache on the admission path
E	Low	TxPool.cs:690-700	<remarks> overstate the shed order; spec-source split; eviction metric gap

A — blob pool. OnInsertedTx is wired to _transactions and _blobTransactions (lines 156–167), and NotSupportedTxFilter only rejects blob-carrying frame txs under persistent blob storage. Under BlobsSupportMode.InMemory such a tx is indexed and collected every head, then silently skipped because line 642 only consults _transactions — so for that config the feature is a no-op. Both neighbours in the same file already handle both pools: RemoveExpiredFrameTransactions sweeps both snapshots, and ContainsTx carries the comment "a type-6 frame tx may carry blobs (blob pool) or not (normal pool), so check both." Non-default config, hence Medium rather than High.

B — the new delegate dependency. This is finding 5's twin, opened up by the head commit. IndexFrameTxDependencies snapshots the sender's EIP-7702 delegation target, but re-indexing still happens only when previousPayer != payer. A sender that delegates (or re-delegates) after admission changes its own account, so the tx is collected and revalidated — but if the payer resolves the same, the method returns at line 699 without re-indexing, and the new delegate never becomes a tracked dependency. A code change at the account whose code the prefix actually runs then triggers nothing. Re-indexing on every successful revalidation costs one TryGetAccount and closes it. Also: no test exercises the delegate path at all — every SponsoredFrameTx sender is codeless, so DelegationTargetOf returns null in all five new cases.

C — hash cache. RemoveTransaction doesn't touch _hashCache, so an evicted tx is AlreadyKnown until it ages out of _longTermCache. That is deliberate and correct for expiry (monotonic, and documented as such at line 596), but revalidation invalidity is head-relative and reverses — a sponsor whose balance dips for one block cannot resubmit any of the transactions it lost. AddCore's capacity eviction already calls DeleteFromLongTerm for exactly this reason. Note the new reservation-release test resubmits under a different sender, which is what hides this.

Testing

The five new tests cover their paths well, and the reservation-lifecycle and reorg cases requested last round are both there. Remaining gaps map onto the findings: no blob-pool case (A), no delegation case at all (B), no "resubmittable after the condition clears" case (C).

I could not build or run the suite in this environment (dotnet build was not permitted), so the analysis is static; the author reports Nethermind.TxPool.Test 726 passed / 1 skipped.

Verdict

Much improved — the round-1 blockers are genuinely fixed, and #4 and #7 were fixed by removing the risky construct rather than patching it. Nothing here is a merge blocker on its own: A and B are silent coverage holes in the new feature rather than regressions, and C is a behavioural sharp edge inherited from the expiry path. All three are small changes, so I'd resolve them before merge rather than as follow-ups.
· branch eip8141-dependency-revalidation

[Marchhill]

@claude review

[claude]

Claude finished @Marchhill's task in 7m 15s —— View job

---

Review: EIP-8141 dependency revalidation (round 3)

• Read the PR diff vs origin/eip8141-simulation-guards
• Verify round-2 findings (A–E) are resolved
• Fresh pass over the new commits (near-expiry shedding, shed horizon, delegate dependency)
• Post review (5 inline comments)

Round-2 findings — all resolved

#	Round-2 finding	Verdict
A	Blob-carrying frame txs indexed but never revalidated	Fixed. The lookup falls through to _blobTransactions (line 641–645) with the convention comment, matching RemoveExpiredFrameTransactions and ContainsTx.
B	Re-index only on a payer move left the delegate stale	Fixed, and better than suggested. The TryRevalidateFrameTransaction wrapper re-indexes on every successful return, so no early exit can skip it; the <remarks> name both the payer and the delegate as head-state snapshots. Revalidation_tracks_a_delegation_installed_after_admission exercises the two-step (delegate installed → later change at the delegate) rather than just asserting the index.
C	Evicted hash stayed in the long-term _hashCache	Fixed. DeleteFromLongTerm on the revalidation path only, with the reversible-vs-monotonic distinction stated at both sites, and a test that resubmits the same tx under the same sender.
D	DelegationTargetOf bypassed the account cache	Fixed for the account read; the GetCode read stays on ReadOnlyStateProvider, which is fair — there is no length/prefix accessor on the interface. See the Low note below on where that read now happens.
E	Remarks overstated the shed order; metric gap	Fixed. Iteration-order caveat stated, the spec-gate/pricing split documented against the expiry sweep, PendingTransactionsEvicted now incremented.

I also re-walked round 1's reservation logic against the current code, since the new re-index wrapper sits on that path: all four previousPayer/payer null combinations still release exactly once (details in the inline note on line 791 — the one wrinkle there is cosmetic, not a leak).

New findings — all in the three commits added since round 2

#	Severity	Where	Issue
1	Medium	TxPool.cs:654-671	The shed pass evicts every candidate, not enough to relieve pressure — the sort is dead work and the remarks overstate
2	Low	TxPool.cs:628-641	Shed pass sees only _transactions, unlike its three neighbours
3	Low	TxPool.cs:67	ExpiryShedHorizonSeconds = 24 bakes a 12 s slot into a chain-agnostic component
4	Low	TxPool.cs:791-795	exposureReleased reported true where nothing was released (doc/naming, not a leak)
5	Low	TxPool.cs:326-336	DelegationTargetOf's code read runs inside the pool's McsLock

1 — the only one I'd hold the merge for. IsFull() is checked once in the guard and never rechecked, so the loop removes all frame transactions inside the horizon. Two things follow. The (Deadline, Fee) sort immediately above decides nothing — every element is removed regardless of position, so the spec's second and third eviction tiers, which this commit exists to deliver, are unreachable; the sort plus a pool-sized ArrayPoolList is per-head work whose output nothing reads. And _isFull is _cacheMap.Count >= _capacity, which is the steady state of a mainnet mempool rather than a spike — so on every head, while the pool is full, every frame transaction within 24 s of its deadline is dropped, even with nothing waiting to enter and even though deadline <= timestamp + 24 still leaves it includable in the next block or two. That last part is what makes the <remarks> sentence "so it never sheds a transaction with useful life left" not hold. SortedPool.Remove calls UpdateIsFull(), so if (!_transactions.IsFull()) break; fixes both halves and is what gives the sort a job.

Testing

The reservation-lifecycle, resubmittability, reorg and delegation cases requested in earlier rounds are all present, and the delegation test is well-constructed — it fails for the right reason (the second head touches only the delegate), rather than asserting index internals. DidNotReceive().Simulate(...) in the no-tracked-dependency case is still the right shape.

The one real gap tracks finding 1: [TestCase(1, true)] / [TestCase(4, false)] both use a single transaction, so they pin down whether the pass runs but not what it selects. Size = 2 with two near-expiry frame transactions, asserting the later deadline (and the higher fee at equal deadlines) survives, is the case that would fail today.

I could not build or run the suite in this environment (dotnet build is not permitted here), so this is static analysis; the author reports Nethermind.TxPool.Test 726 passed / 1 skipped.

Verdict

The revalidation feature itself now looks right to me — round 1 and round 2 findings are genuinely fixed, several by removing the risky construct rather than patching it, and the <remarks> are unusually honest about what is and isn't ordered. The remaining Medium is confined to the newest commit, is a few characters of code, and its main cost today is that the eviction tiers the commit advertises don't actually take effect. Findings 2–5 are small and can be taken or explicitly waved off.
· branch eip8141-dependency-revalidation

[Marchhill]

@claude review

[claude]

Claude finished @Marchhill's task in 10m 20s —— View job

---

Review: EIP-8141 dependency revalidation (round 4)

• Read the PR diff vs origin/eip8141-simulation-guards
• Verify round-3 findings (1–5) are resolved
• Fresh pass over the newest commits
• Post review (3 inline comments)

Round-3 findings — all resolved

#	Round-3 finding	Verdict
1	Shed pass evicted every candidate; sort was dead work	Fixed. if (!pool.IsFull()) break; at line 683, and Shedding_takes_the_nearest_deadline_first_and_stops_at_the_freed_slot pins the selection with two candidates and one slot. See the Low note below on what the break implies for the sort.
2	Shed pass saw only _transactions	Fixed properly. Split into a per-pool helper called for both pools, each gated on its own IsFull() — which is the part that matters, since blob capacity is independent.
3	ExpiryShedHorizonSeconds = 24 bakes in a 12 s slot	Acknowledged, and the rationale now holds. It didn't in round 3; once the shed is bounded by pressure the horizon is only a candidate filter, so a wider window on a fast chain admits more candidates to the deadline order rather than dropping more. Comment says as much.
4	exposureReleased reported true where nothing was released	Fixed, and the rebuttal was right. Narrowing the assignment as I suggested would have introduced a bug: with previousPayer is null and a failed TryReserve, leaving PayerAddress set would make ReleasePayerExposure subtract a reservation never taken. holdsNoReservation is the correct framing and the <param> now covers both cases.
5	DelegationTargetOf's code read under the pool's McsLock	Acknowledged with rationale at the read site. Fair trade: the HasCode gate keeps a codeless sender to one cached account read, and the lifecycle guarantee from staying in the event is worth more.

I also re-walked the round-1/2 fixes against the current text, since the re-index wrapper and the blob-pool fallthrough both sit on the reservation path: all four previousPayer/payer null combinations still release exactly once, TryRevalidateFrameTransaction re-indexes on every successful return so no early exit skips the delegate snapshot, and _hashCache.DeleteFromLongTerm is on the revalidation path only. Lock ordering is one-directional throughout (pool McsLock → FrameTxDependencyIndex._lock, never the reverse), so the new Lock cannot deadlock against inserts.

I checked the load-bearing assumption behind the whole index and it holds: a storage-only write to a dependency account does surface in Block.AccountChanges on both producers — StateProvider.ChangedAddresses() returns every key of _blockChanges, which PersistentStorageProvider.Commit populates through AccountExists for each address whose storage root moved, and BlockAccessListBasedWorldState filters on HasStateChanges, which includes StorageChanges.Length > 0. Account granularity really is a superset of slot granularity here.

New findings

#	Severity	Where	Issue
1	Medium	TxPool.cs:412	Shed pass reads the pressure signal before UpdateBuckets(), so it can shed a live transaction for a slot that frees itself
2	Low	TxPool.cs:673-683	The break makes the shed exactly one tx/head, so the pool-sized sort selects a single minimum and the plural wording overstates
3	Low	TxPool.cs:304-308	GAP note omits block-context dependencies (TIMESTAMP/NUMBER), the one axis that moves every head

1 — the only one I'd resolve before merge. ShedNearlyExpiredFrameTransactions runs at line 412; UpdateBuckets() runs at line 419 and removes transactions — UpdateBucket marks them with a null bottleneck and TxDistinctSortedPool.UpdatePool then calls TryRemove. Those are exactly the transactions the new head invalidated (nonce consumed, balance below value, blob nonce-gap cascade). So on a head where the pool is at capacity and UpdateBuckets is about to drop several stale transactions, this pass has already shed a live frame transaction with up to 24 s of deadline left for a slot nothing needed. Same class as round 3's finding — that one bounded how many are shed, this bounds whether any should be. Frequency is modest, since RemoveProcessedTransactions at line 409 usually takes the pool under capacity first, but the fix is moving one call after UpdateBuckets().

2 is not a correctness problem, just a mismatch between what the code does (free exactly one slot per pool per head — IsFull() is a Count >= Capacity threshold and SortedPool.Remove calls UpdateIsFull()) and what the summary/remarks/PR description imply (a set yielding together). Whichever is intended, a linear min-scan would replace the pool-sized ArrayPoolList + sort if one-per-head stands.

I also chased two things that turned out not to be findings, recorded so they don't get re-raised: mutating tx.PayerAddress on a pooled transaction is safe because the only readers are the pool's own admission filters, which run under the _newHeadLock read lock and so are exclusive with head processing; and nonce gaps opened by these direct RemoveTransaction calls are closed by UpdateBuckets() later in the same head (which is, incidentally, a second reason to keep it before — so if you move the shed pass per finding 1, that ordering stays intact since revalidation stays put).

Testing

Seven new tests, and the cases requested across rounds 1–3 are all present and well-shaped — reservation lifecycle, resubmittability under the same sender, reorg, the two-step delegation case, and the DidNotReceive().Simulate(...) negative. Gaps, in order of what I'd add:

• the fee tiebreak at equal deadlines is unexercised — the two-transaction shed test varies only the deadline, so a.Fee.CompareTo(b.Fee) has no coverage;
• no blob-pool case for either the revalidation lookup (round 2 finding A) or the per-pool shed (round 3 finding 2), so both fixes are static-only;
• nothing covers finding 1's interaction (a full pool holding a transaction UpdateBuckets will drop).

dotnet build is not permitted in this environment, so this is static analysis; the author reports Nethermind.TxPool.Test 726 passed / 1 skipped.

Verdict

This is in good shape. Across four rounds the fixes have consistently been the structural kind rather than patches — the lock-free index replaced outright, holdsNoReservation named for what the caller asks, the shed split per pool — and the <remarks> are unusually candid about which orderings are and aren't delivered. Nothing here is a merge blocker: finding 1 is a bounded over-eviction with a one-line fix, and 2 and 3 are a policy statement and a comment. Worth taking 1 and 2 before merge since both are small; 3 can be waved off if you'd rather keep the GAP list terse.

· branch eip8141-dependency-revalidation ·

[claude]

Medium — the shed pass reads the pressure signal before the head's own bucket cleanup has run, so it can shed a valid transaction to free a slot that was about to free itself.

ShedNearlyExpiredFrameTransactions decides on pool.IsFull(), but UpdateBuckets() at line 419 has not run yet — and it removes transactions: UpdateBucket marks them (updateTx(transactions, tx, null, lastElement)) and TxDistinctSortedPool.UpdatePool then calls TryRemove(_transactionsToRemove[i].Hash!). Those are the transactions the new head invalidated: nonce already consumed by a sibling, sender balance now below tx.Value, and the blob nonce-gap cascade.

So on a head where the pool is at capacity and UpdateBuckets is about to drop, say, five stale transactions, this pass has already shed a live frame transaction with up to 24 s of deadline left for a slot nothing needed. That is the same over-eviction the previous round's break was added to prevent, just one layer up: the break bounds how many are shed, this bounds whether any should be.

Frequency is modest — RemoveProcessedTransactions (line 409) usually drops the whole included set first, so the pool is often already under capacity by the time this runs — but when it does fire it evicts a transaction the node would otherwise have kept, and the fix is to move the call:

Suggested change

	ShedNearlyExpiredFrameTransactions(args.Block);
	RevalidateFrameTransactions(args.Block);

and place ShedNearlyExpiredFrameTransactions(args.Block); immediately after UpdateBuckets(); on line 419. Revalidation must stay where it is (it consumes _frameTxsToRevalidate, and eviction there is not capacity-driven), but shedding is purely a response to pressure and should read the pressure that actually remains.

Fix this →

[claude]

Low — with the break in place the loop can only ever remove one transaction, so the full sort is a O(n log n) way to select a single minimum, and the plural framing overstates the pass.

The break is the right fix for last round's over-eviction, but it makes the shed exactly one transaction per pool per head: IsFull() is _cacheMap.Count >= _capacity and SortedPool.Remove calls UpdateIsFull(), so the first successful RemoveTransaction flips it and the second iteration breaks. Two consequences:

1. Only the minimum is ever read. Sorting a pool-sized ArrayPoolList<(ulong, UInt256, Transaction)> (48 bytes/entry, up to Size entries on a full mainnet pool) to consume element 0 is work the result never justifies — a single linear pass tracking the best (Deadline, Fee) gives the same answer with no list, no sort, and no Comparison<T> indirection. That also removes the second full-pool walk per head this pass adds on top of RemoveExpiredFrameTransactions' snapshot.

2. The wording implies a set. "Sheds the pending frame transactions closest to expiry" (<summary>), "sheds only as far as the pressure goes" (<remarks>), and the PR summary's "the pending frame transactions … are shed, nearest deadline first" all read as if several yield together. What the code does is free exactly one slot per head. That is a defensible policy — it is the smallest step that gives the near-expiry tier any effect at all — but it means the tier's throughput is ~1 tx/slot regardless of how much fee-ordered eviction is happening behind it, which a reader sizing the feature would want stated.

Either is fine; they just shouldn't disagree. If shedding one slot per head is intended, say so and drop the sort in favour of a min-scan. If it is not, the bound wants to be a small headroom target rather than the IsFull() threshold, and then the sort earns its place.

Related, minor: the fee tiebreak is the one part of the order no test pins — Shedding_takes_the_nearest_deadline_first_and_stops_at_the_freed_slot varies only the deadline, so a.Fee.CompareTo(b.Fee) is unexercised. A second [TestCase] with equal deadlines and different MaxPriorityFeePerGas would cover it.

Fix this →

[claude]

Low — the GAP note names one of the two axes the account index cannot see.

Helper contracts reached through CALL* are the right thing to flag, and the account-granularity superset argument holds up: I checked both producers of Block.AccountChanges and a storage-only write does surface the account. StateProvider.ChangedAddresses() returns every key of _blockChanges, which PersistentStorageProvider.Commit populates via AccountExists for each address whose storage root moved; BlockAccessListBasedWorldState filters on HasStateChanges, which includes StorageChanges.Length > 0. So the sender/payer/verifier slot dependencies are genuinely covered by their accounts.

What is not covered, and is not mentioned, is block context. An opaque prefix may branch on TIMESTAMP, NUMBER, or BLOCKHASH — a paymaster that enforces its own deadline inline rather than through an expiry frame is the obvious case. Nothing about such a prefix appears in any change list, so it is never collected, and unlike a helper-contract code change (which at least has a triggering account) the invalidating input moves on every head. RemoveExpiredFrameTransactions covers this for the protocol expiry frame only.

Worth naming in the same sentence so the gap list is complete — one clause, no code change:

Suggested change

	/// <remarks>
	/// EIP-8141 "Direct Evaluation of Protocol-Defined Frames" names the sender, the payer and the
	/// expiry verifier as that set. Helper contracts an opaque prefix reaches through <c>CALL*</c> are
	/// not indexed yet, so a code change at one does not trigger revalidation (EIP8141-GAP).
	/// </remarks>
	/// <remarks>
	/// EIP-8141 "Direct Evaluation of Protocol-Defined Frames" names the sender, the payer and the
	/// expiry verifier as that set. Two kinds of dependency are outside it (EIP8141-GAP): helper
	/// contracts an opaque prefix reaches through <c>CALL*</c>, so a code change at one does not trigger
	/// revalidation; and block context an opaque prefix reads (<c>TIMESTAMP</c>, <c>NUMBER</c>), which no
	/// change list can describe — only the protocol expiry frame is swept, by
	/// <see cref="RemoveExpiredFrameTransactions"/>.
	/// </remarks>

For the record I also chased the EIP-8272 axis and it is not a gap worth listing: a recent-root reference stays valid for RecentRootUsableWindow = 8191 slots (~27 h), far beyond any mempool residency, so ageing out is unreachable in practice — and had it been indexed against RecentRootAddress, whose storage is written every block, every such transaction would be revalidated on every head.

Fix this →