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.

Sign up for GitHub

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

Jump to bottom

psbt: fix channel funding address for simple taproot channels #7932

Merged
merged 2 commits into from
Aug 29, 2023
Merged

psbt: fix channel funding address for simple taproot channels #7932

merged 2 commits into from
Aug 29, 2023

Conversation

guggero
Copy link
Collaborator

@guggero guggero commented Aug 28, 2023

Fixes #7929.

Also adds PSBT funding integration tests for simple taproot channels.

Adding the no-changelog label as this is part of the RC bug fixing effort.

@guggero guggero added funding Related to the opening of new channels with funding transactions on the blockchain psbt taproot taproot chans no-changelog labels Aug 28, 2023
@guggero guggero added this to the v0.17.0 milestone Aug 28, 2023
yyforyongyu
yyforyongyu approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

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

LGTM👍

lnwallet/chanfunding/psbt_assembler.go Show resolved Hide resolved
itest/lnd_psbt_test.go Show resolved Hide resolved
itest/list_on_test.go Show resolved Hide resolved
@guggero guggero requested a review from yyforyongyu August 28, 2023 17:10
@guggero guggero force-pushed the psbt-funding-taproot-chans branch from a41a3b3 to bd11952 Compare August 28, 2023 17:10
@guggero guggero requested a review from Roasbeef August 28, 2023 17:10
Roasbeef
Roasbeef approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🧩

One small suggestion re using a slightly more general utility func for addr type detection.

lnwallet/chanfunding/psbt_assembler.go Outdated Show resolved Hide resolved
lntest/harness.go Show resolved Hide resolved
yyforyongyu
yyforyongyu approved these changes Aug 29, 2023
View reviewed changes
Copy link
Member

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

ACK bd119524539d415489724d78486cd38f8ddf2676

guggero added 2 commits August 29, 2023 10:57
@guggero
chanfunding: fix PSBT funding for taproot chans
646444c
@guggero
itest: unify PSBT channel funding tests, add taproot
590dc0c 
With this commit we unify three existing PSBT channel funding
integration tests into a single one with the goal of testing all three
cases with simple taproot channels as well.
@guggero guggero force-pushed the psbt-funding-taproot-chans branch from bd11952 to 590dc0c Compare August 29, 2023 08:57
@guggero guggero merged commit 73bf21e into lightningnetwork:master Aug 29, 2023
@guggero guggero deleted the psbt-funding-taproot-chans branch August 29, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yyforyongyu yyforyongyu yyforyongyu approved these changes

@Roasbeef Roasbeef Roasbeef approved these changes

Assignees
No one assigned
Labels
funding Related to the opening of new channels with funding transactions on the blockchain no-changelog psbt taproot chans taproot
Projects
None yet
Milestone
v0.17.0
Development

Successfully merging this pull request may close these issues.

[bug]: PSBT funding for SIMPLE_TAPROOT channels returns inconsistent funding address
3 participants
@guggero @Roasbeef @yyforyongyu