multi: scid+zero conf follow up master tracking issue
This is a master tracking issue to track the follow ups for https://github.com/lightningnetwork/lnd/pull/5955. These were mentioned in the PR but are now buried by comments, so I'm lifting them up here for better visibility:
- [x] If zero conf is active, reject all incoming requests if a channel acceptor isn't registered.
- [x] Expose alias information over RPC. Two rough options here:
- Create a new RPC where all the central alias information can be accessed.
- Extend all the relevant RPCs to add new alias fields as necessary.
^ @Crypt-iQ am I missing anything here?
Extend all the relevant RPCs to add new alias fields as necessary.
Some of these RPCs would need quite a bit of plumbing to surface the alias or real SCID and I'm not sure if those are worth it. The caller could instead cross-reference with the main alias API. And in some cases, changing them wouldn't be "correct" as in the case of the APIs that return CircuitKeys. A CircuitKey means a specific thing at the database-level and changing the SCID at the RPC level seems to confuse the meaning and if some LookupCircuit API were exposed, the RPC return values wouldn't work.
That said, some should have the correct info populated whether that's an alias or a real SCID.
The funding manager calls NotifyPendingOpenChannelEvent and NotifyOpenChannelEvent when the channel is pending and when the channel is open, respectively. For zero-conf this happens in quick succession. MarkAsOpen is called before NotifyOpenChannelEvent, so this notification is only called once. If the two notifications are dispatched, followed by MarkAsOpen, followed by the node crashing, it's possible that the notifications weren't received / persisted by the swapper, meaning that there wouldn't be a backup entry for this channel. While technically possible for non-zero-conf channels, it's less likely.
@Crypt-iQ so re the RPCs, are you suggesting that we make a new RPC, or that the information exposed today is mostly "enough"?
it's possible that the notifications weren't received / persisted by the swapper, meaning that there wouldn't be a backup entry for this channel.
On startup, we'll always update the on disk channels w/ the latest local state: https://github.com/lightningnetwork/lnd/blob/10fba3d8590c72e7e17f1d75a1a4176e062ffd46/chanbackup/pubsub.go#L102-L125. Or does your scenario just affect that state after the crash and before the restart?
so re the RPCs, are you suggesting that we make a new RPC, or that the information exposed today is mostly "enough"?
New RPC and some existing RPCs amended.
Or does your scenario just affect that state after the crash and before the restart?
Didn't see that, then it would only affect after crash before restart
The other thing would be to make the graph replacement atomic when the channel reaches 6 confirmations. Right now a crash will lose the policy and have to replace it with the default policy
Both PRs described above have been merged in.