Add AltLeaf support to tapfreighter #1233
Open
+2,996
−1,134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
From some offline review, there are a few clear TODOs:
|
|
The test vectors for |
jharveyb
force-pushed
the
proof_alt_leaves
branch
from
6fa4a2a to
f49820f
Compare
Pull Request Test Coverage Report for Build 12381816079Details
💛 - Coveralls |
1 task
jharveyb
force-pushed
the
proof_alt_leaves
branch
from
f49820f to
1fa5353
Compare
guggero
reviewed
Dec 6, 2024
Roasbeef
requested changes
Dec 6, 2024
jharveyb
force-pushed
the
proof_alt_leaves
branch
from
1fa5353 to
9f4e6a5
Compare
|
Added a new itest to check that the tapfreighter and the vPSBT RPCs are rejecting invalid combinations of |
guggero
force-pushed
the
proof_alt_leaves
branch
2 times, most recently
from
a7e34d5 to
f4b1bc0
Compare
|
@jharveyb, remember to re-request review from reviewers when ready |
gijswijs
reviewed
Dec 16, 2024
guggero
force-pushed
the
proof_alt_leaves
branch
from
f4b1bc0 to
538cf21
Compare
By changing the generic signature of the AltLeaf type we can get rid of some of the weirdness and helper methods surrounding the type.
In this commit, we add an AltLeaves field to Proof, and update Proof en/decode to support this optional field. With a small test change, we also check that Proof.Verify() functions for Proofs that accounted for any present AltLeaves correctly.
In this commit, we add logic to handle adding and removing altLeaves from a TapCommitment. Unlike normal Assets, we never want to update an AltLeaf once inserted into the AltCommitment. This means we need to assert that AltLeaves being added to a TapCommitment don't collide with already committed AltLeaves.
In this commit, we update the passive asset handling to not consider altLeaves as assets. Specifically, any altLeaf in an input Tap commitment should be set in the vInput for a passive asset vPkt, but should not have its own vPkt where it is an input. Overall, input altLeaves do not need to be re-anchored.
guggero
force-pushed
the
proof_alt_leaves
branch
from
538cf21 to
17f4e95
Compare
We don't trim the alt leaves from passive assets. The two places where we use the |
guggero
approved these changes
Dec 17, 2024
|
LGTM 🎉 |
gijswijs
approved these changes
Dec 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This continues the work from #1180 .
Specifically:
AltLeavesfield toProof.ProofwithAltLeavespopulated.AltLeavesfromvOutputs, to anchor outputs for a transfer, and the corresponding transition proof.AltLeavesfor asset inputs during coin selection, and uses theseAltLeaveswhen verifying the input anchor pubkey.Test-wise, the added unit tests cover the new validation methods around
[]AltLeaves, and the new funcs that act on Tap commitments. The changes in later commits intapsendandtapfrieghterare mostly only exercised via the PSBT itest.