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

incorrect use of p2wkh weight #7893

Closed
Crypt-iQ opened this issue Aug 17, 2023 · 2 comments · Fixed by #8279
Closed

incorrect use of p2wkh weight #7893

Crypt-iQ opened this issue Aug 17, 2023 · 2 comments · Fixed by #8279
Assignees
Labels
bug Unintended code behaviour fees Related to the fees paid for transactions (both LN and funding/commitment transactions) P1 MUST be fixed or reviewed utxo sweeping
Milestone

Comments

@Crypt-iQ
Copy link
Collaborator

Should be p2tr weight since we only make p2tr outputs. This won't lead us to create invalid sweep transactions but isn't correct:

if change {
weightEstimate.addP2WKHOutput()
}

@yyforyongyu yyforyongyu added bug Unintended code behaviour fees Related to the fees paid for transactions (both LN and funding/commitment transactions) P2 should be fixed if one has time labels Aug 21, 2023
@yyforyongyu yyforyongyu added P1 MUST be fixed or reviewed and removed P2 should be fixed if one has time labels Sep 5, 2023
@yyforyongyu
Copy link
Collaborator

When fixing this issue we should also updateDustLimitForSize to include P2TRSize here,

switch scriptSize {
case input.P2WPKHSize:
pkscript, _ = input.WitnessPubKeyHash([]byte{})

Atm it won't create any errors, when seeing an input.P2TRSize it will implicitly go to input.P2WSHSize. We should fix it tho as it looks confusing.

@Crypt-iQ
Copy link
Collaborator Author

Crypt-iQ commented Sep 5, 2023

I dont think we can have two identical cases, maybe a comment that says that p2tr and p2wsh are equal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unintended code behaviour fees Related to the fees paid for transactions (both LN and funding/commitment transactions) P1 MUST be fixed or reviewed utxo sweeping
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants