Skip to content

minting: fix re-issuing into existing group with external key #1517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions itest/mint_fund_seal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,9 @@ func testMintExternalGroupKeyChantools(t *harnessTest) {
mintReq2 := CopyRequest(issuableAssets[0])
mintReq2.Asset.Name = "itestbuxx-money-printer-brrr-tranche-2"
mintReq2.Asset.ExternalGroupKey = externalGroupKey
mintReq2.Asset.GroupedAsset = true
mintReq2.Asset.NewGroupedAsset = false
mintReq2.Asset.GroupKey = batchAssets[0].AssetGroup.TweakedGroupKey

assetReqs2 := []*mintrpc.MintAssetRequest{mintReq2}

Expand Down
19 changes: 11 additions & 8 deletions rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,19 +443,19 @@ func (r *rpcServer) MintAsset(ctx context.Context,
// Using a specific group key or anchor implies disabling emission.
case req.Asset.NewGroupedAsset:
if specificGroupKey || specificGroupAnchor {
return nil, fmt.Errorf("must disable emission to " +
"specify a group")
return nil, fmt.Errorf("must not create new grouped " +
"asset to specify an existing group")
}

// A group tapscript root cannot be specified if emission is disabled.
case !req.Asset.NewGroupedAsset && groupTapscriptRootSize != 0:
return nil, fmt.Errorf("cannot specify a group tapscript root" +
"with emission disabled")
"when not creating a new grouped asset")

// A group internal key cannot be specified if emission is disabled.
case !req.Asset.NewGroupedAsset && specificGroupInternalKey:
return nil, fmt.Errorf("cannot specify a group internal key" +
"with emission disabled")
"when not creating a new grouped asset")

// If the asset is intended to be part of an existing group, a group key
// or anchor must be specified, but not both. Neither a group tapscript
Expand All @@ -473,12 +473,14 @@ func (r *rpcServer) MintAsset(ctx context.Context,

if groupTapscriptRootSize != 0 {
return nil, fmt.Errorf("cannot specify a group " +
"tapscript root with emission disabled")
"tapscript root when not creating a new " +
"grouped asset")
}

if specificGroupInternalKey {
return nil, fmt.Errorf("cannot specify a group " +
"internal key with emission disabled")
"internal key when not creating a new " +
"grouped asset")
}

// A group was specified without GroupedAsset being set.
Expand Down Expand Up @@ -608,8 +610,9 @@ func (r *rpcServer) MintAsset(ctx context.Context,
}

rpcsLog.Infof("[MintAsset]: version=%v, type=%v, name=%v, amt=%v, "+
"enable_emission=%v", seedling.AssetVersion, seedling.AssetType,
seedling.AssetName, seedling.Amount, seedling.EnableEmission)
"new_grouped_asset=%v", seedling.AssetVersion,
seedling.AssetType, seedling.AssetName, seedling.Amount,
seedling.EnableEmission)

if scriptKey != nil {
seedling.ScriptKey = *scriptKey
Expand Down
11 changes: 8 additions & 3 deletions tapgarden/planter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2668,14 +2668,19 @@ func (c *ChainPlanter) prepAssetSeedling(ctx context.Context,
// If a group internal key or tapscript root is specified, emission must

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you want to remove "emission" here too?

// also be enabled.
if !req.EnableEmission {
if req.GroupInternalKey != nil {
// For re-issuing grouped assets or regular (non-grouped)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this say "NEW regular (non-grouped)?

// assets, the group internal key shouldn't be set. It is,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// however, set for re-issuance with an external key, because

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it required or optional? I'm able to do a second tranche using external signer without setting it.

// the internal group key is the key we compare the external key
// against.
if req.GroupInternalKey != nil && req.ExternalKey.IsNone() {
return fmt.Errorf("cannot specify group internal key " +
"without enabling emission")
"without creating a new grouped asset")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should you add "first" on the end of this line?

}

if req.GroupTapscriptRoot != nil {
return fmt.Errorf("cannot specify group tapscript " +
"root without enabling emission")
"root without creating a new grouped asset")
}
}

Expand Down
47 changes: 43 additions & 4 deletions tapgarden/seedling.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,49 @@ func (c Seedling) validateFields() error {
func (c Seedling) validateGroupKey(group asset.AssetGroup,
anchorMeta *proof.MetaReveal) error {

// We must be able to sign with the group key.
if !group.GroupKey.IsLocal() {
groupKeyBytes := c.GroupInfo.GroupPubKey.SerializeCompressed()
return fmt.Errorf("can't sign with group key %x", groupKeyBytes)
switch {
// If we have an external key, we need to check that the group key
// matches the external key.
case c.ExternalKey.IsSome():
err := fn.MapOptionZ(
c.ExternalKey, func(extKey asset.ExternalKey) error {
if group.GroupKey == nil {
return fmt.Errorf("group key is nil")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the same as tweaked_group_key?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is just the outer struct holding the info, it doesn't have an RPC couterpart. This error should never happen, it's just defensive to avoid a panic.

}

if group.GroupKey.RawKey.PubKey == nil {
return fmt.Errorf("group raw key is " +

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the same as

/*
The external group key is an optional field that allows specifying an
external signing key for the group virtual transaction during minting.
This key enables signing operations to be performed externally, outside
the daemon.
If this field is set then group_internal_key must be unset, and vice versa.
*/
taprpc.ExternalKey external_group_key = 14;
?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is the pub_key in group_internal_key.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not understanding it, but may be outside of my knowledge scope, so maybe it doesn't need more clarification. not sure.

"nil")
}

pk, err := extKey.PubKey()
if err != nil {
return fmt.Errorf("error getting "+
"external key: %w", err)
}

if !pk.IsEqual(group.RawKey.PubKey) {
return fmt.Errorf("external key " +
"does not match group key")
}

return nil
},
)
if err != nil {
return fmt.Errorf("error validating external key: %w",
err)
}

// If it's not an external key, we need to check that we can actually

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this would mean

/*
The optional key that will be used as the internal key for an asset group
created with this asset.
If this field is set then external_group_key must be unset, and vice versa.
*/
taprpc.KeyDescriptor group_internal_key = 10;
?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily. You only need to specify the group_internal_key when you start a new group and want to tell tapd what internal key (the "raw" keypair from lnd) should be used for the group key construction.
When you mint into an existing group, then the internal key is already defined. So here we check that we can actually sign with that key (meaning the internal key defined in the initial mint is actually owned by the connected lnd node).

// sign with the group key.
default:
if !group.GroupKey.IsLocal() {
groupPubKey := c.GroupInfo.GroupPubKey
groupKeyBytes := groupPubKey.SerializeCompressed()
return fmt.Errorf("can't sign with group key %x",
groupKeyBytes)
}
}

// The seedling asset type must match the group asset type.
Expand Down
Loading