Group key witness support #490

jharveyb · 2023-09-06T22:50:09Z

Implements changes outlined in #466.

Known TODOs:

VM execution in DeriveGroupKey to verify witness correctness
Copy group witness to asset.PrevWitnesses where need so the witness gets serialized
update proof.Verify to use new genesis asset definition
Resolve dependency cycle with tapscript package
Restore working unit tests
Restore working itests

Approach:

Modify the virtualTx methods to add new branches for creating a virtual TX for a genesis grouped asset.
Modify the VM to prevent an early exit for genesis grouped assets, and add early assertions for such assets.
Update DeriveGroupKey to sign over a virtualTx representing minting of a genesis grouped asset.
Update DeriveGroupKey to accept external scripts and support different signing methods.

Open questions:

Can we reuse the existing VirtualTxSigner for the GenesisSigner?
Is there other data needed for a group key script path authorization here besides a single TapLeaf? In CreateTaprootSignature I see validation against other fields but not use of those other fields for signing.
How can we resolve the tapscript dependency cycle? I see two issues, one being the placement of RawKeyGenesisSigner code, which AFAICT is only used in tests. The other is that, the asset package can't call methods from the tapscript package without causing a cycle, but with the current setup DeriveGroupKey must build a virtual TX at some point. I think one option may be to move some of the group signing logic into the tapscript package. As-is I think the definition of DeriveGroupKey could move into tapscript and self-contained parts of that logic could stay in the asset package.

jharveyb · 2023-09-11T22:16:25Z

I don't see a neat solution to the dependency cycle issues, so just moving some of the virtual TX creation logic for these special group genesis assets into asset and accepting some duplication with tapscript to keep this moving forward.

halseth

Def looks to be on the right track! 😀

asset/asset.go

tapscript/tx.go

Roasbeef · 2023-09-12T19:00:05Z

Can we reuse the existing VirtualTxSigner for the GenesisSigner?

I think so? Would imagine the main interface can stay, then the implementation of GenesisSigner can take in a virtual signer.

Is there other data needed for a group key script path authorization here besides a single TapLeaf? In CreateTaprootSignature I see validation against other fields but not use of those other fields for signing.

What do you mean by this? The script path auth is just the existing witness field right? Or do you mean the group key derivation reveal? The group key witness (for minting) should be more or less the same as the flow for signing virtual txns normally. IIUC, we don't need any new fields in the VInput, instead the existing fields are populated as normal. The main difference now is that we swap out the public key (not the input asset) for the group key asset.

In other words, the group key witness is the same thing as signing a normal virtual tx. The only difference is that the input key is the group key, instead of the actual previous asset input. The output then commits to the script key for minting as normal.

jharveyb · 2023-09-12T19:29:17Z

What do you mean by this? The script path auth is just the existing witness field right? Or do you mean the group key derivation reveal? The group key witness (for minting) should be more or less the same as the flow for signing virtual txns normally. IIUC, we don't need any new fields in the VInput, instead the existing fields are populated as normal. The main difference now is that we swap out the public key (not the input asset) for the group key asset.

From the RPC level, what data types to I need to provide as arguments when minting a grouped asset into a group, where I want to use a script path and not a simple signature?

And then, how does that get passed in to the final signing descriptor (I think this part is clearer to me now).

Roasbeef · 2023-09-12T21:38:38Z

From the RPC level, what data types to I need to provide as arguments when minting a grouped asset into a group, where I want to use a script path and not a simple signature?

Ah, I think for that, we'd just make sure we're able to handle such spends internally via unit tests, then defer the expansion of the RPC interface to satisfy more complex group minting conditions. So to start, we can just allow them to specify their own group key and we use that instead of the auto generated one. The caller would then need to handle storing all the tapcript leaves, just like the itests we have to do non keyspend spends for normal assets.

With that said, if we wanted to do some design sketches, I'd say:

what data types to I need to provide as arguments when minting a grouped asset into a group, where I want to use a script path and not a simple signature?

You'd need to specify:

The group key to use.
The full witness field for that group key.

We can't necessarily generate the witness ourselves, as we may not even control the key used. Instead, the caller would use the vPSBT APIs to craft the minting transaction. To facilitate this, we'd need to expose calls that can be used to lock UTXOs (they want to know which outpoint will be used), or we'd add that as an arg to the minting process.

And then, how does that get passed in to the final signing descriptor (I think this part is clearer to me now).

We don't do signing in this context, as we might not be able to. The minting witness is just the witness of a "normal" spend, but it can spend the "all zero" outpoint, as it's a proof that the group key can be satisfied. You can think of it like the PSBT based channel funding for lnd: we don't know what inputs the caller is using, but we can provide them with the information needed to make the funding transaction (the funding outpoint).

asset/asset.go

tapscript/tx.go

asset/asset.go

jharveyb · 2023-09-13T17:15:38Z

Read through the feedback, my current plan is:

Move the virtual TX construction logic behind another interface, asset.GenesisTxBuilder, to resolve dependency issues.
Simplify DeriveGroupKey to only handle the key spend path with an empty tapscript root.
New methods on Asset so its clearer when a grouped genesis asset needs a witness or already has a witness.
Update asset.new() or add logic to the minter s.t. the group key witness is copied into the asset during minting.

WIP branch is here which I'll swap in to this PR: https://github.com/lightninglabs/taproot-assets/tree/group-key-witness-support-wip

jharveyb · 2023-09-14T14:50:05Z

Latest version is on this branch with the described changes. The commits were re-done and should be relatively readable.

Moving on to improving test coverage.

jharveyb · 2023-09-14T22:42:53Z

Another source of dependency issues:

taproot-assets/commitment/asset.go

Line 151 in 843584a

validSig := newAsset.Genesis.VerifySignature(

Verifying the group witness on asset commitment insertion.

Roasbeef · 2023-09-14T22:59:24Z

Verifying the group witness on asset commitment insertion.

So I think we can actually move this out entirely. Looks like the only time we call VerifySignature is Upsert and also when we make a new asset commitment. Now that we have the group key reveal stuff in the proof file verification, we can just move that there. So now we only verify the signature when we create assets, and also when we verify proofs for assets.

asset/mock.go

tapscript/mint.go

asset/asset.go

tapscript/mint.go

virtual_tx_builder.go

asset/asset.go

Roasbeef · 2023-09-14T23:24:43Z

asset/asset.go

+	// Compute the tweaked group key and set it in the asset before
+	// creating the virtual minting transaction.
+	genesisTweak := initialGen.ID()
+	tweakedGroupKey, err := GroupPubKey(rawKey.PubKey, genesisTweak[:], nil)


Eventually should also have the script root here. Could have it be a functional option or something like that for now.

asset/asset.go

jharveyb · 2023-09-15T17:25:36Z

Addressed a few comments, updated this main branch and the WIP branch with unfinished test fixes here:

https://github.com/lightninglabs/taproot-assets/tree/group-key-witness-support-wip

There was an issue with not using the correct branching condition in VirtualTx for reaching VirtualGenesisTxIn - since VirtualTx is called both when building the Virtual TX for signing, and when verifying in the VM, I think we'd actually want to branch on the genesis grouped asset both before and after the group witness has been attached.

However, VM validation for normal state transitions is now failing, so I think the branch condition as I've written it is too broad - I think this could be fixed by also considering prevAssets at that point. So something like:

If both the new asset and the prev asset are genesis grouped assets, then it must be a genesis grouped asset mint, branch to VirtualGenesisTxIn.

We still have the sanity check very early in the VM that I think should stay as HasGenesisWitnessForGroup.

The WIP branch also has some fixes for other tests but those haven't been exercised yet since they usually depend on the VM working anyways.

jharveyb · 2023-09-19T16:16:33Z

Unit tests (excluding one checking the generated proof file) are passing, moving on to itest fixes now.

guggero

Great work, very nice to see this getting close.

asset/asset.go

asset/mock.go

asset/tx.go

asset/interface.go

tapscript/mint.go

asset/asset.go

tapgarden/caretaker.go

commitment/commitment_test.go

vm/vm.go

jharveyb · 2023-09-21T14:43:41Z

Latest push is further fixes to address some unit test fixes + fix group insertion when syncing from a Universe (h/t guggero).

Most itests pass, but reissuance does not work - adding a GroupVerifier interface now to be used when verifying proofs, to check if there is a verified group that matches the group key in the proof.

jharveyb · 2023-09-25T14:32:58Z

Adding a groupVerifier has worked for fixing the reissuance test.

From offline discussion, I realized that the GroupKeyReveal should be omitted for genesis proofs of reissued assets, and instead the asset's group key should only be checked against stored group keys to ensure that the daemon has previously verified the group anchor (along with group witness verification).

However, this introduces new ordering constraints in many places where we handle grouped assets (storing to disk, verifying a proof, or populating a universe tree). The anchor asset for a group MUST be inserted before attempting to verify or store any other member of that asset group.

Right now many DB operations are unordered over e.x. the set of all assets in a minting batch, which I'm updating now.

asset/tx.go

asset/asset.go

tapcfg/server.go

In this commit, we switch the group tapscript root to a slice to better represent an empty tapscript root (BIP-86 tweaking). We also fix some areas where a non-nil root on disk was not returned from DB queries.

In this commit, we define valid genesis assets when members of a group. We also define virtual TxIns and TxOuts for such assets. We combine these changes to support making VirtualTxs for genesis grouped assets with existing virtualTx constructors.

In this commit, we define a new interface for constructing virtual transactions used to represent minting of grouped assets, GenesisTxBuilder. We also redefine GenesisSigner to match VirtualTxSigner, as they both use the same underlying signing API.

In this commit, we modify the early VM sanity checks to prevent an early exit for genesis grouped assets. We also add support for the GenesisPrevOutFetcher, which is needed for virtual TX construction in the VM to match the TX used to produce the group witness.

In this commit, we update DeriveGroupKey to construct a virtualTx representing the mint and produce a witness using the new group key definition that supports script-path spends.

In this commit, we extend the config used to initialize the planter to include the new interfaces needed for group asset minting. We also reorder some operations in the caretaker to simplify the usage of DeriveGroupKey.

In this commit, we update the minter the ensure that valid group witnesses are preesent in genesis grouped assets. Group witnesses are now copied into the asset PrevWitness on asset creation, and then validated by the VM before the minter continues sprouting seedlings.

In this commit, we introduce two new verifier callbacks, which are needed to check that group keys attached to assets have already been imported and verified. This is relevant during proof verification, minting, and universe sync, amongst other areas.

In this commit, we update proof verification to verify group keys for reissued assets and grouped asset transfers. We also update callers of proof.Verify() to passthrough group verifier callbacks.

In this commit, we address the issue of asset insertion ordering during minting. Group anchors must be inserted before reissued assets in the same batch for group info to be stored correctly and asset verification to succeed. We also update configs for other minting subsystems to propogate group verifier callbacks.

In this commit, we ensure that issuances for grouped assets are handled correctly during universe sync. This requires inserting group anchors before reissuances. We also update the group key insertion on universe sync to handle both group anchors and reissuances.

In this commit, we remove vestigial verification of group witnesses when constructing an asset commitment or upserting a leaf that was a grouped asset. This verification is now done on asset minting, proof export and import, and when syncing bootstrap information for an asset group.

halseth

This looks great now 👌 💯

jharveyb · 2023-10-03T11:02:13Z

Rebased and addressed most recent feedback.

Roasbeef

LGTM 🍌

jharveyb requested review from Roasbeef and halseth September 6, 2023 22:50

jharveyb self-assigned this Sep 8, 2023

halseth reviewed Sep 12, 2023

View reviewed changes

asset/asset.go Outdated Show resolved Hide resolved

tapscript/tx.go Outdated Show resolved Hide resolved

tapscript/tx.go Outdated Show resolved Hide resolved

tapscript/tx.go Outdated Show resolved Hide resolved

tapscript/tx.go Outdated Show resolved Hide resolved

Roasbeef requested changes Sep 13, 2023

View reviewed changes

jharveyb force-pushed the group-key-witness-support branch from afe2f53 to 843584a Compare September 14, 2023 14:33

Roasbeef requested changes Sep 14, 2023

View reviewed changes

jharveyb force-pushed the group-key-witness-support branch 2 times, most recently from d02d4ac to f9b692f Compare September 15, 2023 17:13

guggero force-pushed the v0-3-0-staging branch from c4bddfa to eafaca4 Compare September 18, 2023 15:17

jharveyb force-pushed the group-key-witness-support branch 2 times, most recently from fb3ddaf to 3a32d59 Compare September 19, 2023 11:37

guggero self-requested a review September 19, 2023 14:10

jharveyb force-pushed the group-key-witness-support branch from 3a32d59 to 9bd6cdf Compare September 19, 2023 16:03

guggero reviewed Sep 20, 2023

View reviewed changes

jharveyb force-pushed the group-key-witness-support branch from 9bd6cdf to 7618af1 Compare September 21, 2023 14:39

halseth reviewed Sep 26, 2023

View reviewed changes

asset/tx.go Show resolved Hide resolved

asset/tx.go Outdated Show resolved Hide resolved

asset/asset.go Show resolved Hide resolved

tapcfg/server.go Outdated Show resolved Hide resolved

jharveyb force-pushed the group-key-witness-support branch from 02e13fa to d4ce176 Compare September 26, 2023 16:34

guggero force-pushed the v0-3-0-staging branch from eafaca4 to cbcfdd7 Compare October 2, 2023 15:56

jharveyb added 3 commits October 2, 2023 21:17

tapscript: unify mock signing with asset package

a663052

asset+tapdb: make group tapscriptRoot optional

a4cae87

In this commit, we switch the group tapscript root to a slice to better represent an empty tapscript root (BIP-86 tweaking). We also fix some areas where a non-nil root on disk was not returned from DB queries.

jharveyb force-pushed the group-key-witness-support branch from 6476137 to 394a02d Compare October 2, 2023 21:51

jharveyb added 19 commits October 3, 2023 02:16

asset: produce group witnesses over virtual TXs

9d1b761

In this commit, we update DeriveGroupKey to construct a virtualTx representing the mint and produce a witness using the new group key definition that supports script-path spends.

commitment+tapdb: update DeriveGroupKey usage

afc499b

tapgarden+tapcfg: add new interfaces to garden cfg

8bd0aab

In this commit, we extend the config used to initialize the planter to include the new interfaces needed for group asset minting. We also reorder some operations in the caretaker to simplify the usage of DeriveGroupKey.

proof+tapdb: utilize + propogate group verifiers

6d85804

In this commit, we update proof verification to verify group keys for reissued assets and grouped asset transfers. We also update callers of proof.Verify() to passthrough group verifier callbacks.

tapfreighter+tapscript: propogate group verifiers

670b380

rpserver+tapcfg: propogate group verifiers

228bdea

proof: update genesis asset definition

46d2ed7

address+rpcserver: drop group witness from address

308e30e

address+tapscript+vm: Update test setup + helpers

7bb3bda

itest: update test helpers

40100f8

multi: update test vectors to include new reveals

401f91d

tapfreighter+tapdb: fix some loop memory aliasing

6686744

jharveyb force-pushed the group-key-witness-support branch from 394a02d to 6686744 Compare October 3, 2023 00:38

halseth approved these changes Oct 3, 2023

View reviewed changes

Roasbeef approved these changes Oct 3, 2023

View reviewed changes

Roasbeef merged commit 8899ca4 into v0-3-0-staging Oct 3, 2023
14 checks passed

guggero deleted the group-key-witness-support branch October 4, 2023 07:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Group key witness support #490

Group key witness support #490

jharveyb commented Sep 6, 2023 •

edited

Loading

jharveyb commented Sep 11, 2023

halseth left a comment

Roasbeef commented Sep 12, 2023 •

edited

Loading

jharveyb commented Sep 12, 2023

Roasbeef commented Sep 12, 2023

jharveyb commented Sep 13, 2023 •

edited

Loading

jharveyb commented Sep 14, 2023

jharveyb commented Sep 14, 2023

Roasbeef commented Sep 14, 2023

Roasbeef Sep 14, 2023

jharveyb commented Sep 15, 2023

jharveyb commented Sep 19, 2023

guggero left a comment

jharveyb commented Sep 21, 2023

jharveyb commented Sep 25, 2023

halseth left a comment

jharveyb commented Oct 3, 2023

Roasbeef left a comment

Group key witness support #490

Group key witness support #490

Conversation

jharveyb commented Sep 6, 2023 • edited Loading

jharveyb commented Sep 11, 2023

halseth left a comment

Choose a reason for hiding this comment

Roasbeef commented Sep 12, 2023 • edited Loading

jharveyb commented Sep 12, 2023

Roasbeef commented Sep 12, 2023

jharveyb commented Sep 13, 2023 • edited Loading

jharveyb commented Sep 14, 2023

jharveyb commented Sep 14, 2023

Roasbeef commented Sep 14, 2023

Roasbeef Sep 14, 2023

Choose a reason for hiding this comment

jharveyb commented Sep 15, 2023

jharveyb commented Sep 19, 2023

guggero left a comment

Choose a reason for hiding this comment

jharveyb commented Sep 21, 2023

jharveyb commented Sep 25, 2023

halseth left a comment

Choose a reason for hiding this comment

jharveyb commented Oct 3, 2023

Roasbeef left a comment

Choose a reason for hiding this comment

jharveyb commented Sep 6, 2023 •

edited

Loading

Roasbeef commented Sep 12, 2023 •

edited

Loading

jharveyb commented Sep 13, 2023 •

edited

Loading