-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hsmd: Add hsmd_forget_channel to tell hsmd to delete a channel #101
base: 2023-11-remote-hsmd-v23.11
Are you sure you want to change the base?
Conversation
Changelog-Added: Explicitly tell hsmd when it is time to forget a channel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I had just a comment on the code when the channel is deleted, otherwise for me you can upstream it, or if you do not have time I can do it
|
||
/* Tell the hsm to forget the channel, needs to be after it's | ||
* been forgotten here */ | ||
if (hsm_capable(channel->peer->ld, WIRE_HSMD_FORGET_CHANNEL)) { | ||
msg = towire_hsmd_forget_channel(NULL, &channel->peer->id, channel->dbid); | ||
msg = hsm_sync_req(tmpctx, channel->peer->ld, take(msg)); | ||
if (!fromwire_hsmd_forget_channel_reply(msg)) | ||
fatal("HSM gave bad hsm_forget_channel_reply %s", tal_hex(msg, msg)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know when but if the hsmd is telling us that we can not close the channel? it is better to leave the database untouch?
In other words do you think that it is better move this code on top of the https://github.com/lightning-signer/c-lightning/pull/101/files#diff-109e3febdfc51acb2389960ebf7af4fab4deaff873eb35e09a57b3efea5225f3R96 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts:
- The
forget_channel
calls happen 100 blocks after all future activity is not possible because the channel has been closed, swept, etc. It's not about "closing" the channel but rather promising that it will never be referred to again so it's ok to recover it's resources (for VLS recovering the memory is critical on small embedded devices (like the SphinxSigner, aka Stakwork)) - Both CLN and VLS appear to have consistent strategy, forgetting the channel after the last bits are onchain and buried by 100 blocks.
- The problem is when there is confusion with either CLN or VLS out-of-date etc
- VLS's handler for
forget_channel
does almost nothing, it merely sets a flag on the channel that tells us that we saw the node announce that it had forgotten it
So there are two "bad" cases, both caused by only part of the routine above completing:
- VLS forgets but CLN does not
- CLN forgets but VLS does not
In case #1 we have a terrible outcome, CLN asks VLS to sign something related to the channel (the defensive rebroadcast of the "last tx, for example") but the channel is unknown to VLS. This what we saw on home4
. VLS panics, no progress is made because CLN panics as well and restarts and then reissues the same operation again and again ...
Case #2 is not so bad, CLN goes on having forgotten about the channel and VLS will warn "hey, I haven't see the expected forget_channel
from CLN for this channel and keeps holding off. After 2016 blocks (roughly 2 weeks) VLS will give up and delete the channel. So some memory is not available for other channels in the meantime. But it heals in 2 weeks.
So we vastly prefer the 2nd alternative which corresponds to CLN telling it forgot after it forgets for sure.
The operations takeaway is that if you see this warning persisting it is good to check on the channel status on the node side. If something on the CLN side is a little broken you can use the --developer
lightning-cli dev-forget-channel id=0248691d3963b05a...
to clean it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense, thanks to write it down
Signed-off-by: Lakshya Singh <[email protected]>
fix: rpc server port addition
Changelog-Added: Explicitly tell hsmd when it is time to forget a channel
Addresses https://gitlab.com/lightning-signer/validating-lightning-signer/-/issues/435
This PR is matched by https://gitlab.com/lightning-signer/validating-lightning-signer/-/merge_requests/569 on the VLS side.
Both parts of the
forget_channel
interface (CLN and VLS) are forward and backward compatible:So we do not need coordination on submitting / merging / releasing, this PR and the VLS MR can go in any either order