multi: passive Esplora polling for lwwallet

[Roasbeef]

In this PR, we cut the lwwallet's Esplora request rate by an order of
magnitude so it stops getting bounced by mempool.space's HTTP 429 rate
limit on mainnet. The trigger was a single client running off-the-shelf
defaults: with one boarding address and a confirmed UTXO already in
view, every block epoch produced a ~3.5s burst of ListUnspent
retries, on top of two independent goroutines (one for btcwallet, one
for the actor system) each polling GetTipHeight every 5s.

Four atomic commits, each addressing one source of waste. See each
commit message for the per-change rationale w.r.t the incremental
changes; the rest of this body covers the combined picture.

ListUnspent retry budget

The retry on wallet.handleBlockEpoch was tuned for neutrino's
block-processing lag, where btcwallet has to fetch the full block over
P2P and run AddCredit before a freshly confirmed UTXO becomes
visible. We keep that intent (slow backends still get time to catch
up) but move from 10x200ms to 3x1s, which is plenty for any backend
we actually ship and shrinks the per-epoch worst case from ~30 to ~6
backend round trips on the lwwallet path.

Passive default poll interval

The default Esplora poll interval was 5s, which is fine for regtest
where blocks land on demand but bleeds onto mainnet users where the
average inter-block gap is ~600s. We bump the default to 30s, an
order of magnitude under the typical mainnet block interval, and
leave the existing wallet.pollinterval flag for harnesses that mine
blocks themselves.

TipPoller unification

Before this PR the lwwallet ran two parallel tip pollers against the
same Esplora client: EsploraChainService (the chain.Interface
adapter btcwallet uses for sync) and chainsource.ChainBackend (the
event source for the actor system). Each had its own ticker, its own
GetTipHeight, and its own per-block GetBlockHashByHeight +
GetBlockHeader on every detected advance. Two consumers, identical
work, doubled traffic.

We introduce lwwallet.TipPoller as the single tip-detection
authority. One goroutine, one ticker. On each advance it walks
oldHeight+1 -> newHeight, resolves each block's hash and header
once, and fans a TipBlock event out to every active subscriber.
Both EsploraChainService and ChainBackend become passive
subscribers; neither runs its own ticker anymore.

The wallet itself owns the poller's lifecycle: Start it before
either subscriber registers, Stop it after both have torn down. The
standalone darepod path (where a ChainBackend is constructed
without a wallet because auto-unlock hasn't run yet) still works
through the existing NewChainBackend constructor, which now
internally creates and owns its own TipPoller. The new
NewChainBackendWithPoller is the shared-poller entry point the
wallet uses.

One behavioral wrinkle: the prior polling loop ran
checkConfirmations and checkSpends on every tick whether or not
the tip moved, so a confirmation registered against a tx that was
already buried beyond numConfs would fire on the next tick. The
new design only re-checks on tip movement, so we explicitly trigger
a one-shot check at registration time. Same observable behavior, no
busy-polling.

LRU caching for hash-addressed responses

The two old pollers (and any other consumer that fetches a tx or
block by hash) all hit Esplora independently. We add four neutrino
LRU caches in EsploraClient keyed by chainhash.Hash for the
hash-addressed methods: GetBlockHeader, GetRawBlockHeader,
GetRawBlock, and GetRawTx. Capacities are byte-sized via each
value's serialized footprint, so a 4 MiB block costs more cache
budget than an 80-byte header. Reorgs cannot stale these entries
because each is content-addressed by the same hash that uniquely
identifies its body; a stale hit is only possible if the backend
lied about the hash in the first place.

Mutable endpoints (GetTipHeight, GetAddressUtxos, GetTxStatus,
GetFeeEstimates, GetOutspend) deliberately stay uncached. A
stale tip would mask a reorg, a stale UTXO list would mask a pending
spend, etc, etc.

Combined effect

For a single client running against mempool.space:

• Per-block-epoch retry burst: 30 -> 6 backend round trips.
• Steady-state GetTipHeight cadence: 24/min -> 2/min.
• Per-new-block fetches (GetBlockHashByHeight, GetBlockHeader,
  GetRawBlock, GetRawTx): served from the LRU on the second
  consumer rather than re-fetched.

Test plan

• Unit tests pass on lwwallet, wallet, darepod.
• go vet clean on the same packages.
• Run a real client against mempool.space mainnet and confirm
  no 429 responses across a multi-hour window.
• Confirm regtest itest harnesses still detect blocks promptly
  (they override wallet.pollinterval).

[Roasbeef]

@claude review this, @codex review this

[gemini-code-assist]

Code Review

This pull request centralizes chain tip monitoring by introducing a shared TipPoller, which reduces redundant API calls from the wallet's backend components. It also adds LRU caching for transactions, blocks, and headers to the EsploraClient and optimizes notification handling. Feedback identifies several critical areas for improvement: the ChainBackend should avoid full scans when processing new registrations to prevent request bursts, and the TipPoller needs to handle head-of-line blocking and detect reorgs at the same height. Furthermore, caching efficiency can be enhanced by pre-calculating block sizes and implementing singleflight to mitigate cache stampedes.

[gemini-code-assist]

Calling checkConfirmations()—which performs a full scan of all pending registrations—inside a goroutine for every new registration is highly inefficient. If multiple registrations are added in quick succession, this will spawn numerous goroutines, each performing redundant network calls for the entire set of registrations. This could lead to a burst of requests that triggers the very rate limits this PR aims to avoid.

	go func() {
		b.mu.Lock()
		currentHeight := b.bestHeight
		b.mu.Unlock()

		conf := b.checkSingleConf(reg, currentHeight)
		if conf == nil {
			return
		}

		select {
		case reg.confChan <- conf:
			b.mu.Lock()
			delete(b.confRegs, id)
			b.mu.Unlock()
		case <-reg.cancelCh:
		}
	}()

[Roasbeef]

@gemini-code-assist Already addressed in this PR: RegisterConf now spawns runConfOneShot(id, reg) which is scoped to the single new registration (O(1) HTTP per call), not the full-scan checkConfirmations(). The goroutine is tracked in b.wg so Stop() waits for any in-flight one-shot to drain. See chain_backend.go:447-498.

[gemini-code-assist]

Thank you for the clarification. I have reviewed the implementation of runConfOneShot (lines 462-498) and agree that it correctly addresses the efficiency concern by scoping the confirmation check to the individual registration, avoiding the O(N) full-scan overhead.

[gemini-code-assist]

Similar to RegisterConf, calling checkSpends() here triggers a full scan of all spend registrations. This should be optimized to check only the newly added registration to avoid redundant network requests and potential rate-limiting issues.

	go func() {
		if outpoint == nil {
			return
		}

		outspend, err := b.esplora.GetOutspend(
			outpoint.Hash, outpoint.Index,
		)
		if err != nil || !outspend.Spent || !outspend.Status.Confirmed {
			return
		}

		spenderHash, err := chainhash.NewHashFromStr(outspend.Txid)
		if err != nil {
			return
		}

		spendingTx, err := b.esplora.GetRawTx(*spenderHash)
		if err != nil {
			return
		}

		detail := &chainsource.SpendDetail{
			SpentOutPoint:     outpoint,
			SpenderTxHash:     spenderHash,
			SpendingTx:        spendingTx,
			SpenderInputIndex: outspend.Vin,
			SpendingHeight:    int32(outspend.Status.BlockHeight),
		}

		select {
		case reg.spendChan <- detail:
			b.mu.Lock()
			delete(b.spendRegs, id)
			b.mu.Unlock()
		case <-reg.cancelCh:
		}
	}()

[Roasbeef]

@gemini-code-assist Already addressed: RegisterSpend spawns runSpendOneShot(id, reg) scoped to just the new registration. Same b.wg lifecycle as the conf path. See chain_backend.go:543-597.

[gemini-code-assist]

Acknowledged. The implementation of runSpendOneShot correctly addresses the concern by performing an O(1) check for the individual registration, and the use of b.wg ensures proper lifecycle management. No further changes are needed.

[gemini-code-assist]

Performing a blocking send to subscribers within the main polling loop creates a head-of-line blocking risk. If any subscriber (such as EsploraChainService, which performs synchronous network I/O in its handler) is slow to process an event, the entire TipPoller will stall, preventing other subscribers from receiving updates. While the buffer helps, it may not be sufficient during periods of high activity or network latency.

[Roasbeef]

@gemini-code-assist Not applicable to the current implementation. The broadcast path goes through lnd's subscribe.Server (wrapped by EventServer in lwwallet/subscribe.go), which provides a per-client unbounded queue plus a single-goroutine subscriber handler. A slow consumer cannot wedge the broadcaster — its updates queue locally — and there is no send-on-closed race by construction. See the doc comment at subscribe.go:14-20.

[gemini-code-assist]

Thanks for the clarification. You are correct—using lnd's subscribe.Server with per-client unbounded queues effectively mitigates the head-of-line blocking risk I was concerned about. The design ensures that slow consumers do not impact the broadcaster or other subscribers, which is the right approach for this type of event stream.

[gemini-code-assist]

The current logic only detects tip progress if the height increases. However, a chain reorganization can occur at the same height (a 'side-step'). In such a case, newHeight <= oldHeight would be true, and the poller would fail to notify subscribers of the new block hash. This could lead to the wallet remaining on a stale branch until a new block is mined at a higher height.

[Roasbeef]

@gemini-code-assist Documented as a known limitation in TipPoller.poll(). The behavior matches the per-component pollers that preceded the unified TipPoller, and downstream callers re-check status against Esplora on every tip event, so a stale hash at height N converges within one extra tip advance. Treating same-height reorgs as a first-class signal would require tracking (height, hash) rather than just height; deliberately deferred for now since it has not historically caused issues for the confirmation-target use case.

[gemini-code-assist]

Understood. Thank you for the clarification regarding the design trade-offs and the documentation of this limitation.

[gemini-code-assist]

SerializeSize() is an $O(N)$ operation that iterates over all transactions in a block. Since the LRU cache calls Size() during bookkeeping and eviction, calling this repeatedly for large blocks is inefficient. It is better to pre-calculate the size once when the cachedBlock is created and store it in a field.

[Roasbeef]

@gemini-code-assist Fixed. cachedBlock and cachedTx now carry a precomputed size field populated from uint64(len(body)) at Put time in GetRawBlock / GetRawTx. Size() is now O(1) on every LRU bookkeeping or eviction call, with no wire.MsgBlock.SerializeSize() walk. See esplora_cache.go and the Put sites in esplora.go.

[gemini-code-assist]

The pre-calculation of the size field at the cache-fill site effectively resolves the $O(N)$ performance concern. This approach is optimal for the LRU eviction path.

[gemini-code-assist]

This method is susceptible to a 'cache stampede'. If multiple concurrent goroutines request the same block hash before the first request completes and populates the cache, they will all perform redundant HTTP requests to the Esplora backend. Using golang.org/x/sync/singleflight would ensure that only one in-flight request is made for a given hash.

[Roasbeef]

@gemini-code-assist Fixed. Added a singleflight.Group to each of the four content-addressed cache fillers (GetRawBlock, GetRawTx, GetBlockHeader, GetRawBlockHeader). Concurrent misses for the same hash now coalesce into a single HTTP round trip; the work function re-checks the cache before issuing the request so a sibling fetch that populated the cache while we were waiting wins without re-firing. This is real load-bearing now that chainBackend and chainSvc both consume TipPoller events and would otherwise race on every block.

[gemini-code-assist]

The implementation of singleflight.Group correctly addresses the cache stampede issue by coalescing concurrent requests and re-checking the cache within the work function. This pattern is robust and aligns with best practices for memoization in high-concurrency environments.

[claude]

Claude finished @Roasbeef's task in 2m 16s —— View job

---

PR Review: multi: passive Esplora polling for lwwallet

• Read changed files and gather context
• Review lwwallet/tip_poller.go
• Review lwwallet/esplora_cache.go
• Review lwwallet/chain_backend.go
• Review lwwallet/esplora.go and lwwallet/esplora_chain.go
• Review lwwallet/wallet.go, wallet/wallet.go, darepod/config.go

---

Overall this is a solid, well-motivated change. The TipPoller unification and LRU caching are clean abstractions; the retry-budget and poll-interval tuning are well-calibrated. A few items below that are worth addressing before merge, ordered by severity.

---

1. RegisterConf/RegisterSpend immediate-check goroutines scan all regs, not just the new one [HIGH]

lwwallet/chain_backend.go:399 and :451

The comment reads "Run an immediate single-shot check" but the implementation calls the full-scan variants:

go b.checkConfirmations()   // iterates ALL confRegs
go b.checkSpends()          // iterates ALL spendRegs

If N registrations are added concurrently (e.g. during startup for N boarding addresses), this spawns N goroutines each of which issues up to N × (GetTxStatus + GetRawTx) network calls — an O(N²) burst against exactly the rate-limited backend you're fixing. The intent is to check just the one registration that was just added.

The checkSingleConf/checkSpend-for-one pattern already exists in the codebase. Fix for conf (fix →):

// Replace:
go b.checkConfirmations()

// With:
go func() {
    b.mu.Lock()
    currentHeight := b.bestHeight
    b.mu.Unlock()

    conf := b.checkSingleConf(reg, currentHeight)
    if conf == nil {
        return
    }
    select {
    case reg.confChan <- conf:
        b.mu.Lock()
        delete(b.confRegs, id)
        b.mu.Unlock()
    case <-reg.cancelCh:
    }
}()

Apply the analogous change in RegisterSpend: capture reg and id in scope, call the single-outpoint logic, and send directly to reg.spendChan.

---

2. cachedBlock.Size() re-computes SerializeSize() from scratch [MEDIUM]

lwwallet/esplora_cache.go:63

wire.MsgBlock.SerializeSize() walks every transaction's input/output list. At Put time you already have len(body) (the raw HTTP response bytes) available in GetRawBlock. Storing the wire size at construction avoids the walk on every eviction:

type cachedBlock struct {
    block *wire.MsgBlock
    size  uint64
}

func (c cachedBlock) Size() (uint64, error) {
    return c.size, nil
}

// At Put site in GetRawBlock:
_, _ = c.rawBlockCache.Put(blockHash, cachedBlock{
    block: &block,
    size:  uint64(len(body)),
})

Same pattern applies to cachedTx — len(body) from GetRawTx is already the serialized size.

---

3. TipPoller.Start() has a TOCTOU on the started guard [LOW]

lwwallet/tip_poller.go:104

t.mu.Lock()
if t.started { ... }
t.mu.Unlock()         // lock released here

height, err := t.esplora.GetTipHeight()  // network I/O without the lock
// ...
t.mu.Lock()
t.started = true      // set here
t.mu.Unlock()

Two concurrent Start() calls both pass the first lock section and proceed to fetch the initial tip. The second write to t.tipHeight/t.started overwrites the first, and two poll goroutines end up running. In the current call sites the wallet owns Start() and calls it once, so this doesn't bite today, but it's a latent bug. Fix: set started = true (and check it) atomically at the top, under a single lock acquisition, before the network I/O:

t.mu.Lock()
if t.started {
    t.mu.Unlock()
    return fmt.Errorf("tip poller already started")
}
t.started = true   // claim the slot before releasing
t.mu.Unlock()
// ... network I/O proceeds with only one winner ...

---

4. EsploraChainService.GetBestBlock() still makes live HTTP calls [LOW]

lwwallet/esplora_chain.go:131

func (s *EsploraChainService) GetBestBlock() (*chainhash.Hash, int32, error) {
    height, err := s.esplora.GetTipHeight()
    hash, err  := s.esplora.GetBlockHashByHeight(height)
    ...
}

Now that the shared TipPoller keeps a continuously updated tip cache, this method could serve from s.tipPoller.BestBlock() (2 field reads under one mutex) instead of firing 2 HTTP requests. The existing TOCTOU comment in the code explains why GetTipHeight + GetBlockHashByHeight were paired — the poller already solves that by resolving hash at the height it observed. Not a regression, just a missed simplification.

---

5. Reorg blind spot at the same height [INFO / known limitation]

lwwallet/tip_poller.go:245

if newHeight <= oldHeight {
    return
}

A 1-block reorg that replaces a block at height N with a different block at the same height N is invisible until N+1 arrives. This was equally true before this PR (the old per-component pollers had the same check). Worth noting explicitly in either the TipPoller comment or the PR body as a known limitation rather than a future bug report.

---

Minor / Nits

• broadcast head-of-line blocking: The sequential blocking send (with quit fallback) is documented as intentional backpressure. With only two subscribers and a 16-event buffer this is fine for the current workload. Worth a brief mention that adding a third slow subscriber (e.g. one that does synchronous I/O in its handler) could serialize all downstream block processing through a single bottleneck.

• processTipEvent holds mu during channel sends (chain_backend.go:541-557): the default: branch prevents deadlock, but holding mu while executing channel operations is fragile — a future removal of the default case would introduce a deadlock. Consider updating bestHeight/bestHash before the fan-out loop, then dropping the lock before touching the registration maps.

• GetRawTx caches unconfirmed transactions: GetRawTx caches on successful decode without checking status.Confirmed. An unconfirmed tx in the mempool has a stable txid but could be replaced (RBF). Unlikely to matter in practice (callers only call GetRawTx after GetTxStatus confirms it), but the cache comment says "a confirmed transaction is content-addressed by its txid" — worth adding a note that callers are expected to only call this on confirmed txids.

---

Summary: The architecture is right. The only correctness issue worth blocking on is #1 — the full-scan goroutine in RegisterConf/RegisterSpend works against the rate-limiting goal and can be fixed by scoping the goroutine to the single new registration. Items #2 and #3 are recommended cleanup. The rest is low-priority.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08cf5dfba2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Prevent send-on-closed panic in tip subscriber cancel

This cancel path closes a subscriber channel that the poller may still be about to send to: broadcast snapshots t.subs under the mutex, unlocks, then sends later. If a subscriber cancels during that window (for example during shutdown while a new block is being fanned out), the send can hit a closed channel and panic (send on closed channel), crashing the process. Keep close/send ownership on one side (poller) or synchronize cancellation with broadcast.

Useful? React with 👍 / 👎.

[Roasbeef]

@chatgpt-codex-connector Not applicable to the current implementation. The broadcast path was reworked to use lnd's subscribe.Server via the EventServer wrapper (lwwallet/subscribe.go). Its subscriber handler is single-goroutine over Subscribe / SendUpdate / Cancel, so close-channel ownership stays inside the server and there is no send-on-closed race by construction. The per-subscription translate() goroutine reads from the inner client's Updates() channel (closed by the server, never the subscriber) and writes to the typed out channel that it owns and closes itself. See subscribe.go:14-20 and the Cancel() flow at subscribe.go:110-123.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[chatgpt-codex-connector]

Roll back tip poller when wallet startup fails

If BtcWallet.Start() returns an error after tipPoller.Start() succeeds, this function exits without stopping the poller. That leaves the poller goroutine running and keeps tipPoller.started set, so retrying Wallet.Start() on the same instance fails with tip poller already started and leaks startup resources on transient failures. Add cleanup before returning this error.

Useful? React with 👍 / 👎.

[Roasbeef]

@chatgpt-codex-connector Already addressed in this PR. Wallet.Start() builds a rollback []func() slice and pushes w.tipPoller.Stop (and _ = w.BtcWallet.Stop()) after each successful sub-system Start. Any later failure unwinds the slice in reverse via the deferred closure, so a BtcWallet.Start() failure after tipPoller.Start() succeeds tears the poller down before the function returns. The slice is set to nil only on the all-green happy path. See wallet.go:166-202.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[Roasbeef]

@claude review this, @codex review this

[claude]

Claude finished @Roasbeef's task in 4m 1s —— View job

---

Claude finished @Roasbeef's task in ~5m —

---

PR Review: multi: passive Esplora polling for lwwallet

• Read changed files and gather context
• Review lwwallet/tip_poller.go and lwwallet/subscribe.go
• Review lwwallet/esplora_cache.go and lwwallet/esplora.go
• Review lwwallet/chain_backend.go
• Review lwwallet/esplora_chain.go and lwwallet/wallet.go
• Review wallet/wallet.go, darepod/config.go, darepod/server.go
• Synthesize and post final review

---

Good layering across four atomic commits. The architecture is clean and the goals are well-served. One clarification up front on the prior reviews: the previous Claude review's HIGH item (O(N²) full-scan goroutines in RegisterConf/RegisterSpend) and the Gemini HIGH items are already fixed in this version — the current code correctly uses runConfOneShot/runSpendOneShot. What follows is an independent analysis of what remains.

---

1. TipPoller.Start() TOCTOU on started guard [MEDIUM]

lwwallet/tip_poller.go:104-147

The started check is read under lock but the flag is only set much later, after 3 live HTTP calls:

t.mu.Lock()
if t.started { ... }
t.mu.Unlock()       // ← race window opens here

height, err := t.esplora.GetTipHeight()   // concurrent Start() races in here
hash, err := t.esplora.GetBlockHashByHeight(height)
...
t.mu.Lock()
t.started = true    // ← both callers reach this
t.mu.Unlock()

Two concurrent Start() calls both pass the initial guard, both fetch the initial tip, both call t.events.Start() (which subscribe.Server makes idempotent, so that's fine), and both spawn a pollLoop goroutine — resulting in two concurrent poll goroutines writing t.tipHeight/t.tipHash and calling SendUpdate with potentially different snapshots of the chain tip.

The current call sites are single-threaded about Start(), so this is latent rather than immediate. Fix: set started = true before releasing the lock the first time:

t.mu.Lock()
if t.started {
    t.mu.Unlock()
    return fmt.Errorf("tip poller already started")
}
t.started = true   // claim the slot atomically
t.mu.Unlock()

// ... network I/O proceeds with one winner ...

t.mu.Lock()
t.tipHeight = height
// ... (remove the separate started = true)
t.mu.Unlock()

---

2. cachedBlock.Size() walks all transactions on every LRU Put [MEDIUM]

lwwallet/esplora_cache.go:85-91

func (c cachedBlock) Size() (uint64, error) {
    ...
    return uint64(c.block.SerializeSize()), nil  // O(N tx) walk
}

The neutrino LRU calls Size() at Put time (and again at eviction). wire.MsgBlock.SerializeSize() iterates over every transaction's inputs and outputs. The raw HTTP body bytes (len(body)) are available at the Put site in GetRawBlock and represent exactly the budget the cache should charge. Same for cachedTx.Size() — len(body) is on the stack in GetRawTx:

// In GetRawBlock, replace:
if _, putErr := c.rawBlockCache.Put(
    blockHash, cachedBlock{block: &block},
); ...

// With (block struct carries its precomputed size):
type cachedBlock struct {
    block *wire.MsgBlock
    bytes uint64  // len(body) at Put time
}

func (c cachedBlock) Size() (uint64, error) {
    if c.block == nil { return 0, errNilCacheEntry }
    return c.bytes, nil
}

Same pattern for cachedTx. cachedRawHeader is already O(1) (fixed 80), and cachedBlockHeader is also O(1), so those are fine.

---

3. EsploraChainService.GetBestBlock() still fires 2 live HTTP calls [LOW]

lwwallet/esplora_chain.go:132-148

func (s *EsploraChainService) GetBestBlock() (*chainhash.Hash, int32, error) {
    height, err := s.esplora.GetTipHeight()
    hash, err  := s.esplora.GetBlockHashByHeight(height)
    ...
}

This method's original TOCTOU comment ("height fetched first, then hash resolved for that specific height to avoid mismatched values") is now solved by the TipPoller, which resolves the hash at the height it observed and keeps a consistent (height, hash, time) triple under its own mutex. s.tipPoller.BestBlock() returns all three in one lock acquisition. GetBestBlock could drop to zero HTTP round-trips:

func (s *EsploraChainService) GetBestBlock() (*chainhash.Hash, int32, error) {
    height, hash, _ := s.tipPoller.BestBlock()
    return &hash, height, nil
}

Not a regression — just a missed simplification now that the infrastructure is in place.

---

4. WaitForSync issues a live GetTipHeight() call [LOW]

lwwallet/wallet.go:271

func (w *Wallet) WaitForSync(ctx context.Context) error {
    tipHeight, err := w.esplora.GetTipHeight()
    ...
}

One HTTP request per WaitForSync call. Since the TipPoller already has a continuously updated cached tip, w.tipPoller.BestBlock() gives the same value without the round-trip.

---

5. processTipEvent holds b.mu across block registration fan-out [LOW]

lwwallet/chain_backend.go:716-732

b.mu.Lock()
b.bestHeight = event.Height
b.bestHash = event.Hash

for _, reg := range b.blockRegs {
    select {
    case reg.epochChan <- epoch:
    case <-reg.cancelCh:
    default:  // channel full, drop
    }
}
b.mu.Unlock()

The default: case makes each send non-blocking, so this doesn't deadlock today. But holding the lock while executing N channel operations means every other goroutine waiting on b.mu (confirmation checks, new registrations) stalls for the full fan-out. The simpler pattern is to snapshot bestHeight/bestHash, drop the lock, then fan out:

b.mu.Lock()
b.bestHeight = event.Height
b.bestHash = event.Hash
regs := make(map[uint64]*blockRegistration, len(b.blockRegs))
for id, reg := range b.blockRegs { regs[id] = reg }
b.mu.Unlock()

for _, reg := range regs {
    select {
    case reg.epochChan <- epoch:
    case <-reg.cancelCh:
    default:
    }
}

A future maintainer who removes the default: case would introduce a deadlock with the current code; with lock-before-fan-out separated out, that mistake becomes impossible.

---

6. Known limitation: same-height reorgs are invisible [INFO]

lwwallet/tip_poller.go:266

if newHeight <= oldHeight {
    return
}

A block replacement at the same height (newHeight == oldHeight, different hash) goes undetected until the chain advances to height+1. This was equally true before this PR. Worth a brief comment on poll() documenting this as a known limitation so a future reader doesn't file it as a bug.

---

7. darepod interactive-unlock path now defers chain backend entirely [INFO]

darepod/server.go

In the interactive-unlock path (WalletStateLocked), initChainBackend now returns nil (no-op) and defers creation to startLwwallet. This eliminates the double-Esplora-client problem from the old code, which is the right call. The tradeoff is that the chain source actor is also not registered until after unlock, which changes observable startup behavior for users who haven't called UnlockWallet yet. The PR body describes this correctly; just noting it as a behavioral delta to verify in the itest matrix.

---

Summary

The architecture is right and the rate-limiting problem is well-solved. The two items that should be addressed before merge are #1 (TipPoller TOCTOU — low risk today but easy to fix atomically) and #2 (cachedBlock.Size() O(N) on Put — real cost under LRU pressure from 4 MiB mainnet blocks). Items #3 and #4 are simplifications now that the TipPoller infrastructure is in place. Items #5–#7 are low/info.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1be5a223a4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Remove duplicate start of lwwallet chain backend

startLwwallet now calls w.Start() (which already starts w.ChainBackend() inside lwwallet.Wallet.Start) and then, when s.chainBackend == nil, calls s.chainBackend.Start() again. That second start creates another active tip subscription/goroutine for the same backend, which can duplicate block epoch notifications and confirmation/spend rechecks and effectively reintroduce extra Esplora traffic. In lwwallet mode this path is taken on initial startup, so the backend should only be assigned here, not started again.

Useful? React with 👍 / 👎.

[Roasbeef]

@chatgpt-codex-connector Fixed. Removed the second s.chainBackend.Start() call in startLwwallet. Wallet.Start() already starts the chain backend internally (lwwallet/wallet.go:191) as part of its startup sequence, so the deferred-init block in startLwwallet now only assigns s.chainBackend = w.ChainBackend() for the chain source actor registration — no second subscribe + duplicate handleTipEvents goroutine.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.