txnprovider/txpool: extract best's wait loop into a defer-locked helper

[yperbasis]

Follow-up to #23333; to be done after that PR merges.

#23333 fixes a deadlock where best returned early on ctx.Done() while still holding p.lock. The fix is a point-unlock, which leaves best with a manually managed lock region that has several exits:

• p.lock.Lock() at the top of the function
• p.lock.Unlock() inside the wait loop, on the ctx.Done() early return (added by txnprovider/txpool: release the pool lock when a caller stops waiting for a block #23333)
• p.lock.Unlock() after the wait loop, before poolDB.BeginRo (required by the two-lock ordering with the RoTxsLimiter, see the comment in the code)
• p.lock.Lock() + defer p.lock.Unlock() for the rest of the function

History shows this structure is error-prone: #16680 had to replace the original defer-based unlock with manual unlocks (for the RoTx ordering), and #16750 then added an early return in the wait loop and missed the unlock — exactly the deadlock #23333 fixes. The next early exit added to the loop (for example a max-wait deadline or a shutdown-flag check) would again need to hand-roll its own unlock; forgetting it compiles cleanly and passes the existing tests.

Proposal

Extract the wait loop into a helper that owns the lock for its whole scope:

func (p *TxPool) waitForBlock(ctx context.Context, onTopOf uint64) error {
	p.lock.Lock()
	defer p.lock.Unlock()
	for last := p.lastSeenBlock.Load(); last < onTopOf; last = p.lastSeenBlock.Load() {
		select {
		case <-ctx.Done():
			return ctx.Err()
		default:
			// continue
		}
		p.logger.Trace("[txpool] Waiting for block", "expecting", onTopOf, "lastSeen", last,
			"pending", p.pending.Len(), "baseFee", p.baseFee.Len(), "queued", p.queued.Len())
		p.lastSeenCond.Wait()
	}
	return nil
}

best then starts with:

if err := p.waitForBlock(ctx, onTopOf); err != nil {
	return false, 0, err
}
tx, err := p.poolDB.BeginRo(ctx)
if err != nil {
	return false, 0, err
}
defer tx.Rollback()
p.lock.Lock()
defer p.lock.Unlock()

This is behavior-identical: Cond.Wait re-acquires p.lock before it returns, so the helper's deferred unlock replaces both manual unlock sites one-for-one, and the lock is released at exactly the same points as today. Any future early exit from the wait loop gets the unlock for free. The long comment about the RoTxsLimiter two-lock ordering moves to the call site in best, in front of poolDB.BeginRo. The only open detail is the txRequested field of the trace line: pass n into the helper as an extra argument, or drop the field.

TestBestReleasesTheLockWhenTheCallerGivesUpWaitingForABlock (added in #23333) pins the ctx-cancel path and must stay green through the refactor.

Out of scope: waking a caller that is already parked in p.lastSeenCond.Wait() when its context is canceled (today it stays parked until the next block broadcast or shutdown) — that is a separate behavior change.

[VBulikov]

yperbasis added Imp2 as Importance