commitment: Add support for minting new Taro commitments #23

wpaulino · 2022-04-19T21:54:04Z

AssetCommitment represents the inner MS-SMT within the Taro protocol committing to a set of assets under the same ID/family. Assets within this tree, which are leaves represented as the serialized asset TLV payload, are keyed by their asset_script_key.

TaroCommitment represents the outer MS-SMT within the Taro protocol committing to a set of asset commitments. Asset commitments, which are leaves represented as asset_version || asset_tree_root || asset_sum, are keyed by their asset_family_key or asset_id otherwise.

wpaulino · 2022-04-19T21:56:47Z

Looking for feedback on whether these are good abstractions to move forward with. I haven't written any tests for this yet since we don't have any test vectors and the tests would just end up reusing the same internal routines as the code itself, which doesn't achieve much. Ideally the test vectors expose the actual byte streams so that we can verify correctness that way.

wpaulino · 2022-04-21T02:09:10Z

Pushed a new update using the initial abstractions added to implement verification, I think they fit quite nicely. With what's present in the current diff, we should have enough to start writing some tests demonstrating inclusion and exclusion asset proofs within Taro commitments. I'll likely get to this after a first review pass.

Roasbeef · 2022-04-22T01:41:12Z

With what's present in the current diff, we should have enough to start writing some tests demonstrating inclusion and exclusion asset proofs within Taro commitments.

Dope! Looks like the creation of the 1-of-1 holographic beefzard card isn't too far off...

Thinking more about collectibles, it may make sense to eventually define structured proofs either as part of the asset meta or the body. As an example, lets say I want to do something similar to the keybase sig chain, and link the creation of said collectible to my other verified internets identities (github, twitter, keybase, etc).

asset/asset.go

commitment/asset.go

Roasbeef · 2022-04-22T01:34:30Z

commitment/taro.go

+	var rootSum [8]byte
+	binary.BigEndian.PutUint64(rootSum[:], c.TreeRoot.NodeSum())
+	leafParts := [][]byte{
+		{byte(c.Version)}, TaroMarker[:], rootHash[:], rootSum[:],


I've been thinking of even using an invalid leaf version here, which would make the verification of commitment uniqueness as bit easier...

Though the market byte also already enables this type of verification.

Roasbeef · 2022-04-22T01:37:26Z

commitment/taro.go

+		return txscript.AssembleTaprootScriptTree(commitmentLeaf).
+			RootNode.TapHash()
+	}
+	// TODO: Expose an easy way to construct merkle proofs for this


👍

After needing to work around the tapscript structure to make uniqueness of the commitment possible, I think it might've been a mistake to enforce the ordering property here during hashing. You only need 1 bit of information to determine the order or the leaves, which is pretty trivial in terms of the amount of bytes added to the control block proof (a bit string of left vs right for each hashed element).

c'est la vie I guess

commitment/mint.go

commitment/proof.go

wpaulino · 2022-04-26T00:59:00Z

@Roasbeef most of the verification part of this diff has been moved to a new branch as I work through the rest of the Taro proof file format. I'll be opening another PR incorporating those changes later on.

lightninglabs-deploy · 2022-05-03T01:23:43Z

@wpaulino, remember to re-request review from reviewers when ready

Roasbeef · 2022-05-04T23:57:08Z

Has some linter failures w/ CI.

wpaulino · 2022-05-05T02:33:16Z

@Roasbeef ✅

commitment/mint.go

asset/asset.go

asset/encoding.go

wpaulino · 2022-05-10T18:47:40Z

This now depends on #29.

Roasbeef

Been using the contents of this PR in a local branch and haven't run into any fundamental issues w/ the API.

The one thing I realized along the way is that we shouldn't be using the raw schnorr keys as keys into any level of SMT, as the x-coordinate isn't uniformly random (you'd need to use things like elligator to achieve that). I've noted this at the relevant areas.

My only other main comment is surrounding what appears to be implicit non-inclusion proof behavior when asking for a proof of an asset that doesn't exist.

asset/asset.go

asset/encoding.go

commitment/asset.go

Roasbeef · 2022-05-11T03:49:55Z

commitment/asset.go

+	if c.FamilyKey == nil {
+		_, _ = h.Write(c.ID[:])
+	} else {
+		_, _ = h.Write(schnorr.SerializePubKey(&c.FamilyKey.Key))


Thinking about this a bit more, I think we should also apply an invocation of sha2 at this level, to retain the property that an individual can't 100% predict which location in the tree an entry will end up in. Otherwise it may be feasible for someone to attempt to target an x-coord that collides with a given assetID. By forcing things into the same domain, we're able to sidestep those concerns (and also still allow for NUMs points to be used here as well).

Safe to do this in another PR, as we'll need to update the BIP as well.

Went through them all and updated them in this PR. I'll keep this in mind for the other in-flight PRs, but we'll likely want to do a final sweep anyway.

Roasbeef · 2022-05-11T03:55:38Z

commitment/taro.go

+	var rootSum [8]byte
+	binary.BigEndian.PutUint64(rootSum[:], c.TreeRoot.NodeSum())
+	leafParts := [][]byte{
+		{byte(c.Version)}, TaroMarker[:], rootHash[:], rootSum[:],


Though the market byte also already enables this type of verification.

commitment/taro.go

commitment/mint.go

commitment/taro.go

commitment/commitment_test.go

A recent commit introduced a change to create new assets with a genesis witness by default, which consists of a non-nil PrevID of all zeros. `PrevID` contains a `btcec.PublicKey`, which is not a valid point when its X and Y coordinates are both zero, leading to a failure upon deserialization. Now, upon deserializing, we'll inspect the bytes to be read and skip parsing the `btcec.PublicKey` when it's all zeros.

We expose a new function to derive an asset's FamilyKey based on some internal private key and the asset's genesis.

wpaulino · 2022-05-11T20:30:58Z

commitment/asset.go

+	assetFamilyKey := assets[0].FamilyKey
+	assetsMap := make(map[[32]byte]*asset.Asset, len(assets))
+	for _, asset := range assets {
+		if assetFamilyKey != asset.FamilyKey {


Thinking this should also verify the sig is valid for the original asset genesis, though from this point it would be unclear which genesis was the one used to generate that sig.

Can instead be added a later step during sanity check before inclusion in an actual output?

Seems like it should go here since we're already doing similar types of validation. Perhaps worth opening an issue so it doesn't go unnoticed.

Roasbeef

LGTM 🥙

Roasbeef · 2022-05-12T01:26:33Z

commitment/asset.go

+}
+
+// NewAssetCommitment constructs a new commitment for the given assets capable
+// of computing merkle proofs. All assets provided should be related, i.e.,


I think we might eventually want to lift some of these restrictions to give better access to batched asset creation. Ultimately it should also be possible to just do this individually for the relevant set of assets, then merged them all into a single tree afterwards.

I'd need to circle back on some BIP stuff though, right now it's possible if you use a new output for each batch (since we use the output index to derive the asset ID). Alternatively, we'd just want to enforce some unique asset meta for each fo them, which also enables batching

Batching is still possible as is, you'd just instantiate multiple AssetCommitments for each set of related assets, and merge them together into a single Taro commitment tree with NewTaroCommitment. You are correct though in that with just the existing Mint function as is this is not possible.

Roasbeef · 2022-05-12T01:35:22Z

commitment/asset.go

+	assetFamilyKey := assets[0].FamilyKey
+	assetsMap := make(map[[32]byte]*asset.Asset, len(assets))
+	for _, asset := range assets {
+		if assetFamilyKey != asset.FamilyKey {


Can instead be added a later step during sanity check before inclusion in an actual output?

jharveyb

LGTM, just some nits.

commitment/asset.go

commitment/taro.go

jharveyb · 2022-05-12T05:45:13Z

commitment/mint.go

+
+	// Amount is the amount of assets that should be minted for `ScriptKey`.
+	// NOTE: This should be nil when minting `Collectible` assets.
+	Amount *uint64


In the BIP for the VM, we use an output value / MS-SMT sum value of 1 in the virtual transaction for verifying transfers of Collectibles.

My understanding is that default won't conflict with the value here?

Correct, that is happening at a different layer. If you follow mintAssets, you can can see how nil gets mapped to 1 unit per collectible asset.

commitment/commitment_test.go

AssetCommitment represents the inner MS-SMT within the Taro protocol committing to a set of assets under the same ID/family. Assets within this tree, which are leaves represented as the serialized asset TLV payload, are keyed by their `asset_script_key`. TaroCommitment represents the outer MS-SMT within the Taro protocol committing to a set of asset commitments. Asset commitments, which are leaves represented as `asset_version || asset_tree_root || asset_sum`, are keyed by their `asset_family_key` or `asset_id` otherwise.

…known rpc: turn UNKNOWN batch state into err, move mint related RPCs into mintrpc package

wpaulino requested a review from Roasbeef April 19, 2022 21:56

wpaulino force-pushed the commitment-mint branch 2 times, most recently from a7efc73 to 19b1621 Compare April 21, 2022 02:06

Roasbeef reviewed Apr 22, 2022

View reviewed changes

Roasbeef mentioned this pull request Apr 22, 2022

bip-taro: implement base asset issuance #19

Closed

wpaulino force-pushed the commitment-mint branch 2 times, most recently from 7a21d88 to 7847330 Compare April 26, 2022 00:57

wpaulino force-pushed the commitment-mint branch from 7847330 to 685d015 Compare May 4, 2022 22:55

wpaulino force-pushed the commitment-mint branch from 685d015 to 2baaca7 Compare May 5, 2022 02:29

wpaulino force-pushed the commitment-mint branch from 2baaca7 to d96c72b Compare May 5, 2022 20:19

blangel-nydig reviewed May 6, 2022

View reviewed changes

commitment/mint.go Outdated Show resolved Hide resolved

blangel-nydig reviewed May 6, 2022

View reviewed changes

asset/asset.go Show resolved Hide resolved

wpaulino force-pushed the commitment-mint branch from d96c72b to ee3f593 Compare May 6, 2022 16:45

wpaulino commented May 9, 2022

View reviewed changes

asset/encoding.go Show resolved Hide resolved

wpaulino mentioned this pull request May 9, 2022

vm: Implement Taro VM based on initial script version #27

Merged

wpaulino force-pushed the commitment-mint branch from 6b931f6 to 6530a98 Compare May 10, 2022 18:47

wpaulino requested a review from Roasbeef May 11, 2022 00:19

Roasbeef requested changes May 11, 2022

View reviewed changes

Roasbeef reviewed May 11, 2022

View reviewed changes

commitment/commitment_test.go Show resolved Hide resolved

wpaulino added 3 commits May 11, 2022 10:20

asset: Use correct records size hint in EncodeRecords

37e45d2

asset: Add FamilyKey derivation

68643d3

We expose a new function to derive an asset's FamilyKey based on some internal private key and the asset's genesis.

wpaulino added 2 commits May 11, 2022 13:25

asset: Hash asset FamilyKey whenever used as a MS-SMT key

9cbedd2

asset: Add Leaf method to Asset

4249f54

wpaulino force-pushed the commitment-mint branch from 6530a98 to cb0a3fa Compare May 11, 2022 20:27

wpaulino commented May 11, 2022

View reviewed changes

wpaulino requested a review from Roasbeef May 11, 2022 20:31

wpaulino force-pushed the commitment-mint branch 2 times, most recently from 54063a7 to afaa6b0 Compare May 11, 2022 23:53

Roasbeef mentioned this pull request May 12, 2022

asset+tlv: generate Taro NUMs point for uniformity in serialization code and bonus points #31

Closed

Roasbeef approved these changes May 12, 2022

View reviewed changes

Roasbeef requested a review from jharveyb May 12, 2022 01:35

Roasbeef mentioned this pull request May 12, 2022

commitment: enable heterogeneous batched asset creation. #32

Closed

jharveyb reviewed May 12, 2022

View reviewed changes

wpaulino added 2 commits May 12, 2022 10:34

commitment: Add support for minting new Taro commitments

adfcddf

wpaulino force-pushed the commitment-mint branch from afaa6b0 to 733ef44 Compare May 12, 2022 18:01

wpaulino added 2 commits May 12, 2022 11:07

commitment: Add Taro commitment proof for assets

46d45bb

commitment: Add initial test cases

8c7f16e

wpaulino force-pushed the commitment-mint branch from 733ef44 to 8c7f16e Compare May 12, 2022 18:08

Roasbeef mentioned this pull request May 12, 2022

assets: during asset creation check that the key family sig is correct #33

Closed

Roasbeef merged commit 2020c11 into main May 12, 2022

wpaulino deleted the commitment-mint branch May 13, 2022 00:07

Roasbeef pushed a commit to Roasbeef/taro that referenced this pull request Apr 6, 2023

Merge pull request lightninglabs#23 from lightninglabs/batch-state-un…

b32da38

…known rpc: turn UNKNOWN batch state into err, move mint related RPCs into mintrpc package

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

commitment: Add support for minting new Taro commitments #23

commitment: Add support for minting new Taro commitments #23

wpaulino commented Apr 19, 2022

wpaulino commented Apr 19, 2022

wpaulino commented Apr 21, 2022

Roasbeef commented Apr 22, 2022

Roasbeef Apr 22, 2022

Roasbeef May 11, 2022

Roasbeef Apr 22, 2022

wpaulino commented Apr 26, 2022

lightninglabs-deploy commented May 3, 2022

Roasbeef commented May 4, 2022

wpaulino commented May 5, 2022

wpaulino commented May 10, 2022

Roasbeef left a comment

Roasbeef May 11, 2022

wpaulino May 11, 2022

Roasbeef May 11, 2022

wpaulino May 11, 2022

Roasbeef May 12, 2022

wpaulino May 12, 2022

Roasbeef left a comment

Roasbeef May 12, 2022

wpaulino May 12, 2022

Roasbeef May 12, 2022

jharveyb left a comment

jharveyb May 12, 2022

wpaulino May 12, 2022

commitment: Add support for minting new Taro commitments #23

commitment: Add support for minting new Taro commitments #23

Conversation

wpaulino commented Apr 19, 2022

wpaulino commented Apr 19, 2022

wpaulino commented Apr 21, 2022

Roasbeef commented Apr 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wpaulino commented Apr 26, 2022

lightninglabs-deploy commented May 3, 2022

Roasbeef commented May 4, 2022

wpaulino commented May 5, 2022

wpaulino commented May 10, 2022

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jharveyb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment