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

Simples changes that are required to support taproot LN channels #129

Merged
merged 7 commits into from
Aug 20, 2024

Conversation

sstone
Copy link
Member

@sstone sstone commented Aug 7, 2024

We need to be able to serialize taproot script trees, compare x-only pubkeys, and create taproot sessions to verify partial signatures.
This changes are imported from #128 which includes unrelated more complex changes to our PSBT implementation,

@sstone sstone added this to the 0.20.0 milestone Aug 12, 2024
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

I don't understand why the ScriptTree encoding is necessary, you don't seem to use it for the PSBT fields (which only require encoding/decoding script leaves)? Also, the leaf encoding defined in the PSBT bip doesn't match the encoding you're using?

@sstone
Copy link
Member Author

sstone commented Aug 12, 2024

I don't understand why the ScriptTree encoding is necessary, you don't seem to use it for the PSBT fields (which only require encoding/decoding script leaves)? Also, the leaf encoding defined in the PSBT bip doesn't match the encoding you're using?

Because we persist input information along with our peer's signature (or partial signature) in both eclair and lightning-kmp and for taproot outputs that includes a scrip tree. It's not related to PSBT but I'll see if I can reuse the format that has been proposed.

@t-bast
Copy link
Member

t-bast commented Aug 12, 2024

Got it, that's what confused me, this is unrelated to PSBT. It seems to me that we only need to serialize the leaves in order to re-build the tree, don't we? We would lose the "leafIndex" but maybe we can re-work that to get rid of it?

@sstone
Copy link
Member Author

sstone commented Aug 14, 2024

I've added the serialisation format for script trees defined in BIP371 (and kept ours which I find simpler :)).

@sstone sstone marked this pull request as ready for review August 20, 2024 08:12
sstone added 2 commits August 20, 2024 11:56
The id field was used in tests only, was not part of the tree hash and was not serialised.
We also remove our custom serialisation format and keep only the format defined in BIP371.
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@sstone sstone merged commit de10997 into master Aug 20, 2024
4 checks passed
@sstone sstone deleted the snapshot/script-tree-serde branch August 20, 2024 12:45
@sstone sstone mentioned this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants