lightninglabs / taproot-assets

A layer 1 daemon, for the Taproot Assets Protocol specification, written in Go (golang)
MIT License
463 stars 111 forks source link

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

Closed Roasbeef closed 6 months ago

Roasbeef commented 1 year ago

As is today, users can use the vPSBT calls to add arbitrary scripts to encumber tap assets. Right now this needs to be done mostly manually, as we currently only have low-level calls.

In order to allow minting directly into a multi-sig like script on both the Bitcoin and Tap layer, we should allow users to pass in this extra script information either during batch creation, or finalization. With this in place, users would be able to mint directly into an arbitrary on-chain script, as well as tapscript. After this, one missing piece would still be being able to pass in PSBTs into the normal send call, as those PSBT witnesses would be used to carry out the on chain Bitcoin spend.

This issue is meant to track only the minting of assets directly into tapscripts.

Steps To Completion

With the above, users can mint arbitrary asset to a top-level multi-sig anchor output, and also specify arbitrary scripts for each of the asset leaves created in that batch.

dstadulis commented 1 year ago

rather than using the internal wallet key solely as the top level key with a blank tapscript sibling, the tapscript sibling specified by the user should be used in place.

:+1:

Feature unlocks: Minting into a multisig key

dstadulis commented 1 year ago

Some code may exist that @guggero but may not be exposed

dstadulis commented 12 months ago

Highlights:

jharveyb commented 12 months ago

Dug around a bit more, right now we have support for adding custom siblings to addresses and vPSBT outputs, which is exposed via RPC.

Relevant type is commitment.TapscriptPreimage.

jharveyb commented 11 months ago

Another consideration is the storage of the full tapscript tree being used here. There are a few options:

Roasbeef commented 10 months ago

lnd API that lets use store arbitrary tapscript stuff: https://lightning.engineering/api-docs/api/lnd/wallet-kit/import-tapscript

jharveyb commented 10 months ago

lnd API that lets use store arbitrary tapscript stuff: https://lightning.engineering/api-docs/api/lnd/wallet-kit/import-tapscript

IIUC there is no matching 'export' option to allow a user to generate a valid script spend from an imported tapscript tree? So we'd want to maybe reuse the code behind this import call to implement option # 2 outlined above.

Roasbeef commented 10 months ago

IIUC there is no matching 'export' option to allow a user to generate a valid script spend from an imported tapscript tree?

IIRC, it ends up showing up as an account, we have some itests that exercise this: https://github.com/lightningnetwork/lnd/blob/9afe1b72dcc845b275af8d22036f2fa4c484eeea/itest/lnd_taproot_test.go#L1091

jharveyb commented 10 months ago

Update from offline discussion:

The LND importTapscript API doesn't really fit here. It would cause a lot of excess chain-watching for keys that should never appear on-chain (like a tapscript tree root for a script key or group key). There also isn't a great way to recall the tapscript tree, which is needed for witness validation (and possibly also for exposing to the end user).

The current plan for the internal API / interface is for an abstract interface that supports storing a mapping between a Schnorr key (the internal key) and a full tapscript tree. The other operation would be fetching the full tree given a Schnorr key.

We can implement this with the DB by adding a new table(s) that has a script key or group key and a foreign key, and stores the tapscript tree as a serialized blob.

We'll need to fetch this tree during witness validation (like perhaps a sanity check for when users provide a custom witness?). For the initial PR we can implement MVP tests by only checking a non-empty tapscript tree and accompanying witness with the VM vs. actually exposing any of this in the minter.

jharveyb commented 10 months ago

Relevant RPC types we can directly borrow from LND (omitting the full key only option):

https://github.com/lightningnetwork/lnd/blob/2824fe27d1525beaa056224bf5a3783cdaa215cf/lnrpc/walletrpc/walletkit.proto#L667

One other wrinkle that's appearing is storing tweaks for incoming seedling requests; these requests don't have script keys yet, so there is no key to associated a tweak with. I think we could instead have a two-step lookup, where we have one table mapping (asset_minting_batches.batch_id, asset_seedlings.asset_name) to the tweak, and then another mapping internal_key to an entry in the first table.

So we create the first entry when storing the seedling, and then the second after we derive the internal key for the asset script key.

This definitely expands the scope of the DB changes I was thinking were necessary for this feature, but I think it makes sense it we want to have the same storage/persistence guarantees for these tweaks that we have now for the full seedling.

jharveyb commented 9 months ago

A relevant existing DB field is script_keys.tweak: https://github.com/lightninglabs/taproot-assets/blob/c44a9cd07ef97877eaedcfa05544c06f49dc8380/tapdb/sqlc/migrations/000002_assets.up.sql#L178

Which could be used as a pseudo-foreign key to the new table storing tapscript trees. So that new table could have an optional foreign key to internal_keys OR store the tapscript root for a tree.

Adding a new field to asset_seedlings may be a better option overall.

jharveyb commented 9 months ago

Re: DB layout, I'm now in favor of a minimal table of just (root_hash, raw_tree).

The asset_groups and script_keys tables already have a field for referencing a tweak or tapscript root, so the tapscript tree root hash can be used like a foreign key in these cases.

Avoiding any actual foreign keys should make it easier to reuse tapscript trees, once stored, across multiple key types. Ex. I construct a 2-of-3 multisig tree and store it, and then use that tree as a tweak for both an asset script key and the internal key when deriving the on-chain taproot key.

Roasbeef commented 9 months ago

Re: DB layout, I'm now in favor of a minimal table of just (root_hash, raw_tree).

So raw_tree here is the TLV serialization of the wallet struct?

If we wanted to denormalize a bit, we could split it up into something like this:

CREATE TABLE tapscript_root (
     id BIGINT PRIMARY KEY, 

     root_hash BLOB NOT NULL UNIQUE
)

CREATE TABLE tapscript_scripts (
     id BIGINT PRIMARY KEY, 

     serialized_script_leaf BLOB NOT NULL UNIQUE,
)

CREATE TABLE tapscript_leaves (
     id BIGINT PRIMARY KEY,

     raw_script_id BIGINT NOT NULL REFERNCES tapscript_scripts(id),

     tapscript_root_hash_id BIGINT NOT NULL REFERNCES tapscript_root(id)
)

This minimizes the amount of duplicate data, and allows stuff like tapscript_scripts to be re-used.

Then as you mentioned for script_keys, we'd have it store the tapscript_root.root_hash field.

Avoiding any actual foreign keys should make it easier to reuse tapscript trees, once stored, across multiple key types. Ex. I construct a 2-of-3 multisig tree and store it, and then use that tree as a tweak for both an asset script key and the internal key when deriving the on-chain taproot key.

Why isn't it possible with the version listed above? You still include the root_hash field as the tweak value.

With the version above, you're able to re-use even more as a script is free floating and can be included in any leaf you want to create.

jharveyb commented 9 months ago

Re: DB layout, I'm now in favor of a minimal table of just (root_hash, raw_tree).

So raw_tree here is the TLV serialization of the wallet struct?

Yes, though storing leaves and root hashes as you described does seem better.

If we wanted to denormalize a bit, we could split it up into something like this:

CREATE TABLE tapscript_root (
     id BIGINT PRIMARY KEY, 

     root_hash BLOB NOT NULL UNIQUE
)

CREATE TABLE tapscript_scripts (
     id BIGINT PRIMARY KEY, 

     serialized_script_leaf BLOB NOT NULL UNIQUE,
)

CREATE TABLE tapscript_leaves (
     id BIGINT PRIMARY KEY,

     raw_script_id BIGINT NOT NULL REFERNCES tapscript_scripts(id),

     tapscript_root_hash_id BIGINT NOT NULL REFERNCES tapscript_root(id)
)

This minimizes the amount of duplicate data, and allows stuff like tapscript_scripts to be re-used.

This does look like a better option than storing a big serialized blob. IIUC we don't need any extra UNIQUE constraint on the tapscript_leaves table.

Then as you mentioned for script_keys, we'd have it store the tapscript_root.root_hash field.

Avoiding any actual foreign keys should make it easier to reuse tapscript trees, once stored, across multiple key types. Ex. I construct a 2-of-3 multisig tree and store it, and then use that tree as a tweak for both an asset script key and the internal key when deriving the on-chain taproot key.

Why isn't it possible with the version listed above? You still include the root_hash field as the tweak value.

I should have been more clear there, I meant something like having a internal_key_id as part of tapscript_root. Which would then prevent associating the same tapscript root with multiple keys. I think we both agree that just referencing the tree root hash is the better option for associating a key and a tapscript tree.

jharveyb commented 9 months ago

Writing back offline discussion:

The links from rootHash to scriptLeaf must also include an index, as the leaves are ordered and that order must persist across a store & load.

The logic to assemble a Tapscript tree in txscript only builds a balanced Tapscript tree, but users may want to use an external tool to build an unbalanced tree (as mentioned in the BIP, this could save on fees if they know the probability of using some or all of the scripts included). To support that use, we can accept either two TapHashes (that represent the top two internal nodes in a Tapscript tree) or a list of TapLeaf objects (we'll build and store a balanced Tapscript tree).

jharveyb commented 7 months ago

PR to implement the first two features will have to be a follow-up to #827.

jharveyb commented 6 months ago

Implemented in #827 , being exposed in #866 .