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

[tap channels preparation 1/?]: Add QueryInternalKey and QueryScriptKey RPCs, pre-fill script key info in vPSBT #765

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

guggero
Copy link
Member

@guggero guggero commented Jan 11, 2024

A couple of preparatory commits extracted from the TAP channel simulation branch.

@guggero guggero requested review from Roasbeef and ffranr January 11, 2024 16:42
@guggero guggero force-pushed the tap-channels-prep-1 branch 2 times, most recently from 9e39e8e to 3f4f12e Compare January 11, 2024 16:59
Copy link
Contributor

@ffranr ffranr left a comment

Choose a reason for hiding this comment

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

That was easy to review, thanks!

I think I'll be able to approve quickly on Monday.

tapdb/addrs.go Outdated Show resolved Hide resolved
tapdb/addrs.go Outdated Show resolved Hide resolved
tapdb/addrs_test.go Outdated Show resolved Hide resolved
perms/perms.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
tapfreighter/wallet.go Outdated Show resolved Hide resolved
@guggero guggero force-pushed the tap-channels-prep-1 branch from 3f4f12e to 80c62f9 Compare January 15, 2024 12:32
@guggero
Copy link
Member Author

guggero commented Jan 15, 2024

Thank you for the thorough review, @ffranr. I addressed all your comments.

Copy link
Contributor

@ffranr ffranr left a comment

Choose a reason for hiding this comment

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

👍

Added a comment on generalisation, otherwise I think this looks good.

tapscript/util.go Outdated Show resolved Hide resolved
rpcserver.go Show resolved Hide resolved
asset/asset.go Outdated Show resolved Hide resolved
fn/func.go Outdated Show resolved Hide resolved
@dstadulis dstadulis added this to the v0.4 milestone Jan 24, 2024
@dstadulis
Copy link
Collaborator

@guggero should @ffranr review the latest commit or are more commits needed?

@guggero
Copy link
Member Author

guggero commented Jan 29, 2024

Doesn't need a re-review, nothing major changed other than addressing @ffranr's comments.

We'll need to be able to fetch the key family and index for an internal
key of an address or asset by just specifying the internal public key.
We add this method to the TapAddressBook as that already has a similar
method for the script key.
When signing a virtual PSBT, we don't want to use the key descriptor
info of the input asset's script key, as that information is not
available when the input asset is created from a de-serialized asset.
So previous to the change in this commit the SignVirtualPsbt RPC method
could only be called on vPSBTs that were funded through FundVirtualPsbt
that populated the key information.
After this commit it is possible to sign a vPSBT that was created from
an input proof only since the required key information is loaded from
the database before signing.
The asset version v1 (segwit) allows an asset to be committed without
the TxWitness field. Therefore, when comparing two such assets, we must
also ignore the TxWitness field.

If we don't do this, then the deep equal check when creating a proof for
a split asset fails when checking if the split root asset was committed
to the root tree.
@guggero guggero force-pushed the tap-channels-prep-1 branch from f9a7286 to 3e7d61d Compare January 29, 2024 13:38
@guggero guggero requested a review from Roasbeef January 29, 2024 13:39
@guggero
Copy link
Member Author

guggero commented Jan 29, 2024

Comments addressed, should be good for final round of review.

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 🐍

@Roasbeef Roasbeef merged commit eb3ed93 into main Jan 30, 2024
14 checks passed
@guggero guggero deleted the tap-channels-prep-1 branch January 30, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants