Skip to content
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

lnwallet+peer: properly process update_fee messages sent after a shutdown has been initiated #6522

Closed
2 tasks
Roasbeef opened this issue May 11, 2022 · 2 comments · Fixed by #8167
Closed
2 tasks
Assignees
Labels
channel closing Related to the closing of channels cooperatively and uncooperatively P1 MUST be fixed or reviewed spec
Milestone

Comments

@Roasbeef
Copy link
Member

Today when we receive a shutdown requests, we'll remove the link from the switch entirely also (

lnd/peer/brontide.go

Lines 2695 to 2701 in ddeccf8

// Optimistically try a link shutdown, erroring out if it
// failed.
if err := p.tryLinkShutdown(chanID); err != nil {
peerLog.Errorf("failed link shutdown: %v", err)
req.Err <- err
return
}
). This means that any inbound messages to the link will be dropped from the peer's readHandler. This was fine when we wanted to not process any further updates, but breaks down given that the spec requires (being updated to make this more clear) that one still process updateFee messages after a shutdown has been sent (but before the first closing signed).

What we should still remove the link from the switch and send out a disable, but we need to special case for the sig+revoke and UpdateFee messages when we're in this state. One way to do this (given we already have the channel struct at that point) would be to manually feed in these messages into the channel state machine.

Steps To Completion

  • Update the readHandler loop to send any update messages post channel close into the chan closer
  • Have the chan close then manually apply only the UpdateFee message, generating the corresponding messages needed to fully lock in the change.
@Roasbeef Roasbeef added channel closing Related to the closing of channels cooperatively and uncooperatively spec labels May 11, 2022
@Crypt-iQ
Copy link
Collaborator

I have a partial branch for this that puts the link in shutdown mode where it won't route any htlc's. It still needs to be able to fail existing htlc's and send fee messages, but it always makes progress towards the eventual closing_signed state

@Roasbeef Roasbeef added this to the v0.16.0 milestone May 24, 2022
@Roasbeef Roasbeef moved this to 🔖 Ready in lnd v0.16.0 Aug 23, 2022
@Roasbeef Roasbeef linked a pull request Aug 24, 2022 that will close this issue
@saubyk saubyk moved this from 🔖 Ready to 👀 In review in lnd v0.16.0 Oct 25, 2022
@saubyk saubyk modified the milestones: v0.16.0, v0.16.1 Feb 16, 2023
@saubyk saubyk removed this from lnd v0.16.0 Feb 16, 2023
@saubyk
Copy link
Collaborator

saubyk commented Mar 13, 2023

prioed for 17

@saubyk saubyk modified the milestones: v0.16.1, v0.17.0 Mar 13, 2023
@saubyk saubyk added this to lnd v0.17 Mar 26, 2023
@saubyk saubyk moved this to 🔖 Ready in lnd v0.17 Mar 26, 2023
@saubyk saubyk removed this from lnd v0.17 Jun 15, 2023
@saubyk saubyk modified the milestones: v0.17.0, v0.17.1 Jun 15, 2023
@saubyk saubyk added the P1 MUST be fixed or reviewed label Aug 8, 2023
@saubyk saubyk removed this from the High Priority milestone Aug 8, 2023
@saubyk saubyk linked a pull request Nov 17, 2023 that will close this issue
7 tasks
@saubyk saubyk assigned ProofOfKeags and unassigned Crypt-iQ Nov 17, 2023
@saubyk saubyk added this to the v0.18.0 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
channel closing Related to the closing of channels cooperatively and uncooperatively P1 MUST be fixed or reviewed spec
Projects
None yet
4 participants