Proof-system-v1 #1453

gijswijs · 2025-03-22T15:07:24Z

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.

This change is

coveralls · 2025-03-22T15:27:04Z

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%

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%

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

💛 - Coveralls

asset/asset.go

tapsend/send_test.go

asset/asset.go

proof/encoding.go

proof/verifier.go

itest/psbt_test.go

proof/taproot.go

guggero

Nice work, this looks pretty good 🎉
Have a couple of suggestions, after which we can probably take this out of draft.

guggero · 2025-04-04T10:47:48Z

tapsend/send.go

@@ -2026,15 +2038,6 @@ func ValidateAnchorInputs(anchorPacket *psbt.Packet, packets []*tappsbt.VPacket,
 		)
 	}

-	// Each input must have a valid set of AltLeaves at this point.


Since there might've already been alt leaves in the vPacket, should we still validate them? Or is this added back in another place in another commit? Still going through the diff.

I'm not sure if I follow. This is a sanity check that makes sure we have at least a valid set of AltLeaves. If we don't have them here during ValidateAnchorInputs we can fail early.
Yes, we do check them during FundPacket as well, so it should not be possible to have invalid AltLEaves at this point in the code, but it doesn't hurt to check here. Also, future flows might be different and warrant a check here even more.

proof/encoding.go

proof/append.go

proof/verifier.go

proof/append.go

proof/append_test.go

tapsend/proof_test.go

guggero · 2025-04-11T12:42:55Z

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

guggero

Did another pass. Definitely looks better, but I think we still need an iteration on the core verification logic, to make it easier to understand.

proof/append.go

tapsend/proof.go

proof/verifier.go

tapsend/proof_test.go

proof/verifier.go

guggero · 2025-04-11T15:20:51Z

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 c9b1d54 does need to go into this one instead...
Let me know if I should fix my own mess by rebasing this one.

Roasbeef

Did a clean review, focusing on the tests in particular. Feels like we're pretty close.

The linter is failing, and also the unit tests panic as one of the test cases is incomplete:

--- FAIL: TestVerifyV1ExclusionProof (0.12s)
    --- FAIL: TestVerifyV1ExclusionProof/multiple_assets_with_an_exclusion_proof_each (0.00s)

proof/append.go

proof/append_test.go

proof/verifier.go

proof/verifier_test.go

tapsend/send_test.go

Roasbeef

Reviewed 25 of 25 files at r1, 28 of 28 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status: all files reviewed, 61 unresolved discussions (waiting on @gijswijs and @guggero)

guggero

I pushed a single fixup commit that adds more unit tests (and also uncovered a small bug that is fixed too). Feel free to check and apply that commit directly (not attribution required).

Other than that only nits are left in the code (which I didn't fix, I leave that to you).

The litd itest failure is related to the changes here though:

error funding channel: unable to bind PSBT: rpc error: code = Unknown desc = error verifying PSBT: funding output not found in PSBT

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

asset/asset.go

proof/append.go

proof/taproot.go

proof/verifier.go

Roasbeef · 2025-05-15T18:09:27Z

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 · 2025-05-15T19:32:21Z

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.

Because we retrieve AltLeaves from the proof during anchor input validation, we don't need to store them in the vInputs struct. This commit removes the AltLeaves field from the vInputs struct and the associated methods.

Since we're only ever passing nil (and we use the previous witness in the script/burn key), we just remove the witness parameter completely.

This commit stores the STXOs in the output commitments using AltLeaves.

This commit adds a new field to the CommitmentProof struct that will house STXO proofs. We also add encoding/decoding functions for that new sub type and update the test vectors accordingly.

Co-authored-by: Gijs van Dam <gijs@lightning.engineering>

Instead of using alt leaves from the inputs, we use the alt leaves from the proof. We also remove the deduplication func `AddLeafKeysVerifyMerge` because it's not needed anymore.

This commit adds the STXO exclusion proof to the proof parameters. It stores them in `params.ExclusionProofs[].CommitmentProof.STXOProofs[]` where they are later encoded when creating the actual proofs.

The proof version is bumped to indicate the usage and support of stxo style proofs.

`verifyExclusionProofs` now checks if the stxo exclusion proofs are present in the `ExclusionProofs[idx].CommitmentProof` and if so will verify those in preference of the old style proofs.

This commit adds an itest that test whether the stxo exclusion proof is added to the proof suffix and whether the proof is actually a valid exclusion proof for the minimal asset we expect to see in the stxo set.

This commit adds v1 inclusion proof verification.

Since we now produce STXO commitments in certain situations, it makes sense to update the test vectors that are produced from a regtest itest run and are therefore not deterministic.

This commit moves the split commit witness trimming into the addToFundingCommitment and then use that function in both places where we create a commitment from assets during the channel funding process.

To avoid breaking backward compatibility with older nodes and existing channels, we need to make sure we don't add STXO proofs to channel related transactions.

guggero · 2025-05-16T14:23:50Z

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.

guggero

With the above comment and a (hopefully) green CI, I approve this PR. Great work, super happy to finally see this getting over the finish line.

levmi assigned gijswijs Mar 24, 2025

levmi added proofs P1 labels Mar 24, 2025

levmi added this to Taproot-Assets Project Board Mar 24, 2025

github-project-automation bot moved this to 🆕 New in Taproot-Assets Project Board Mar 24, 2025

levmi moved this from 🆕 New to 🏗 In progress in Taproot-Assets Project Board Mar 24, 2025

guggero self-requested a review March 24, 2025 17:48

Roasbeef requested changes Mar 28, 2025

View reviewed changes

github-project-automation bot moved this from 🏗 In progress to 👀 In review in Taproot-Assets Project Board Mar 28, 2025

gijswijs force-pushed the proof-system-v2 branch from 4eef445 to f65456c Compare April 3, 2025 20:45

guggero reviewed Apr 4, 2025

View reviewed changes

gijswijs force-pushed the proof-system-v2 branch 3 times, most recently from 9c4d5c9 to 0055fb3 Compare April 9, 2025 15:29

guggero self-requested a review April 9, 2025 15:30

gijswijs force-pushed the proof-system-v2 branch from 0055fb3 to 7af5271 Compare April 10, 2025 15:27

gijswijs changed the title ~~Proof-system-v2~~ Proof-system-v1 Apr 10, 2025

gijswijs force-pushed the proof-system-v2 branch from 7af5271 to e0e4bf8 Compare April 11, 2025 11:21

guggero mentioned this pull request Apr 11, 2025

Extract non-controversion commits from Proof-system-v1 PR #1465

Merged

guggero force-pushed the proof-system-v2 branch from e0e4bf8 to c971e08 Compare April 11, 2025 12:41

guggero changed the base branch from main to extract-refactors-from-proof-v1 April 11, 2025 12:42

guggero reviewed Apr 11, 2025

View reviewed changes

gijswijs force-pushed the proof-system-v2 branch from c971e08 to da7b212 Compare April 11, 2025 15:13

guggero force-pushed the extract-refactors-from-proof-v1 branch 2 times, most recently from 171f7ae to c8a7971 Compare April 11, 2025 15:19

gijswijs force-pushed the proof-system-v2 branch from da7b212 to 6badb61 Compare April 12, 2025 11:51

guggero changed the base branch from extract-refactors-from-proof-v1 to main April 17, 2025 16:32

gijswijs force-pushed the proof-system-v2 branch from be261fd to 0c55785 Compare May 12, 2025 09:17

Roasbeef requested changes May 13, 2025

View reviewed changes

gijswijs requested a review from Roasbeef May 13, 2025 17:57

gijswijs force-pushed the proof-system-v2 branch from e12c0fd to 0a1d453 Compare May 13, 2025 19:15

Roasbeef reviewed May 13, 2025

View reviewed changes

guggero reviewed May 14, 2025

View reviewed changes

asset/asset.go Outdated Show resolved Hide resolved

proof/append.go Outdated Show resolved Hide resolved

proof/taproot.go Show resolved Hide resolved

proof/verifier.go Outdated Show resolved Hide resolved

proof/verifier.go Outdated Show resolved Hide resolved

gijswijs force-pushed the proof-system-v2 branch from 225aaba to 5cd73a4 Compare May 15, 2025 14:40

gijswijs requested a review from guggero May 15, 2025 14:41

gijswijs force-pushed the proof-system-v2 branch 2 times, most recently from 81c690e to 2bc57a2 Compare May 15, 2025 15:42

gijswijs and others added 14 commits May 16, 2025 13:33

multi: remove AltLeaves from vInputs

6585405

Because we retrieve AltLeaves from the proof during anchor input validation, we don't need to store them in the vInputs struct. This commit removes the AltLeaves field from the vInputs struct and the associated methods.

multi: remove witness parameter from 'NewAltLeaf`

b37437d

Since we're only ever passing nil (and we use the previous witness in the script/burn key), we just remove the witness parameter completely.

tapsend: store STXOs in output commitments

0631116

This commit stores the STXOs in the output commitments using AltLeaves.

proof+tappsbt: add STXOProofs to CommitmentProof

55a5adc

This commit adds a new field to the CommitmentProof struct that will house STXO proofs. We also add encoding/decoding functions for that new sub type and update the test vectors accordingly.

proof: add STXO inclusion proofs for non-splits

91690b7

Co-authored-by: Gijs van Dam <gijs@lightning.engineering>

tapsend+asset: validate with altleaves from proof

f141fce

Instead of using alt leaves from the inputs, we use the alt leaves from the proof. We also remove the deduplication func `AddLeafKeysVerifyMerge` because it's not needed anymore.

tapsend+proof: Add STXO exclusion proof to params

4c76b64

This commit adds the STXO exclusion proof to the proof parameters. It stores them in `params.ExclusionProofs[].CommitmentProof.STXOProofs[]` where they are later encoded when creating the actual proofs.

multi: add new proof version TransitionV1

e502363

The proof version is bumped to indicate the usage and support of stxo style proofs.

proof: Add stxo exclusion proof verification

5714598

`verifyExclusionProofs` now checks if the stxo exclusion proofs are present in the `ExclusionProofs[idx].CommitmentProof` and if so will verify those in preference of the old style proofs.

itest: stxo exclusion proof test

e341586

This commit adds an itest that test whether the stxo exclusion proof is added to the proof suffix and whether the proof is actually a valid exclusion proof for the minimal asset we expect to see in the stxo set.

multi: produce TransitionV1 proofs in unit tests

0656c4f

This commit adds v1 inclusion proof verification.

proof+tappsbt: update regtest test vectors

826e529

Since we now produce STXO commitments in certain situations, it makes sense to update the test vectors that are produced from a regtest itest run and are therefore not deterministic.

tapchannel: refactor commitment creation

1e9e38a

This commit moves the split commit witness trimming into the addToFundingCommitment and then use that function in both places where we create a commitment from assets during the channel funding process.

tapchannel+tapsend: disable STXO asset creation for channels

76c0706

To avoid breaking backward compatibility with older nodes and existing channels, we need to make sure we don't add STXO proofs to channel related transactions.

guggero force-pushed the proof-system-v2 branch from 2bc57a2 to 76c0706 Compare May 16, 2025 14:11

guggero approved these changes May 16, 2025

View reviewed changes

gijswijs requested a review from Roasbeef May 19, 2025 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proof-system-v1 #1453

Proof-system-v1 #1453

gijswijs commented Mar 22, 2025 •

edited by Roasbeef

Loading

coveralls commented Mar 22, 2025 •

edited

Loading

guggero left a comment

guggero Apr 4, 2025

gijswijs Apr 7, 2025

guggero commented Apr 11, 2025

guggero left a comment

guggero commented Apr 11, 2025

Roasbeef left a comment

Roasbeef left a comment

guggero left a comment

Roasbeef commented May 15, 2025

guggero commented May 15, 2025

guggero commented May 16, 2025

guggero left a comment

Proof-system-v1 #1453

Are you sure you want to change the base?

Proof-system-v1 #1453

Conversation

gijswijs commented Mar 22, 2025 • edited by Roasbeef Loading

coveralls commented Mar 22, 2025 • edited Loading

Pull Request Test Coverage Report for Build 15070461131

Details

💛 - Coveralls

guggero left a comment

Choose a reason for hiding this comment

guggero Apr 4, 2025

Choose a reason for hiding this comment

gijswijs Apr 7, 2025

Choose a reason for hiding this comment

guggero commented Apr 11, 2025

guggero left a comment

Choose a reason for hiding this comment

guggero commented Apr 11, 2025

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

Roasbeef commented May 15, 2025

guggero commented May 15, 2025

guggero commented May 16, 2025

guggero left a comment

Choose a reason for hiding this comment

gijswijs commented Mar 22, 2025 •

edited by Roasbeef

Loading

coveralls commented Mar 22, 2025 •

edited

Loading