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

multi: send P2TR addrs by default for co-op close, add new option_any_segwit feature bit #6633

Merged
merged 8 commits into from
Aug 12, 2022

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Jun 10, 2022

In this commit, we implement the new option_any_segwit feature bit which enables us to start sending P2TR addresses for co-op closes as well as for upfront shutdown addresses. Along the way, we tighten up our validation a bit to only allow segwit v0 and v1 addresses.

@Roasbeef Roasbeef added channel closing Related to the closing of channels cooperatively and uncooperatively needs review PR needs review by regular contributors labels Jun 10, 2022
@Roasbeef Roasbeef added this to the v0.15.1 milestone Jun 10, 2022
lnwallet/wallet.go Outdated Show resolved Hide resolved
lnwallet/wallet.go Show resolved Hide resolved
lnwallet/chancloser/chancloser.go Show resolved Hide resolved
funding/manager.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

looks good! left one question about possible interop bug in future world where witness versions 2-16 are a thing. My interpretation of the spec is that if we advertise this bit, the peer will think it is fine for them to send a shutdown script of these witness version. With this pr, we will error out if they do so

lnwallet/wallet.go Show resolved Hide resolved
lnwallet/chancloser/chancloser.go Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Member Author

The refactoring I did in #6770 should make it easier to write unit tests for this change as well.

@lightninglabs-deploy
Copy link

@Roasbeef, remember to re-request review from reviewers when ready

@Roasbeef
Copy link
Member Author

Roasbeef commented Aug 5, 2022

Ok, I think this is g2g now: just pushed up fixes, a new unit test, and also an itest to make sure negotiation works e2e.

Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

go.mod Outdated Show resolved Hide resolved
feature/manager.go Show resolved Hide resolved
Copy link
Collaborator

@Crypt-iQ Crypt-iQ left a comment

Choose a reason for hiding this comment

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

lgtm but one comment

lnwallet/wallet.go Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Member Author

Roasbeef commented Aug 9, 2022

Proper version should be pushed up now (w/o the commit that bumps the Go version).

// * sha256(initiatorKey || responderKey)[26:]
// * where both keys are the multi-sig keys of the respective parties
// - sha256(initiatorKey || responderKey)[26:]
// -- where both keys are the multi-sig keys of the respective parties
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a quirk of updating to Go 1.19 btw...

In this commit, we add awareness of the option_shutdown_anysegwit that
permits both sides to send newer segwit based addresses. This'll
eventually enable us to send taproot addresses for co-op close.
In this commit, we catch up our logic with the latest version of the
spec that removed support for normal p2kh and p2sh addresses for co-op
closes, in order to make dust calculations more uniform.
We only want to allow p2wkh, p2tr, and p2wsh addresses, so we'll utilize
the newly public wallet function to restrict this.
If the ShutdownAnySegwitOptional option is active, then we can safely
send these newer addresses.
@Roasbeef Roasbeef merged commit a1cd773 into lightningnetwork:master Aug 12, 2022
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 needs review PR needs review by regular contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants