Part 1: Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support #1272

ffranr · 2025-01-03T23:19:32Z

This PR builds on Oli's WIP to demonstrate asset minting using the new V1 asset group key formulation. It integrates chantools to mint assets with an external asset group signing key (taproot internal key), offering a viable option for users who wish to keep their asset group signing key in cold storage.

A new integration test is included to simulate user behavior by leveraging chantools via the command line. The test verifies that the minter can successfully sign an asset into the asset group using the external asset group signing key.

Notes for Reviewers

Work which will be included in a future PR(s), ordered by most important first:

Tweaked NUMS key asset group key formulation (multi: add new v2 version of GKR based on Pedersen commitments #1290 (review))
Doc TODO (SealBatch RPC endpoint accepts signed group PSBT #1295)

TODO(guggero/ffranr): Replace this step by allowing the CLI to take the signed
PSBT instead. We can list the batch to get the asset ID of each pending asset
and match the PSBT's input previous output to find out what signed PSBT belongs
to what asset (in case there are multiple).

itest: ensure minter can spend asset minted using external key.
Unit tests for ExternalKey type and methods (draft: Add unit test for method ExternalKey.PubKey #1284)
Unit tests for method PendingAssetGroup.PSBT.
Add unit tests for CLI arg parsing before calling into RPC.

coveralls · 2025-01-08T15:46:45Z

Pull Request Test Coverage Report for Build 12808414882

Details

214 of 772 (27.72%) changed or added relevant lines in 11 files are covered.
20 unchanged lines in 7 files lost coverage.
Overall coverage decreased (-0.2%) to 40.723%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
proof/mint.go	7	11	63.64%
tapdb/assets_common.go	40	47	85.11%
cmd/tapcli/assets.go	0	49	0.0%
rpcserver.go	0	68	0.0%
tapgarden/planter.go	106	193	54.92%
itest/utils.go	0	112	0.0%
asset/asset.go	46	161	28.57%
itest/chantools_harness.go	0	116	0.0%

Files with Coverage Reduction	New Missed Lines	%
tapdb/addrs.go	2	75.12%
commitment/tap.go	2	84.32%
asset/asset.go	2	75.59%
tapgarden/planter.go	3	71.15%
rpcserver.go	3	0.0%
universe/interface.go	4	51.95%
tapgarden/caretaker.go	4	68.87%

Totals
Change from base Build 12791730946:	-0.2%
Covered Lines:	26528
Relevant Lines:	65142

💛 - Coveralls

ffranr · 2025-01-09T02:35:04Z

I think these are the necessary changes. This PR can't really be made smaller in any meaningful way.

ffranr · 2025-01-10T13:02:14Z

As mentioned during our last call, I’ve updated taproot-assets/tapdb/sqlc/migrations/000026_asset_group_version_customsubtree.down.sql to include the recreation of the previous version of key_group_info_view as part of the down migration. While we’re not actively using down migrations, it’s still beneficial to have this in place as a precaution.

taprpc/mintrpc/mint.proto

asset/asset.go

cmd/tapcli/assets.go

taprpc/mintrpc/mint.proto

tapgarden/planter.go

taprpc/mintrpc/mint.proto

tapdb/sqlc/migrations/000026_asset_group_version_customsubtree.up.sql

tapdb/assets_common.go

itest/mint_fund_seal_test.go

gijswijs

Great work. I have added some small comments throughout.

nit: the commit message of commit a993070 claims the commit populates the external key in the seedling instance, but I don't think that happens in that commit.

I would love to see a itest that tests the re-issuance of an asset with cold minting.

Would also like to see a unit test that tests both the V0 and the V1 flow re GroupTapscriptRoot and a custom tapscript root. Whatever approach we take regarding implicit or explicit flow @Roasbeef suggested, we will end up with two separate flows that are currently not fully covered by unit tests.

asset/asset.go

taprpc/mintrpc/mint.proto

asset/asset.go

tapdb/sqlc/queries/assets.sql

tapdb/sqlc/migrations/000026_asset_group_version_customsubtree.up.sql

Introduce a new method to support future usage in the tapgarden package.

Refactored the `FundBatch` method to use a general return type, `FundBatchResp`. This change improves code health and allows for easier extension by enabling the state machine to return additional fields in the future.

Updated the documentation for the `MintAsset.group_tapscript_root` field to clarify its purpose: it now represents the custom tapscript subtree root for V1 group key reveals and serves as the tapscript tree root for V0 group key reveals.

Add the `CustomTapscriptRoot` field to both the `GroupKey` and `GroupKeyRequest` types. This field stores the user-defined custom tapscript subtree root, which is committed to by the asset group key. Update the `buildGroupReqs` function to leverage this new field when constructing group key requests for generating asset group keys.

Introduce a new `GroupKeyVersion` type to represent the group key version in `asset.GroupKey` and `asset.GroupKeyRequest`. Add a `version` field to both `asset.GroupKey` and `asset.GroupKeyRequest`, with logic to populate it. Enhance the verification process by adding version-specific checks.

Update `GroupKeyRequest` methods to support the generation of version 1 (V1) group keys.

Add a new PSBT field to the `UnsealedAssets` RPC message type, which contains the byte-serialized PSBT equivalent of the group virtual transaction for unsealed assets. As a result, the `FundBatch` and `ListBatches` RPC endpoints now return group virtual PSBTs. Include logic to generate the group virtual PSBT. The PSBT is unsigned and is provided to allow signing with an external cold private key.

tapdb/sqlc/migrations/000026_asset_group_version_customsubtree.up.sql

GeorgeTsagk

LGTM, +tACK 🥇
great work!

Extended the `asset_groups` database table with two new columns: `version` and `custom_subtree_root_id`. These fields are required to store and retrieve new GroupKey data for use during mint proof generation, where the group key reveal is formulated. `custom_subtree_root_id` is a foreign key referencing the `root_id` in the `tapscript_roots` table. It links a custom tapscript subtree to the asset group, enabling an upgrade path towards storing user-defined asset group key scripts. This column can be `NULL` when no custom subtree is associated.

Write the group key version and custom subtree root to the database.

Read group key version and custom subtree root from the database.

Added support for group key reveal V1 alongside the existing group key reveal V0 when generating mint proofs.

Adds a test to ensure the new PSBT field introduced in the previous commit can be used to derive a transaction identical to the group virtual transaction.

Update the `build-itest` Makefile target, used as a subcommand during `make itest`, to ensure `chantools` is properly set up for integration tests. Additionally, update `.gitignore` to exclude the `chantools` build directory.

Add a test harness to execute the chantools binary via the command line and parse its output.

Add a new integration test, `testMintExternalGroupKeyChantools`, to validate minting two assets into the same asset group across two different batches. The test ensures asset group signatures are generated using chantools with an externally managed signing key for each batch.

Add externalKey and customTapscriptRoot arguments to the mock function NewGroupKeyRequestNoErr, enabling them to be specified during testing.

Add TestAssetGroupV1 to verify the ability to store and load an asset group of version 1.

ffranr self-assigned this Jan 3, 2025

dstadulis requested a review from GeorgeTsagk January 6, 2025 15:32

dstadulis added this to the v0.6 milestone Jan 6, 2025

dstadulis added wallet psbt group_key labels Jan 6, 2025

ffranr force-pushed the cold-group-key branch 3 times, most recently from c766bbe to cabfd1d Compare January 8, 2025 15:30

dstadulis mentioned this pull request Jan 9, 2025

docs: Clarify various aspects of external-group-key.md and add specificity #1282

Open

6 tasks

ffranr force-pushed the cold-group-key branch 2 times, most recently from 8af293f to 4666b23 Compare January 9, 2025 02:19

ffranr marked this pull request as ready for review January 9, 2025 02:19

ffranr requested a review from gijswijs January 9, 2025 02:19

ffranr force-pushed the cold-group-key branch 2 times, most recently from 1e543e2 to e61d638 Compare January 9, 2025 02:29

ffranr changed the title ~~Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support~~ Part 1: Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support Jan 9, 2025

This was referenced Jan 9, 2025

Add unit test for method ExternalKey.PubKey #1284

Merged

Add PSBT field to UnsealedAsset RPC message #1214

Closed

ffranr force-pushed the cold-group-key branch from e61d638 to c0c6795 Compare January 10, 2025 12:59

ffranr mentioned this pull request Jan 10, 2025

PoC: Use tweaked NUMS key for non-spendable Taproot script leaves #1289

Closed

Roasbeef requested changes Jan 12, 2025

View reviewed changes

gijswijs reviewed Jan 13, 2025

View reviewed changes

ffranr mentioned this pull request Jan 14, 2025

[feature]: add asset metadata to unsigned minting group virtual PSBT #1298

Open

ffranr added 3 commits January 14, 2025 16:12

multi: fix formatting and doc comments

b96c9ff

asset: add GroupKeyRevealTapscript.Root method

749ee1c

Introduce a new method to support future usage in the tapgarden package.

tapgarden: introduce FundBatchResp return type for FundBatch

aec207c

Refactored the `FundBatch` method to use a general return type, `FundBatchResp`. This change improves code health and allows for easier extension by enabling the state machine to return additional fields in the future.

ffranr and others added 5 commits January 15, 2025 21:31

asset: add support for V1 group keys in GroupKeyRequest methods

a2aa3e9

Update `GroupKeyRequest` methods to support the generation of version 1 (V1) group keys.

ffranr force-pushed the cold-group-key branch from c4a9222 to b801f9f Compare January 15, 2025 22:04

ffranr enabled auto-merge January 15, 2025 22:05

Roasbeef reviewed Jan 15, 2025

View reviewed changes

tapdb/sqlc/migrations/000026_asset_group_version_customsubtree.up.sql Outdated Show resolved Hide resolved

ffranr requested review from Roasbeef and removed request for Roasbeef January 16, 2025 00:29

ffranr disabled auto-merge January 16, 2025 00:47

GeorgeTsagk approved these changes Jan 16, 2025

View reviewed changes

ffranr and others added 10 commits January 16, 2025 10:56

tapdb: write new GroupKey fields to database rows

5d47774

Write the group key version and custom subtree root to the database.

tapdb: read new GroupKey fields from database rows

015fc04

Read group key version and custom subtree root from the database.

proof: support group key reveal V1 in mint proof generation

5532602

Added support for group key reveal V1 alongside the existing group key reveal V0 when generating mint proofs.

itest: verify group virtual transaction can be derived from PSBT

777cc5e

Adds a test to ensure the new PSBT field introduced in the previous commit can be used to derive a transaction identical to the group virtual transaction.

Makefile: enhance build-itest target for chantools setup

09e24f7

Update the `build-itest` Makefile target, used as a subcommand during `make itest`, to ensure `chantools` is properly set up for integration tests. Additionally, update `.gitignore` to exclude the `chantools` build directory.

itest: add harness for invoking chantools binary

e0dba5b

Add a test harness to execute the chantools binary via the command line and parse its output.

unittests+mock: add missing args to NewGroupKeyRequestNoErr

334a73b

Add externalKey and customTapscriptRoot arguments to the mock function NewGroupKeyRequestNoErr, enabling them to be specified during testing.

tapdb: add unit test for asset group version 1

5ca4168

Add TestAssetGroupV1 to verify the ability to store and load an asset group of version 1.

ffranr force-pushed the cold-group-key branch from b801f9f to 5ca4168 Compare January 16, 2025 12:01

ffranr requested a review from Roasbeef January 16, 2025 12:01

ffranr enabled auto-merge January 16, 2025 12:01

guggero disabled auto-merge January 16, 2025 12:28

guggero merged commit b15fc0a into main Jan 16, 2025
18 checks passed

guggero deleted the cold-group-key branch January 16, 2025 12:33

This was referenced Jan 16, 2025

[feature]: optionally generate asset group key without internal key tweak during minting #1226

Closed

[feature]: Enable asset group witness creation using external PSBT signer during minting #1207

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part 1: Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support #1272

Part 1: Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support #1272

ffranr commented Jan 3, 2025 •

edited

Loading

coveralls commented Jan 8, 2025 •

edited

Loading

ffranr commented Jan 9, 2025 •

edited

Loading

ffranr commented Jan 10, 2025

gijswijs left a comment

GeorgeTsagk left a comment

Part 1: Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support #1272

Part 1: Asset Minting with V1 Asset Group Key and Chantools Cold Storage Support #1272

Conversation

ffranr commented Jan 3, 2025 • edited Loading

Notes for Reviewers

coveralls commented Jan 8, 2025 • edited Loading

Pull Request Test Coverage Report for Build 12808414882

Details

💛 - Coveralls

ffranr commented Jan 9, 2025 • edited Loading

ffranr commented Jan 10, 2025

gijswijs left a comment

Choose a reason for hiding this comment

GeorgeTsagk left a comment

Choose a reason for hiding this comment

ffranr commented Jan 3, 2025 •

edited

Loading

coveralls commented Jan 8, 2025 •

edited

Loading

ffranr commented Jan 9, 2025 •

edited

Loading