Proof-system-v2

[gijswijs]

This pull request introduces several enhancements and refactors related to inclusion and exclusion proof handling. The key changes include:

• STXO Commitment and Proofs:
  • Implemented commitment to spent assets within Taproot Assets using AltLeaves.
  • Added encoders and decoders for CommitmentProof structures that are used for the STXO proofs.
  • Introduced STXO inclusion proofs and proof verification for non-split assets.
  • Added STXO exclusion proofs and verification for the outputs that don't involve the asset spending the STXOs
• Code Refactoring and Improvements:
  • Minor refactors of code throughout
  • Improved wording in comments for clarity.
  • Removed unnecessary parameters and assertions
  • Refactor the code of proof/verifier.go
• Testing:
  • Added integration tests for STXO inclusion proofs.
  • Added integration tests for STXO exclusion proofs.
  • Implemented unit tests for CommitmentProofs encoding and decoding.

Considerations

• Potentially remove the commit multi: remove AssertInputsUnique. Although with this proof system we are able to parse a proof for non-unique inputs, older nodes cannot. So for now this check should probably remain in place.
• Explore adding proofs to asset splits.

[coveralls]

Pull Request Test Coverage Report for Build 15070461131

Details

• 453 of 633 (71.56%) changed or added relevant lines in 18 files are covered.
• 32 unchanged lines in 10 files lost coverage.
• Overall coverage increased (+0.3%) to 37.084%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
proof/records.go	14	15	93.33%
tappsbt/interface.go	0	1	0.0%
proof/mock.go	23	26	88.46%
tapchannel/aux_closer.go	0	3	0.0%
tapfreighter/wallet.go	0	4	0.0%
itest/assertions.go	0	5	0.0%
tapchannel/commitment.go	0	6	0.0%
proof/append.go	39	46	84.78%
tapchannel/aux_sweeper.go	0	9	0.0%
proof/mint.go	9	23	39.13%
<!--	Total:	453	633

Files with Coverage Reduction	New Missed Lines	%
commitment/tap.go	1	72.05%
address/address.go	2	67.47%
address/mock.go	2	88.24%
asset/group_key.go	2	57.89%
proof/verifier.go	2	81.42%
tappsbt/create.go	2	26.74%
asset/asset.go	4	47.35%
tapchannel/aux_leaf_signer.go	5	43.08%
tappsbt/decode.go	6	84.51%
tappsbt/encode.go	6	92.27%
<!--	Total:	32

Totals
Change from base Build 15055985108:	0.3%
Covered Lines:	26885
Relevant Lines:	72497

---

💛 - Coveralls

[guggero]

Extracted some commits into #1465 to ease review on this one. Pushed up a rebase.

[guggero]

Sorry, I don't think I made things easier for you as an author by splitting the PR into two. And now I noticed that the commit https://github.com/lightninglabs/taproot-assets/commit/c9b1d54430cdd7c4cdf3e1e65e7a33babc428afd does need to go into this one instead... Let me know if I should fix my own mess by rebasing this one.

[Roasbeef]

I think we can remove the TODO, since only the root assets need to have an inclusion proof, since the split outputs themselves should be covered by the split commitment and exclusion proof. But wanted to confirm with both of you first...

Correct. We if an asset is a split (has split commitment proof), then we don't want to add the input to the STXO at all. Otherwise, we'll get a different tree depending on the order that we inserted them (we derive the alt leaf using the script key of an asset).

Related to that that if a prev ID is all zeroes, then we also don't want to add them. Otherwise many issued assets in an output would introduce a similar issue as above.

[Roasbeef]

Is this PR ready to leave draft? We should start to run CI against it.

[gijswijs]

Related to that that if a prev ID is all zeroes, then we also don't want to add them. Otherwise many issued assets in an output would introduce a similar issue as above.

I think that is covered by:

https://github.com/lightninglabs/taproot-assets/blob/79e199feca0512b9cd26977a0ab4ffd673ca9385/asset/asset.go#L1472-L1477

We use this func to determine if we need to collect and commit to STXOs here:

https://github.com/lightninglabs/taproot-assets/blob/79e199feca0512b9cd26977a0ab4ffd673ca9385/asset/asset.go#L2175-L2180

[Roasbeef]

So I'll need to take a look into that. But hopefully it's something small (will take a closer look tomorrow).

I think what's missing is that we need to mind the alt leaves when we manually create the funding output our selves.

[guggero]

I think what's missing is that we need to mind the alt leaves when we manually create the funding output our selves.

Yeah, I got everything to work today. Now I'm just making sure this doesn't introduce another backward incompatibility vector.

[guggero]

I've pushed up a set of changes that make everything backward compatible. What we decided to do in this PR is the following:

• We do NOT produce any V1 proofs in this PR (outside of unit tests). Otherwise newly minted or transferred assets would not be recognized by older nodes ("unknown proof version 1").
• We DO however add STXO commitments to root outputs and add STXO inclusion/exclusion proofs to any generated proofs.
  • STXO commitments are only added to root outputs so on-chain sends to older nodes still work as expected. So we can test that we can correctly spend any inputs that have STXO commitments in them.
  • Without a proof being set as V1, any STXO inclusion/exclusion proofs will not be verified. But we can make sure that old nodes can correctly parse such proofs but just put the data in the unknownOddTypes as they don't know them yet.
• For channels, we disable both of the above. Meaning we don't produce any STXO commitments and we don't use V1 proofs either. Doing so unilaterally would result in a mismatch in signatures and force closures. Which means we'll need a specific upgrade plan for channels in the future.

[Roasbeef]

We do NOT produce any V1 proofs in this PR (outside of unit tests). Otherwise newly minted or transferred assets would not be recognized by older nodes ("unknown proof version 1")

What if we still set all the fields, but don't yet bump the version? The new TLVs would all be odd. We then flip the new version at a point in the future once the new fields are showing up more often in public proofs.

We'd then still verify as normal.

Which means we'll need a specific upgrade plan for channels in the future.

Makes sense, we can add another version to the initial set of pre funding messages that we send to the other side.

STXO commitments are only added to root outputs so on-chain sends to older nodes still work as expected. So we can test that we can correctly spend any inputs that have STXO commitments in them.

Is this actually a change in behavior from the earlier versions? IIUC, we never set it for splits anyway, as they spend a prevID of zero.

[gijswijs]

What if we still set all the fields, but don't yet bump the version? The new TLVs would all be odd. We then flip the new version at a point in the future once the new fields are showing up more often in public proofs.

That is what we suggest with this approach. Also, since the code to verify the new proofs is in this release. If we bump the version in the future, older nodes (from v0.6.0 onwards) can verify v1 proofs.

Is this actually a change in behavior from the earlier versions? IIUC, we never set it for splits anyway, as they spend a prevID of zero.

This is not a change in behavior. This is how we did it anyway.