tapgarden: tapscript tree support for minting anchor outputs #768

jharveyb · 2024-01-16T20:03:55Z

Fixes #343.

Adding support for a batch key tapscript sibling first (BTC anchor-level), which would be supplied during the Finalize call for the batch. This should be a smaller change than per-asset support, while still using similar interfaces and DB changes.

TODOs:

unit tests for taptree en/decode
unit tests for tapscript sibling during Finalize call

I think given the related DB changes the per-asset sibling support should be in this PR (adding further TODOs) vs. a separate PR, but that may cause some commit reordering later on.

This change is

jharveyb · 2024-01-18T01:15:06Z

itests are panicking as the DB-backed implementation of TapscriptTreeStore is not wired up yet.

dstadulis · 2024-01-22T18:10:51Z

Next milestone for the PR:
Have the unit tests passing

We will try to split up the PR. if the itest test be modified to be independent to the DB changes, PR would be possible to be split.

jharveyb · 2024-01-23T17:55:05Z

Proof generation during minting now includes the tapscript sibling, and this includes the fixes for tapscript preimage verification.

The DB implementation will be a follow-on PR.

Roasbeef

Nice work! This helps us cover the ability to use custom tapscript trees at the top-level Bitcoin output. Spotted a few areas we can use some new types to avoid nil pointer checks when we want an arg to actually be optional w/o relying on any specific pointer semantics.

I think to round out this PR, we'll want to make a concrete implementation for the new tree store in the DB. We can denormalize a bit by having a table that refs an internal key and then build out the tree storage in another table to be ref'd as a foreign key. This'll let us de-dup a bit, as we can have the tree storage primary key be the root hash.

Then we can add an itest to ensure that we can spend from the output using normal keypend. From there we can opt to push out the final component of accepting a valid witness somehow (send call that uses a PSBT?) so we can make further progess on the group key component.

asset/encoding.go

asset/mock.go

internal/test/helpers.go

tapgarden/batch.go

tapgarden/caretaker.go

Roasbeef · 2024-01-23T23:01:33Z

commitment/taproot.go

-	// Enforce that it is not including another Taproot Asset commitment.
-	if bytes.Contains(preimage, TaprootAssetsMarker[:]) {
+	// Verify that the script is not including a Taproot Asset commitment.
+	if IsTaprootAssetCommitmentScript(script) {


I think ok as is if we want to keep the exiting control flow, but in the future I'd like to re-work the control flow to assert all the preconditions up front, then have this just be a dumb method that computes the tap leaf hash.

agree.. Dumb method "TapLeafHash" and smart method which checks whatever validity constraints on the leaf

Went with always enforcing these checks before computing the TapHash, as sometimes we're initializing TapscriptPreimage from a decoded Proof vs. using a safe constructor.

commitment/taproot.go

jharveyb · 2024-01-24T17:23:25Z

Expanding this PR to include the DB implementation and a itest that will cover a key-spend & script spend of the minting output.

commitment/taproot.go

asset/asset.go

asset/encoding.go

commitment/taproot.go

rpcserver.go

tapgarden/planter_test.go

jharveyb · 2024-02-05T16:46:01Z

Latest push adds a DB implementation of TapscriptTreeManager, without unit tests. We also flesh out the TapscriptTreeNodes type, which represents the two formats we'll accept for tapscript trees on the RPC, and store tapscript trees on disk.

Remaining TODOs, in descending priority order:

tapdb unit tests for implementation of TapscriptTreeManager
DB changes to add tapscript sibling hash for minting batches + accompanying unit test coverage
Itest with a non-empty minting batch tapscript tree + script path spend of minting UTXO
Refactor commitment.TapscriptPreimage type to use safe constructors and getters, extend test coverage for tapscript sibling validation
Examine restart-safety for TapscriptTreeManager operations

jharveyb · 2024-02-23T18:43:18Z

Pushed final update with some commits in the middle to finish making commitment.TapscriptPreimage a safer type. Added extra validation logic to constructors but also TapHash() itself, as that gets used in proof validation logic on a preimage decoded directly from a proof file (so no checks were performed before that call).

Looks like I've introduced a flake to the multi-asset group itest + changed the preimage encoding unintentionally, investigating now. The intent is to not change the preimage encoded format in this PR.

Rebased + test issues are resolved, ready for final review.

guggero

Awesome, LGTM 🎉

asset/interface.go

tapgarden/interface.go

tapgarden/planter.go

In this commit, we introduce the TapscriptTreeManager. This interface stores a tapscript tree associated with a root hash, which could be a stored alongside a batch key, asset script key, or group key.

This commit adds three new tables that enable us to store tapscript trees, with deduplication of tapscript leaves. A tree can also be stored as two TapHashes representing a TapBranch, allowing for externally managed tapscript trees. These tables are not intended to be modified with the basic queries added in this commit, but wrapper functions in the next commit.

In this commit, we expose helpers to upsert and delete tapscript trees. These are used to add support for tapscript trees during minting.

In this commit, we remove an overly strict version check when asserting that a Tapscript script is not a Taproot Asset Commitment. All versions, including unknown future versions, should be rejected in this context.

In this commit, we update how tapscript preimages are validated before use. We define safe constructors for tapscript preimages, and require their use by making the raw preimage bytes private. We also add checks when decoding a preimage to further prevent improper construction.

Roasbeef

LGTM 🦖

jharveyb self-assigned this Jan 17, 2024

jharveyb linked an issue Jan 17, 2024 that may be closed by this pull request

tapgarden/taprpc: permit providing a tapscript sibling hash for minting calls #343

Closed

3 tasks

dstadulis requested a review from Roasbeef January 22, 2024 16:37

dstadulis added this to the v0.4 milestone Jan 22, 2024

jharveyb force-pushed the minting_tap_tree_support branch from 026d404 to 77cf410 Compare January 23, 2024 07:03

jharveyb marked this pull request as ready for review January 23, 2024 17:53

jharveyb requested review from guggero and ffranr and removed request for Roasbeef January 23, 2024 17:53

jharveyb changed the title ~~WIP: tapscript tree support during minting~~ tapgarden: tapscript tree support for minting anchor outputs Jan 23, 2024

Roasbeef requested review from Roasbeef and removed request for guggero January 23, 2024 19:53

Roasbeef requested changes Jan 23, 2024

View reviewed changes

JeremyRubin reviewed Jan 24, 2024

View reviewed changes

commitment/taproot.go Outdated Show resolved Hide resolved

JeremyRubin reviewed Jan 24, 2024

View reviewed changes

commitment/taproot.go Outdated Show resolved Hide resolved

JeremyRubin reviewed Jan 24, 2024

View reviewed changes

commitment/taproot.go Outdated Show resolved Hide resolved

Roasbeef reviewed Jan 24, 2024

View reviewed changes

commitment/taproot.go Outdated Show resolved Hide resolved

ffranr removed their request for review January 25, 2024 16:50

jharveyb force-pushed the minting_tap_tree_support branch 3 times, most recently from 2f06c7c to fb4df87 Compare January 30, 2024 23:43

Roasbeef requested changes Jan 31, 2024

View reviewed changes

jharveyb force-pushed the minting_tap_tree_support branch 3 times, most recently from 268dd37 to 7b130ff Compare February 5, 2024 16:34

This was referenced Feb 22, 2024

[bug]: unit flake: race in custodian from proof import #808

Closed

[feature]: custom Group Key API for minting #722

Closed

jharveyb added 2 commits February 23, 2024 13:14

fn: update helper funcs for option and either

0213563

asset: add en/decode support for taptree nodes

117e19b

jharveyb force-pushed the minting_tap_tree_support branch from f17b2d7 to cbac1e1 Compare February 23, 2024 18:22

jharveyb requested review from guggero and Roasbeef February 23, 2024 18:42

guggero approved these changes Feb 26, 2024

View reviewed changes

asset/interface.go Outdated Show resolved Hide resolved

tapgarden/interface.go Show resolved Hide resolved

tapgarden/planter.go Show resolved Hide resolved

jharveyb added 17 commits February 26, 2024 15:01

asset+tapgarden: TapscriptTreeManager interface

db7c1de

In this commit, we introduce the TapscriptTreeManager. This interface stores a tapscript tree associated with a root hash, which could be a stored alongside a batch key, asset script key, or group key.

tapdb: add tapscript sibling to batch table

f4634c5

tapdb: MintingStore supports TapscriptTreeManager

9160bfa

In this commit, we expose helpers to upsert and delete tapscript trees. These are used to add support for tapscript trees during minting.

tapgarden: add tapscript tree arg to finalize call

a6e7f53

proof: add tapscript sibling as a minting blob opt

491737d

commitment: remove vers check in commitment checks

3e103a3

In this commit, we remove an overly strict version check when asserting that a Tapscript script is not a Taproot Asset Commitment. All versions, including unknown future versions, should be rejected in this context.

proof: update handling of tapscript preimages

a030f57

address: update handling of tapscript preimages

f6dd14a

multi: update test handling of tapscript preimages

2d3d1e1

tapgarden: propogate taptree sibling for batch key

8856f68

tapdb: update minting queries to set batch sibling

28a5ccc

taprpc: add tapscript sibling to finalize call

8169ee4

rpc: add tapscript sibling to FinalizeBatch

f5fef0f

tapgarden: test minting with batch tapscript tree

96c5357

itest: add script path spend of minting output

35c8156

jharveyb force-pushed the minting_tap_tree_support branch from cbac1e1 to 35c8156 Compare February 26, 2024 20:02

Roasbeef approved these changes Feb 27, 2024

View reviewed changes

Roasbeef merged commit bab8f11 into main Feb 27, 2024
14 checks passed

guggero deleted the minting_tap_tree_support branch February 27, 2024 07:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tapgarden: tapscript tree support for minting anchor outputs #768

tapgarden: tapscript tree support for minting anchor outputs #768

jharveyb commented Jan 16, 2024 •

edited by Roasbeef

Loading

jharveyb commented Jan 18, 2024

dstadulis commented Jan 22, 2024

jharveyb commented Jan 23, 2024

Roasbeef left a comment

Roasbeef Jan 23, 2024

JeremyRubin Jan 24, 2024

jharveyb Feb 21, 2024

jharveyb commented Jan 24, 2024

jharveyb commented Feb 5, 2024 •

edited

Loading

jharveyb commented Feb 23, 2024

guggero left a comment

Roasbeef left a comment

tapgarden: tapscript tree support for minting anchor outputs #768

tapgarden: tapscript tree support for minting anchor outputs #768

Conversation

jharveyb commented Jan 16, 2024 • edited by Roasbeef Loading

jharveyb commented Jan 18, 2024

dstadulis commented Jan 22, 2024

jharveyb commented Jan 23, 2024

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef Jan 23, 2024

Choose a reason for hiding this comment

JeremyRubin Jan 24, 2024

Choose a reason for hiding this comment

jharveyb Feb 21, 2024

Choose a reason for hiding this comment

jharveyb commented Jan 24, 2024

jharveyb commented Feb 5, 2024 • edited Loading

jharveyb commented Feb 23, 2024

guggero left a comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

jharveyb commented Jan 16, 2024 •

edited by Roasbeef

Loading

jharveyb commented Feb 5, 2024 •

edited

Loading