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

tapchannel: use defer to ensure lease unlocked if funding failure #1033

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Jul 19, 2024

Once we call fundVirtualPacket, we've locked inputs assuming it succeeds. If for w/e reason, the remote peer fails to send us the first ack (before the call to completeChannelFunding), then before this commit, we would fail to unlock the inputs. Only after the call to completeChannelFunding would we unlock the UTXOs.

In this commit, we use two new defers to ensure that we always unlock the UTXOs if the pre-setup fails, and also if completeChannelFunding fails.

Fixes #1019

Once we call `fundVirtualPacket`, we've locked inputs assuming it
succeeds. If for w/e reason, the remote peer fails to send us the first
ack (before the call to `completeChannelFunding`), then before this
commit, we would fail to unlock the inputs. Only after the call to
`completeChannelFunding` would we unlock the UTXOs.

In this commit, we use two new defers to ensure that we always unlock
the UTXOs if the pre-setup fails, and also if `completeChannelFunding`
fails.
@@ -1028,13 +1031,6 @@ func (f *FundingController) completeChannelFunding(ctx context.Context,
return nil, err
}

fundingState.lockedAssetInputs = fn.Map(
Copy link
Member Author

@Roasbeef Roasbeef Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this down (called earlier) so we track the locked inputs as soon as the initial vPSBT funding finishes.

Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM 🎉

Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
nice trick with the setupSuccess/completeSuccess flags

@guggero guggero merged commit 7035ded into main Jul 19, 2024
16 checks passed
@guggero guggero deleted the defer-utxo-cleanup-funding branch July 19, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[bug]: In Ver lightning-terminal:v0.13.99-alpha.rc4
3 participants