tappsbt: add AltLeaf support to vPacket #1180
Conversation
|
Remaining TODOs from existing feedback:
Worth noting that with this more minimal encoder, the minimal size for a leaf is 42 bytes vs. 128 for a normal |
Pull Request Test Coverage Report for Build 11804187328Details
💛 - Coveralls |
jharveyb
force-pushed
the
psbt_aux_leaves
branch
2 times, most recently
from
191d54c to
29ff5f3
Compare
|
Rebased, addressed all TODO items, and added some further changes to the definition of an Specifically, the Prop tests were updated to reflect this, and there are failing cases for each condition of |
jharveyb
force-pushed
the
psbt_aux_leaves
branch
from
29ff5f3 to
bf178a2
Compare
jharveyb
force-pushed
the
psbt_aux_leaves
branch
from
bf178a2 to
3e06a8e
Compare
In this commit, we add the AltLeaf interface, which is used to carry non-Asset data inside a Tap commitment. The Asset type meets this interface, and we add a separate encoder that removes static fields for an Asset being used as an AltLeaf.
In this commit, we add property tests for the AltLeaf validation logic and the AltLeaf encoder and decoder. We also add Generator functions for many types, including the Asset type.
In this commit, we add a new test to ensure that if a Packet is missing the Unknown values we need to identify VPackets, the decoder fails. We also check that decoding does not fail if Unknown values unrelated to VPackets are included.
jharveyb
force-pushed
the
psbt_aux_leaves
branch
from
3e06a8e to
fda8a6d
Compare
|
Addressed nits, updated encoding to drop the |
There was a problem hiding this comment.
LGTM 🦚
Nice work! Rapid testdata dir adds a bit of bloat, but I think in the short term, it's best to continue committing the test cases to the repo. In the future, we may end up reaching for an external corpus like we do with fuzzing stuff, or perhaps there's a way to minimize the test cases it adds after the fact.
|
|
||
| // Always use the normal witness encoding, since the asset version is | ||
| // always V0. | ||
| if len(a.PrevWitnesses) > 0 { |
There was a problem hiding this comment.
Will the alt leaf version ever have a use for the prev witness value?
There was a problem hiding this comment.
Hmm I don't expect it to, but that's also the only field left here that's 'consensus' wrt. being handled by parts of tapd, compared to the unknown odd field. Which may be useful for future features.
Given the size limits for []AltLeaves I'm not too worried about it causing dysfunction.
| var ( | ||
| // Simple generators. | ||
|
|
||
| VersionGen = rapid.SampledFrom([]Version{V0, V1}) |
There was a problem hiding this comment.
Nice! Was writing some tests with rapid in another PR and nearly reached to write custom gens for everything, but decided not to, but glad you did
Yeah, the framework has some settings around minimization but I think that does not account for the size of the input, but maybe how quickly the code under test fails? The default shrinking time is 30 seconds, but reaching failing cases was much faster than that. I'm not sure exactly how we can shrink the fail cases, especially which structs that have value fields and not pointers. |
Introduce the AltLeaves field for vPackets, which can be used to carry non-asset data alongside a Taproot Assets transfer. This data will be anchored alongside any assets involved in a transfer, allowing for other protocols to operate in the same anchor TXs as Taproot Assets.