lnrpc/chainrpc: surface re-org depth and Done over the chain notifier#10943
lnrpc/chainrpc: surface re-org depth and Done over the chain notifier#10943ellemouton wants to merge 2 commits into
Conversation
03d9901 to
a8baef0
Compare
🟠 PR Severity: HIGH
🟠 High (3 files)
🟢 Low (2 files)
AnalysisThe highest-severity files touched are under No bump conditions apply: only 4 non-generated files changed (well under the 20-file threshold), ~92 non-generated/non-doc lines changed (well under the 500-line threshold), and no critical packages (lnwallet, htlcswitch, contractcourt, sweep, etc.) are touched. Since this PR modifies a public RPC API surface (chainnotifier), it warrants review from someone familiar with the lnrpc API conventions and backward-compatibility concerns, consistent with HIGH severity. To override, add a |
a8baef0 to
188d227
Compare
Temporary replace onto the lnd branch that adds the reorg depth and Done events to the ChainNotifier proto (lightningnetwork/lnd#10943). Lands before the code change so each commit builds. Drop this once that lnd change is merged and tagged.
The ChainNotifier confirmation/spend streams previously exposed re-org only as a payload-less event and had no terminal event for reaching re-org safety depth. Add a depth field to the Reorg message and a new Done event to both the ConfEvent and SpendEvent oneofs so the gRPC surface can carry the same information lnd already tracks internally (ConfirmationEvent.NegativeConf depth and the Done channel). This is an additive, backwards-compatible proto change; the server side is wired in a follow-up commit.
The RegisterConfirmationsNtfn/RegisterSpendNtfn handlers read the internal NegativeConf depth and Done channels but dropped both at the gRPC boundary: the re-org depth was discarded and Done was conveyed only by returning (closing the stream), which a caller cannot distinguish from an unrelated teardown. Forward the re-org depth on the confirmation path's Reorg message and send an explicit Done event before exiting on both the confirmation and spend paths. Spend-side re-org depth is left as 0 because the notifier does not currently track it internally.
188d227 to
8b4da07
Compare
Temporary replace onto the lnd branch that adds the reorg depth and Done events to the ChainNotifier proto (lightningnetwork/lnd#10943). Lands before the code change so each commit builds. Drop this once that lnd change is merged and tagged.
Summary
The
ChainNotifierconfirmation/spend gRPC streams drop two pieces ofinformation that lnd already tracks internally:
ConfirmationEvent.NegativeConfcarries the depth as anint32, but the server discarded it and sent an emptyReorg{}(the protomessage was a
// TODO(wilmer)placeholder).ConfirmationEvent.Done/SpendEvent.Donefire oncea watch is past re-org safety depth, but the server conveyed this only by
returning (closing the stream), which a client cannot distinguish from an
unrelated teardown. There was no
Doneevent in theConfEvent/SpendEventoneof at all.
This makes the chain-event surface reversible and terminable end to end, which
reorg-safety-aware consumers (e.g. anything that gates terminal state eviction
on reaching re-org safety depth) need.
Changes
depthfield toReorg, add a newDonemessage, and adddoneto both theConfEventandSpendEventoneofs. Additive andbackwards compatible.
Reorg, and send an explicitDoneevent before exiting on both theconfirmation and spend paths. Spend-side re-org depth is left at 0 because the
notifier does not track one internally.
Notes
SpendEvent.Reorgis a bare signal with no depth, so surfacing it wouldrequire a deeper notifier change.