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

sweep: use p2tr output as change weight #8279

Merged
merged 1 commit into from Jan 18, 2024

Conversation

Crypt-iQ
Copy link
Collaborator

We now use p2tr outputs as change

Fixes #7893

@Crypt-iQ Crypt-iQ added fees Related to the fees paid for transactions (both LN and funding/commitment transactions) bug fix labels Dec 14, 2023
@Crypt-iQ Crypt-iQ added this to the v0.18.0 milestone Dec 14, 2023
Copy link
Collaborator

@ziggie1984 ziggie1984 left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM

Some minor nits:

  1. Add a comment to the DustLimitForSize for P2TR which you propose in incorrect use of p2wkh weight #7893
  2. Update the ChangeScript comment here for P2TR:

    lnd/server.go

    Lines 4662 to 4663 in ac9ca02

    // Specifically, the script generated is a version 0, pay-to-witness-pubkey-hash
    // (p2wkh) output.

    and here:

    lnd/sweep/sweeper.go

    Lines 248 to 249 in ac9ca02

    // GenSweepScript generates a P2WKH script belonging to the wallet where
    // funds can be swept.
  3. Missing Release-Docs.

Copy link
Collaborator

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

Pending a release note📝

@lightninglabs-deploy
Copy link

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

Copy link
Collaborator

@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.

I guess we can skip the release notes for this one.

@guggero guggero merged commit dda5c45 into lightningnetwork:master Jan 18, 2024
23 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix fees Related to the fees paid for transactions (both LN and funding/commitment transactions) no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incorrect use of p2wkh weight
5 participants