From 36043713f69f5d79ea315879d354cddce74feabe Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:27 +0100 Subject: [PATCH 01/16] rpcserver: fix formatting --- rpcserver.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rpcserver.go b/rpcserver.go index ab402660e..da61bd1d9 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -3387,11 +3387,13 @@ func (r *rpcServer) BurnAsset(ctx context.Context, // We found the asset group, so we can use the group key to // burn the asset. groupKey = &assetGroup.GroupPubKey + case errors.Is(err, address.ErrAssetGroupUnknown): // We don't know the asset group, so we'll try to burn the // asset using the asset ID only. rpcsLog.Debug("Asset group key not found, asset may not be " + "part of a group") + case err != nil: return nil, fmt.Errorf("error querying asset group: %w", err) } From d97e02257fcff65172a2a861a1fce8ff40ec5bd6 Mon Sep 17 00:00:00 2001 From: ffranr Date: Mon, 17 Feb 2025 16:34:28 +0100 Subject: [PATCH 02/16] fn+tapchannel: condense code with fn.DerefPanic --- fn/memory.go | 28 ++++++++++++++++++++++++++++ tapchannel/aux_funding_controller.go | 9 ++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/fn/memory.go b/fn/memory.go index 0c71abc3a..3af0e82bc 100644 --- a/fn/memory.go +++ b/fn/memory.go @@ -1,5 +1,13 @@ package fn +import "errors" + +var ( + // ErrNilPointerDeference is returned when a nil pointer is + // dereferenced. + ErrNilPointerDeference = errors.New("nil pointer dereference") +) + // Ptr returns the pointer of the given value. This is useful in instances // where a function returns the value, but a pointer is wanted. Without this, // then an intermediate variable is needed. @@ -36,3 +44,23 @@ func CopySlice[T any](slice []T) []T { copy(newSlice, slice) return newSlice } + +// Deref safely dereferences a pointer. If the pointer is nil, it returns the +// zero value of type T and an error. +func Deref[T any](ptr *T) (T, error) { + if ptr == nil { + var zero T + return zero, ErrNilPointerDeference + } + + return *ptr, nil +} + +// DerefPanic dereferences a pointer. If the pointer is nil, it panics. +func DerefPanic[T any](ptr *T) T { + if ptr == nil { + panic(ErrNilPointerDeference) + } + + return *ptr +} diff --git a/tapchannel/aux_funding_controller.go b/tapchannel/aux_funding_controller.go index 38159f03f..6456cad12 100644 --- a/tapchannel/aux_funding_controller.go +++ b/tapchannel/aux_funding_controller.go @@ -2002,13 +2002,8 @@ func (f *FundingController) validateWitness(outAsset asset.Asset, // of proofs. But the chain lookup will need them to look up transaction // and block information in those proofs, so it's easiest to provide // them as a single file that can be iterated through. - proofFile, err := proof.NewFile( - proof.V0, fn.Map(inputAssetProofs, - func(p *proof.Proof) proof.Proof { - return *p - }, - )..., - ) + derefProofs := fn.Map(inputAssetProofs, fn.DerefPanic) + proofFile, err := proof.NewFile(proof.V0, derefProofs...) if err != nil { return fmt.Errorf("unable to create proof file: %w", err) } From 483ca888795c04bd343fb81ce2e0079b5c3b3f7a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:30 +0100 Subject: [PATCH 03/16] tappsbt: fix copy on nil asset This commit avoids a nil pointer panic when a vPacket is copied that doesn't have an input asset set yet. --- tappsbt/interface.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tappsbt/interface.go b/tappsbt/interface.go index 5e988d3d4..579a06abc 100644 --- a/tappsbt/interface.go +++ b/tappsbt/interface.go @@ -407,11 +407,16 @@ type VInput struct { // Copy creates a deep copy of the VInput. func (i *VInput) Copy() *VInput { + var copiedAsset *asset.Asset + if i.asset != nil { + copiedAsset = i.asset.Copy() + } + return &VInput{ PInput: i.PInput, PrevID: i.PrevID, Anchor: i.Anchor, - asset: i.asset.Copy(), + asset: copiedAsset, // We never expect the individual fields of the proof to change // while it is assigned to a virtual input. So not deep copying // it here is fine. From 29d095eeb9c65c9bccafe36163e76fe1f0272b9a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:32 +0100 Subject: [PATCH 04/16] fn: don't copy nil slice --- fn/memory.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fn/memory.go b/fn/memory.go index 3af0e82bc..4c56497f3 100644 --- a/fn/memory.go +++ b/fn/memory.go @@ -40,6 +40,10 @@ func ToArray[T ByteArray](v []byte) T { // CopySlice returns a copy of the given slice. Does a shallow copy of the // slice itself, not the underlying elements. func CopySlice[T any](slice []T) []T { + if slice == nil { + return nil + } + newSlice := make([]T, len(slice)) copy(newSlice, slice) return newSlice From 427f9e10d38489a0d9315fd01bb11b0cd8e7ba1d Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:33 +0100 Subject: [PATCH 05/16] tapchannel: don't set inputs on template During funding, the input slice of the virtual packet template is going to be overwritten anyway, so we can leave that unset. This will also help in the next commit where we don't necessarily have access to an asset ID anymore. --- tapchannel/aux_funding_controller.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tapchannel/aux_funding_controller.go b/tapchannel/aux_funding_controller.go index 6456cad12..ad8b58626 100644 --- a/tapchannel/aux_funding_controller.go +++ b/tapchannel/aux_funding_controller.go @@ -785,11 +785,6 @@ func (f *FundingController) fundVirtualPacket(ctx context.Context, // case our destination will just be the OP_TRUE tapscript that we use // for the funding output. pktTemplate := &tappsbt.VPacket{ - Inputs: []*tappsbt.VInput{{ - PrevID: asset.PrevID{ - ID: assetID, - }, - }}, Outputs: []*tappsbt.VOutput{{ Amount: amt, AssetVersion: asset.V1, From 6b0fe16a920375702a43fd2e6d7212a01f5850d2 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:34 +0100 Subject: [PATCH 06/16] rpcserver+tapchannel: add asset specifier to funding desc --- rpcserver.go | 104 ++++++++++++++++++++------- tapchannel/aux_funding_controller.go | 24 +++---- 2 files changed, 90 insertions(+), 38 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index da61bd1d9..d27c05df3 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -6429,54 +6429,71 @@ func MarshalAssetFedSyncCfg( } // unmarshalAssetSpecifier unmarshals an asset specifier from the RPC form. -func unmarshalAssetSpecifier(req *rfqrpc.AssetSpecifier) (*asset.ID, +func unmarshalAssetSpecifier(s *rfqrpc.AssetSpecifier) (*asset.ID, *btcec.PublicKey, error) { + if s == nil { + return nil, nil, fmt.Errorf("asset specifier must be specified") + } + + return parseAssetSpecifier( + s.GetAssetId(), s.GetAssetIdStr(), s.GetGroupKey(), + s.GetGroupKeyStr(), + ) +} + +// parseAssetSpecifier parses an asset specifier from the RPC form. +func parseAssetSpecifier(reqAssetID []byte, reqAssetIDStr string, + reqGroupKey []byte, reqGroupKeyStr string) (*asset.ID, *btcec.PublicKey, + error) { + // Attempt to decode the asset specifier from the RPC request. In cases // where both the asset ID and asset group key are provided, we will // give precedence to the asset ID due to its higher level of // specificity. var ( - assetID *asset.ID - - groupKeyBytes []byte - groupKey *btcec.PublicKey - - err error + assetID *asset.ID + groupKey *btcec.PublicKey + err error ) switch { // Parse the asset ID if it's set. - case len(req.GetAssetId()) > 0: + case len(reqAssetID) > 0: + if len(reqAssetID) != sha256.Size { + return nil, nil, fmt.Errorf("asset ID must be 32 bytes") + } + var assetIdBytes [32]byte - copy(assetIdBytes[:], req.GetAssetId()) + copy(assetIdBytes[:], reqAssetID) id := asset.ID(assetIdBytes) assetID = &id - case len(req.GetAssetIdStr()) > 0: - assetIDBytes, err := hex.DecodeString(req.GetAssetIdStr()) + case len(reqAssetIDStr) > 0: + assetIDBytes, err := hex.DecodeString(reqAssetIDStr) if err != nil { return nil, nil, fmt.Errorf("error decoding asset "+ "ID: %w", err) } + if len(assetIDBytes) != sha256.Size { + return nil, nil, fmt.Errorf("asset ID must be 32 bytes") + } + var id asset.ID copy(id[:], assetIDBytes) assetID = &id // Parse the group key if it's set. - case len(req.GetGroupKey()) > 0: - groupKeyBytes = req.GetGroupKey() - groupKey, err = btcec.ParsePubKey(groupKeyBytes) + case len(reqGroupKey) > 0: + groupKey, err = btcec.ParsePubKey(reqGroupKey) if err != nil { return nil, nil, fmt.Errorf("error parsing group "+ "key: %w", err) } - case len(req.GetGroupKeyStr()) > 0: - groupKeyBytes, err := hex.DecodeString( - req.GetGroupKeyStr(), - ) + case len(reqGroupKeyStr) > 0: + groupKeyBytes, err := hex.DecodeString(reqGroupKeyStr) if err != nil { return nil, nil, fmt.Errorf("error decoding group "+ "key: %w", err) @@ -7057,8 +7074,21 @@ func (r *rpcServer) FundChannel(ctx context.Context, return nil, fmt.Errorf("error parsing peer pubkey: %w", err) } - if len(req.AssetId) != sha256.Size { - return nil, fmt.Errorf("asset ID must be 32 bytes") + assetID, groupKey, err := parseAssetSpecifier( + req.GetAssetId(), "", nil, "", + ) + if err != nil { + return nil, fmt.Errorf("error parsing asset specifier: %w", err) + } + + // For channel funding, we need to make sure that the group key is set + // if the asset is grouped. + assetSpecifier, err := r.specifierWithGroupKeyLookup( + ctx, assetID, groupKey, + ) + if err != nil { + return nil, fmt.Errorf("error creating asset specifier: %w", + err) } if req.AssetAmount == 0 { @@ -7069,12 +7099,12 @@ func (r *rpcServer) FundChannel(ctx context.Context, } fundReq := tapchannel.FundReq{ - PeerPub: *peerPub, - AssetAmount: req.AssetAmount, - FeeRate: chainfee.SatPerVByte(req.FeeRateSatPerVbyte), - PushAmount: btcutil.Amount(req.PushSat), + PeerPub: *peerPub, + AssetSpecifier: assetSpecifier, + AssetAmount: req.AssetAmount, + FeeRate: chainfee.SatPerVByte(req.FeeRateSatPerVbyte), + PushAmount: btcutil.Amount(req.PushSat), } - copy(fundReq.AssetID[:], req.AssetId) chanPoint, err := r.cfg.AuxFundingController.FundChannel(ctx, fundReq) if err != nil { @@ -7087,6 +7117,30 @@ func (r *rpcServer) FundChannel(ctx context.Context, }, nil } +// specifierWithGroupKeyLookup returns an asset specifier that has the group key +// set if it's a grouped asset. +func (r *rpcServer) specifierWithGroupKeyLookup(ctx context.Context, + assetID *asset.ID, groupKey *btcec.PublicKey) (asset.Specifier, error) { + + var result asset.Specifier + + if assetID != nil && groupKey == nil { + dbGroupKey, err := r.cfg.TapAddrBook.QueryAssetGroup( + ctx, *assetID, + ) + switch { + case err == nil && dbGroupKey.GroupKey != nil: + groupKey = &dbGroupKey.GroupPubKey + + case err != nil: + return result, fmt.Errorf("unable to query asset "+ + "group: %w", err) + } + } + + return asset.NewSpecifier(assetID, groupKey, nil, true) +} + // EncodeCustomRecords allows RPC users to encode Taproot Asset channel related // data into the TLV format that is used in the custom records of the lnd // payment or other channel related RPCs. This RPC is completely stateless and diff --git a/tapchannel/aux_funding_controller.go b/tapchannel/aux_funding_controller.go index ad8b58626..a37d08ff9 100644 --- a/tapchannel/aux_funding_controller.go +++ b/tapchannel/aux_funding_controller.go @@ -751,10 +751,11 @@ func (f *fundingFlowIndex) fromMsg(chainParams *address.ChainParams, // fundVirtualPacket attempts to fund a new vPacket using the asset wallet to // find the asset inputs required to satisfy a funding request. func (f *FundingController) fundVirtualPacket(ctx context.Context, - assetID asset.ID, amt uint64) (*tapfreighter.FundedVPacket, error) { + specifier asset.Specifier, amt uint64) (*tapfreighter.FundedVPacket, + error) { - log.Infof("Funding new vPacket channel, asset_id=%v, amt=%v", - assetID, amt) + log.Infof("Funding new vPacket channel, asset=%s, amt=%v", &specifier, + amt) // Our funding script key will be the OP_TRUE addr that we'll use as // the funding script on the asset level. @@ -795,16 +796,14 @@ func (f *FundingController) fundVirtualPacket(ctx context.Context, ChainParams: &f.cfg.ChainParams, Version: tappsbt.V1, } - fundDesc, err := tapsend.DescribeRecipients( - ctx, pktTemplate, f.cfg.GroupKeyIndex, - ) - if err != nil { - return nil, fmt.Errorf("unable to describe recipients: %w", err) + fundDesc := &tapsend.FundingDescriptor{ + AssetSpecifier: specifier, + Amount: amt, + CoinSelectType: tapsend.Bip86Only, } // Fund the packet. This will derive an anchor internal key for us, but // we'll overwrite that later on. - fundDesc.CoinSelectType = tapsend.Bip86Only return f.cfg.AssetWallet.FundPacket(ctx, fundDesc, pktTemplate) } @@ -1544,8 +1543,7 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex, // With our initial state created, we'll now attempt to fund the // channel on the TAP level with a vPacket. fundingVpkt, err := f.fundVirtualPacket( - fundReq.ctx, fundReq.AssetID, - fundReq.AssetAmount, + fundReq.ctx, fundReq.AssetSpecifier, fundReq.AssetAmount, ) if err != nil { return fmt.Errorf("unable to fund vPacket: %w", err) @@ -2028,8 +2026,8 @@ type FundReq struct { // TODO(roasbeef): also need p2p address? PeerPub btcec.PublicKey - // AssetID is the asset that we're funding the channel with. - AssetID asset.ID + // AssetSpecifier is the asset that we're funding the channel with. + AssetSpecifier asset.Specifier // AssetAmount is the amount of the asset that we're funding the channel // with. From 65ccbf2ca42f8c29c97d752acc9a72b9346e2319 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:35 +0100 Subject: [PATCH 07/16] tapfreighter+tapsend: don't use FundingDescriptor We don't need the full funding descriptor, so this refactor makes it easier to use ValidateInputs and AssetFromTapCommitment in other situations. --- tapfreighter/wallet.go | 3 ++- tapsend/send.go | 24 +++++++++++++----------- tapsend/send_test.go | 37 ++++++++++++++++++++++++------------- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/tapfreighter/wallet.go b/tapfreighter/wallet.go index 7c8a754d4..1cc13359e 100644 --- a/tapfreighter/wallet.go +++ b/tapfreighter/wallet.go @@ -666,7 +666,8 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, } fullValue, err := tapsend.ValidateInputs( - inputCommitments, assetType, fundDesc, + inputCommitments, assetType, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, err diff --git a/tapsend/send.go b/tapsend/send.go index a0a42083c..8ed294303 100644 --- a/tapsend/send.go +++ b/tapsend/send.go @@ -280,26 +280,27 @@ func DescribeAddrs(addrs []*address.Tap) (*FundingDescriptor, error) { // AssetFromTapCommitment uses a script key to extract an asset from a given // Taproot Asset commitment. func AssetFromTapCommitment(tapCommitment *commitment.TapCommitment, - desc *FundingDescriptor, inputScriptKey btcec.PublicKey) (*asset.Asset, - error) { + specifier asset.Specifier, + inputScriptKey btcec.PublicKey) (*asset.Asset, error) { // The top-level Taproot Asset tree must have a non-empty asset tree at // the leaf specified by the funding descriptor's asset (group) specific // commitment locator. + tapKey := asset.TapCommitmentKey(specifier) assetCommitments := tapCommitment.Commitments() - assetCommitment, ok := assetCommitments[desc.TapCommitmentKey()] + assetCommitment, ok := assetCommitments[tapKey] if !ok { return nil, fmt.Errorf("input commitment does "+ - "not contain asset_id=%x: %w", desc.TapCommitmentKey(), + "not contain asset=%s: %w", &specifier, ErrMissingInputAsset) } // Determine whether issuance is disabled for the asset. - issuanceDisabled := !desc.AssetSpecifier.HasGroupPubKey() + issuanceDisabled := !specifier.HasGroupPubKey() - assetId, err := desc.AssetSpecifier.UnwrapIdOrErr() + assetId, err := specifier.UnwrapIdOrErr() if err != nil { - return nil, err + return nil, fmt.Errorf("asset from tap commitment: %w", err) } // The asset tree must have a non-empty Asset at the location @@ -321,7 +322,8 @@ func AssetFromTapCommitment(tapCommitment *commitment.TapCommitment, // ValidateInputs validates a set of inputs against a funding request. It // returns true if the inputs would be spent fully, otherwise false. func ValidateInputs(inputCommitments tappsbt.InputCommitments, - expectedAssetType asset.Type, desc *FundingDescriptor) (bool, error) { + expectedAssetType asset.Type, specifier asset.Specifier, + outputAmount uint64) (bool, error) { // Extract the input assets from the input commitments. inputAssets := make([]*asset.Asset, 0, len(inputCommitments)) @@ -336,7 +338,7 @@ func ValidateInputs(inputCommitments tappsbt.InputCommitments, // Gain the asset that we'll use as an input and in the process // validate the selected input and commitment. inputAsset, err := AssetFromTapCommitment( - tapCommitment, desc, *senderScriptKey, + tapCommitment, specifier, *senderScriptKey, ) if err != nil { return false, err @@ -363,12 +365,12 @@ func ValidateInputs(inputCommitments tappsbt.InputCommitments, // Ensure that the input assets are sufficient to cover the amount // being sent. - if totalInputsAmount < desc.Amount { + if totalInputsAmount < outputAmount { return false, ErrInsufficientInputAssets } // Check if the input assets are fully spent. - isFullValueSpend = totalInputsAmount == desc.Amount + isFullValueSpend = totalInputsAmount == outputAmount case asset.Collectible: isFullValueSpend = true diff --git a/tapsend/send_test.go b/tapsend/send_test.go index 30f576a9d..bbcaa4014 100644 --- a/tapsend/send_test.go +++ b/tapsend/send_test.go @@ -1918,7 +1918,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fundDesc := addrToFundDesc(state.address1) inputAsset, err := tapsend.AssetFromTapCommitment( - &state.asset1TapTree, fundDesc, state.spenderScriptKey, + &state.asset1TapTree, fundDesc.AssetSpecifier, + state.spenderScriptKey, ) if err != nil { return nil, nil, err @@ -1927,7 +1928,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fullValue, err := tapsend.ValidateInputs( tappsbt.InputCommitments{ state.asset1PrevID: &state.asset1TapTree, - }, inputAsset.Type, fundDesc, + }, inputAsset.Type, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, nil, err @@ -1944,8 +1946,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fundDesc := addrToFundDesc(state.address1CollectGroup) inputAsset, err := tapsend.AssetFromTapCommitment( - &state.asset1CollectGroupTapTree, fundDesc, - state.spenderScriptKey, + &state.asset1CollectGroupTapTree, + fundDesc.AssetSpecifier, state.spenderScriptKey, ) if err != nil { return nil, nil, err @@ -1955,7 +1957,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fullValue, err := tapsend.ValidateInputs( tappsbt.InputCommitments{ state.asset1CollectGroupPrevID: inputCommitment, - }, inputAsset.Type, fundDesc, + }, inputAsset.Type, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, nil, err @@ -1972,7 +1975,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fundDesc := addrToFundDesc(state.address1) inputAsset, err := tapsend.AssetFromTapCommitment( - &state.asset2TapTree, fundDesc, state.spenderScriptKey, + &state.asset2TapTree, fundDesc.AssetSpecifier, + state.spenderScriptKey, ) if err != nil { return nil, nil, err @@ -1981,7 +1985,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fullValue, err := tapsend.ValidateInputs( tappsbt.InputCommitments{ state.asset2PrevID: &state.asset2TapTree, - }, inputAsset.Type, fundDesc, + }, inputAsset.Type, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, nil, err @@ -1998,7 +2003,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fundDesc := addrToFundDesc(state.address2) inputAsset, err := tapsend.AssetFromTapCommitment( - &state.asset1TapTree, fundDesc, state.spenderScriptKey, + &state.asset1TapTree, fundDesc.AssetSpecifier, + state.spenderScriptKey, ) if err != nil { return nil, nil, err @@ -2007,7 +2013,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fullValue, err := tapsend.ValidateInputs( tappsbt.InputCommitments{ state.asset1PrevID: &state.asset1TapTree, - }, inputAsset.Type, fundDesc, + }, inputAsset.Type, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, nil, err @@ -2024,7 +2031,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fundDesc := addrToFundDesc(state.address1CollectGroup) inputAsset, err := tapsend.AssetFromTapCommitment( - &state.asset1TapTree, fundDesc, state.spenderScriptKey, + &state.asset1TapTree, fundDesc.AssetSpecifier, + state.spenderScriptKey, ) if err != nil { return nil, nil, err @@ -2033,7 +2041,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fullValue, err := tapsend.ValidateInputs( tappsbt.InputCommitments{ state.asset1PrevID: &state.asset1TapTree, - }, inputAsset.Type, fundDesc, + }, inputAsset.Type, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, nil, err @@ -2059,7 +2068,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fundDesc := addrToFundDesc(*address1testnet) inputAsset, err := tapsend.AssetFromTapCommitment( - &state.asset1TapTree, fundDesc, state.receiverPubKey, + &state.asset1TapTree, fundDesc.AssetSpecifier, + state.receiverPubKey, ) if err != nil { return nil, nil, err @@ -2068,7 +2078,8 @@ var addressValidInputTestCases = []addressValidInputTestCase{{ fullValue, err := tapsend.ValidateInputs( tappsbt.InputCommitments{ state.asset1PrevID: &state.asset1TapTree, - }, inputAsset.Type, fundDesc, + }, inputAsset.Type, fundDesc.AssetSpecifier, + fundDesc.Amount, ) if err != nil { return nil, nil, err From d7615032986e06deb0a5d0660306ee8b9ca46e1a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:37 +0100 Subject: [PATCH 08/16] proof+tapfreighter: move ProofExporter to proof.Exporter --- proof/archive.go | 11 ++++++++--- tapfreighter/chain_porter.go | 10 ++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/proof/archive.go b/proof/archive.go index 40c122da4..b8a452c73 100644 --- a/proof/archive.go +++ b/proof/archive.go @@ -133,9 +133,8 @@ type AnnotatedProof struct { *AssetSnapshot } -// Archiver is the main storage backend the ProofArchiver uses to store and -// query for proof files. -type Archiver interface { +// Exporter is used to fetch proofs by their unique identifier. +type Exporter interface { // FetchProof fetches a proof for an asset uniquely identified by the // passed ProofIdentifier. // @@ -144,6 +143,12 @@ type Archiver interface { // locator then ErrMultipleProofs should be returned to indicate more // specific fields need to be set in the Locator (e.g. the OutPoint). FetchProof(ctx context.Context, id Locator) (Blob, error) +} + +// Archiver is the main storage backend the ProofArchiver uses to store and +// query for proof files. +type Archiver interface { + Exporter // FetchIssuanceProof fetches the issuance proof for an asset, given the // anchor point of the issuance (NOT the genesis point for the asset). diff --git a/tapfreighter/chain_porter.go b/tapfreighter/chain_porter.go index 3b6190ce8..4803d11ce 100644 --- a/tapfreighter/chain_porter.go +++ b/tapfreighter/chain_porter.go @@ -40,13 +40,6 @@ type ProofImporter interface { replace bool, proofs ...*proof.AnnotatedProof) error } -// ProofExporter is used to fetch input proofs to -type ProofExporter interface { - // FetchProof attempts to fetcb a serialized proof file from the local - // archive based on the passed locator. - FetchProof(ctx context.Context, id proof.Locator) (proof.Blob, error) -} - // ChainPorterConfig is the main config for the chain porter. type ChainPorterConfig struct { // Signer implements the Taproot Asset level signing we need to sign a @@ -80,7 +73,8 @@ type ChainPorterConfig struct { ProofWriter ProofImporter - ProofReader ProofExporter + // ProofReader is used to fetch input proofs. + ProofReader proof.Exporter // ProofCourierDispatcher is the dispatcher that is used to create new // proof courier handles for sending proofs based on the protocol of From d1a6ee3b612d985b79bc29a0e898fdcfedceacaa Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:38 +0100 Subject: [PATCH 09/16] multi: refactor funding packet creation into functions As a preparation for a larger refactor on how we create virtual packets and distribute the coins selected for funding a transfer, we turn previous methods into functions. This will allow us to move those functions and easier unit test them. --- tapcfg/server.go | 32 ++++++------ tapfreighter/chain_porter.go | 1 + tapfreighter/wallet.go | 99 +++++++++++++++++++++++------------- 3 files changed, 81 insertions(+), 51 deletions(-) diff --git a/tapcfg/server.go b/tapcfg/server.go index a91d51a75..c149dd550 100644 --- a/tapcfg/server.go +++ b/tapcfg/server.go @@ -556,22 +556,22 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, ProofUpdates: proofArchive, ErrChan: mainErrChan, }), - AssetCustodian: tapgarden.NewCustodian( - &tapgarden.CustodianConfig{ - ChainParams: &tapChainParams, - WalletAnchor: walletAnchor, - ChainBridge: chainBridge, - GroupVerifier: tapgarden.GenGroupVerifier( - context.Background(), assetMintingStore, - ), - AddrBook: addrBook, - ProofArchive: proofArchive, - ProofNotifier: multiNotifier, - ErrChan: mainErrChan, - ProofCourierDispatcher: proofCourierDispatcher, - ProofRetrievalDelay: cfg.CustodianProofRetrievalDelay, ProofWatcher: reOrgWatcher, - }, - ), + // nolint: lll + AssetCustodian: tapgarden.NewCustodian(&tapgarden.CustodianConfig{ + ChainParams: &tapChainParams, + WalletAnchor: walletAnchor, + ChainBridge: chainBridge, + GroupVerifier: tapgarden.GenGroupVerifier( + context.Background(), assetMintingStore, + ), + AddrBook: addrBook, + ProofArchive: proofArchive, + ProofNotifier: multiNotifier, + ErrChan: mainErrChan, + ProofCourierDispatcher: proofCourierDispatcher, + ProofRetrievalDelay: cfg.CustodianProofRetrievalDelay, + ProofWatcher: reOrgWatcher, + }), ChainBridge: chainBridge, AddrBook: addrBook, AddrBookDisableSyncer: cfg.AddrBook.DisableSyncer, diff --git a/tapfreighter/chain_porter.go b/tapfreighter/chain_porter.go index 4803d11ce..9dce8c5ed 100644 --- a/tapfreighter/chain_porter.go +++ b/tapfreighter/chain_porter.go @@ -1228,6 +1228,7 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) { log.Debugf("Signing %d passive assets", len(currentPkg.PassiveAssets)) + err = wallet.SignPassiveAssets(currentPkg.PassiveAssets) if err != nil { p.unlockInputs(ctx, ¤tPkg) diff --git a/tapfreighter/wallet.go b/tapfreighter/wallet.go index 1cc13359e..7859aff06 100644 --- a/tapfreighter/wallet.go +++ b/tapfreighter/wallet.go @@ -274,12 +274,22 @@ func (f *AssetWallet) FundAddressSend(ctx context.Context, } // createPassivePacket creates a virtual packet for the given passive asset. -func createPassivePacket(params *address.ChainParams, passiveAsset *asset.Asset, +func createPassivePacket(passiveAsset *asset.Asset, activePackets []*tappsbt.VPacket, anchorOutputIndex uint32, anchorOutputInternalKey keychain.KeyDescriptor, prevOut wire.OutPoint, inputProof *proof.Proof, inputAltLeaves []*asset.Asset) (*tappsbt.VPacket, error) { + if len(activePackets) == 0 { + return nil, errors.New("no active packets provided") + } + + if activePackets[0].ChainParams == nil { + return nil, errors.New("chain params not set in active packet") + } + + params := activePackets[0].ChainParams + // Specify virtual input. inputAsset := passiveAsset.Copy() vInput := tappsbt.VInput{ @@ -443,8 +453,9 @@ func (f *AssetWallet) FundPacket(ctx context.Context, } }() - pkt, err := f.fundPacketWithInputs( - ctx, fundDesc, vPkt, selectedCommitments, + pkt, err := fundPacketWithInputs( + ctx, f.cfg.AssetProofs, f.cfg.KeyRing, f.cfg.AddrBook, fundDesc, + vPkt, selectedCommitments, ) if err != nil { return nil, err @@ -555,8 +566,9 @@ func (f *AssetWallet) FundBurn(ctx context.Context, // The virtual transaction is now ready to be further enriched with the // split commitment and other data. - fundedPkt, err := f.fundPacketWithInputs( - ctx, fundDesc, vPkt, selectedCommitments, + fundedPkt, err := fundPacketWithInputs( + ctx, f.cfg.AssetProofs, f.cfg.KeyRing, f.cfg.AddrBook, fundDesc, + vPkt, selectedCommitments, ) if err != nil { return nil, err @@ -576,7 +588,7 @@ func (f *AssetWallet) FundBurn(ctx context.Context, // output as tappsbt.TypePassiveSplitRoot. If that's not the // case, we'll return as burning all assets in an anchor output // is not supported. - otherAssets, err := f.hasOtherAssets( + otherAssets, err := hasOtherAssets( fundedPkt.InputCommitments, []*tappsbt.VPacket{vPkt}, ) if err != nil { @@ -596,7 +608,7 @@ func (f *AssetWallet) FundBurn(ctx context.Context, // hasOtherAssets returns true if the given input commitments contain any other // assets than the ones given in the virtual packets. -func (f *AssetWallet) hasOtherAssets(inputCommitments tappsbt.InputCommitments, +func hasOtherAssets(inputCommitments tappsbt.InputCommitments, vPackets []*tappsbt.VPacket) (bool, error) { for idx := range inputCommitments { @@ -629,10 +641,15 @@ func (f *AssetWallet) hasOtherAssets(inputCommitments tappsbt.InputCommitments, } // fundPacketWithInputs funds a virtual transaction with the given inputs. -func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, - fundDesc *tapsend.FundingDescriptor, vPkt *tappsbt.VPacket, +func fundPacketWithInputs(ctx context.Context, exporter proof.Exporter, + keyRing KeyRing, addrBook AddrBook, fundDesc *tapsend.FundingDescriptor, + vPkt *tappsbt.VPacket, selectedCommitments []*AnchoredCommitment) (*FundedVPacket, error) { + if vPkt.ChainParams == nil { + return nil, errors.New("chain params not set in virtual packet") + } + assetId, err := fundDesc.AssetSpecifier.UnwrapIdOrErr() if err != nil { return nil, err @@ -658,8 +675,8 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, totalInputAmt += anchorAsset.Asset.Amount } - inputCommitments, err := f.setVPacketInputs( - ctx, selectedCommitments, vPkt, + inputCommitments, err := setVPacketInputs( + ctx, exporter, selectedCommitments, vPkt, ) if err != nil { return nil, err @@ -681,7 +698,7 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, for idx := range vPkt.Outputs { vOut := vPkt.Outputs[idx] - tweakedKey, err := f.cfg.AddrBook.FetchScriptKey( + tweakedKey, err := addrBook.FetchScriptKey( ctx, vOut.ScriptKey.PubKey, ) switch { @@ -737,7 +754,7 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, "key is spendable: %w", err) } if unSpendable && !fullValue { - changeScriptKey, err := f.cfg.KeyRing.DeriveNextKey( + changeScriptKey, err := keyRing.DeriveNextKey( ctx, asset.TaprootAssetsKeyFamily, ) if err != nil { @@ -786,14 +803,14 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, continue } - newInternalKey, err := f.cfg.KeyRing.DeriveNextKey( + newInternalKey, err := keyRing.DeriveNextKey( ctx, asset.TaprootAssetsKeyFamily, ) if err != nil { return nil, err } vOut.SetAnchorInternalKey( - newInternalKey, f.cfg.ChainParams.HDCoinType, + newInternalKey, vPkt.ChainParams.HDCoinType, ) } @@ -809,7 +826,7 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context, // setVPacketInputs sets the inputs of the given vPkt to the given send eligible // commitments. It also returns the assets that were used as inputs. -func (f *AssetWallet) setVPacketInputs(ctx context.Context, +func setVPacketInputs(ctx context.Context, exporter proof.Exporter, eligibleCommitments []*AnchoredCommitment, vPkt *tappsbt.VPacket) (tappsbt.InputCommitments, error) { @@ -829,8 +846,8 @@ func (f *AssetWallet) setVPacketInputs(ctx context.Context, // We'll also include an inclusion proof for the input asset in // the virtual transaction. With that a signer can verify that // the asset was actually committed to in the anchor output. - inputProof, err := f.fetchInputProof( - ctx, assetInput.Asset, assetInput.AnchorPoint, + inputProof, err := fetchInputProof( + ctx, exporter, assetInput.Asset, assetInput.AnchorPoint, ) if err != nil { return nil, fmt.Errorf("error fetching input proof: %w", @@ -840,7 +857,7 @@ func (f *AssetWallet) setVPacketInputs(ctx context.Context, // Create the virtual packet input including the chain anchor // information. err = createAndSetInput( - vPkt, idx, f.cfg.ChainParams, assetInput, inputProof, + vPkt, idx, assetInput, inputProof, ) if err != nil { return nil, fmt.Errorf("unable to create and set "+ @@ -857,12 +874,11 @@ func (f *AssetWallet) setVPacketInputs(ctx context.Context, // createAndSetInput creates a virtual packet input for the given asset input // and sets it on the given virtual packet. func createAndSetInput(vPkt *tappsbt.VPacket, idx int, - params *address.ChainParams, assetInput *AnchoredCommitment, - inputProof *proof.Proof) error { + assetInput *AnchoredCommitment, inputProof *proof.Proof) error { internalKey := assetInput.InternalKey derivation, trDerivation := tappsbt.Bip32DerivationFromKeyDesc( - internalKey, params.HDCoinType, + internalKey, vPkt.ChainParams.HDCoinType, ) anchorPkScript, anchorMerkleRoot, _, err := tapsend.AnchorOutputScript( @@ -965,7 +981,7 @@ func createAndSetInput(vPkt *tappsbt.VPacket, idx int, } // fetchInputProof fetches the proof for the given asset input from the archive. -func (f *AssetWallet) fetchInputProof(ctx context.Context, +func fetchInputProof(ctx context.Context, exporter proof.Exporter, inputAsset *asset.Asset, anchorPoint wire.OutPoint) (*proof.Proof, error) { @@ -978,9 +994,7 @@ func (f *AssetWallet) fetchInputProof(ctx context.Context, if inputAsset.GroupKey != nil { proofLocator.GroupKey = &inputAsset.GroupKey.GroupPubKey } - inputProofBlob, err := f.cfg.AssetProofs.FetchProof( - ctx, proofLocator, - ) + inputProofBlob, err := exporter.FetchProof(ctx, proofLocator) if err != nil { return nil, fmt.Errorf("cannot fetch proof for input "+ "asset: %w", err) @@ -1152,7 +1166,7 @@ func verifyInclusionProof(vIn *tappsbt.VInput) error { // determinePassiveAssetAnchorOutput determines the best anchor output to attach // passive assets to. If no suitable output is found, a new anchor output is // created. -func (f *AssetWallet) determinePassiveAssetAnchorOutput(ctx context.Context, +func determinePassiveAssetAnchorOutput(ctx context.Context, keyRing KeyRing, activePackets []*tappsbt.VPacket) (*keychain.KeyDescriptor, uint32, error) { @@ -1174,7 +1188,7 @@ func (f *AssetWallet) determinePassiveAssetAnchorOutput(ctx context.Context, } // Ignore any anchor outputs that are not local to us. - if !f.cfg.KeyRing.IsLocalKey(ctx, anchorKeyDesc) { + if !keyRing.IsLocalKey(ctx, anchorKeyDesc) { continue } @@ -1230,7 +1244,7 @@ func (f *AssetWallet) determinePassiveAssetAnchorOutput(ctx context.Context, // If we're _still_ here, it means we haven't found a good candidate to // attach our passive assets to. We'll create a new anchor output for // them. - newInternalKey, err := f.cfg.KeyRing.DeriveNextKey( + newInternalKey, err := keyRing.DeriveNextKey( ctx, asset.TaprootAssetsKeyFamily, ) if err != nil { @@ -1242,19 +1256,34 @@ func (f *AssetWallet) determinePassiveAssetAnchorOutput(ctx context.Context, } // CreatePassiveAssets creates passive asset packets for the given active -// packets and input Taproot Asset commitments. +// packets and input Taproot Asset commitments. This is just a convenience +// wrapper around the function with the same name. This avoids callers needing +// to have references to a key ring and proof exporter if they already have an +// instance of AssetWallet. func (f *AssetWallet) CreatePassiveAssets(ctx context.Context, activePackets []*tappsbt.VPacket, inputCommitments tappsbt.InputCommitments) ([]*tappsbt.VPacket, error) { + return CreatePassiveAssets( + ctx, f.cfg.KeyRing, f.cfg.AssetProofs, activePackets, + inputCommitments, + ) +} + +// CreatePassiveAssets creates passive asset packets for the given active +// packets and input Taproot Asset commitments. +func CreatePassiveAssets(ctx context.Context, keyRing KeyRing, + exporter proof.Exporter, activePackets []*tappsbt.VPacket, + inputCommitments tappsbt.InputCommitments) ([]*tappsbt.VPacket, error) { + // We want to identify the best anchor output to use to attach our // passive assets. This is only for the database entry, so we can show // the number of passive assets in a transfer to the user somewhere. If // we don't find an appropriate output, it might mean we're not creating // transfer input/output entries at all, and we can just create a new // output for them. - anchorOutDesc, anchorOutIdx, err := f.determinePassiveAssetAnchorOutput( - ctx, activePackets, + anchorOutDesc, anchorOutIdx, err := determinePassiveAssetAnchorOutput( + ctx, keyRing, activePackets, ) if err != nil { return nil, fmt.Errorf("unable to determine passive asset "+ @@ -1303,8 +1332,8 @@ func (f *AssetWallet) CreatePassiveAssets(ctx context.Context, // When there are left over passive assets, we need to create // packets for them as well. for _, passiveAsset := range passiveAssets { - inputProof, err := f.fetchInputProof( - ctx, passiveAsset, prevID.OutPoint, + inputProof, err := fetchInputProof( + ctx, exporter, passiveAsset, prevID.OutPoint, ) if err != nil { return nil, fmt.Errorf("error fetching input "+ @@ -1322,7 +1351,7 @@ func (f *AssetWallet) CreatePassiveAssets(ctx context.Context, scriptKey.SerializeCompressed()) passivePacket, err := createPassivePacket( - f.cfg.ChainParams, passiveAsset, activePackets, + passiveAsset, activePackets, anchorOutIdx, *anchorOutDesc, prevID.OutPoint, inputProof, altLeaves, ) From 67f4a74dce29ffb84a815260f3a7d78d7a631357 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:39 +0100 Subject: [PATCH 10/16] tapfreighter: move funding related code into its own file This is a pure code move. We're going to add unit tests for the moved functions in the next commit. --- tapfreighter/fund.go | 393 +++++++++++++++++++++++++++++++++++++++++ tapfreighter/wallet.go | 374 --------------------------------------- 2 files changed, 393 insertions(+), 374 deletions(-) create mode 100644 tapfreighter/fund.go diff --git a/tapfreighter/fund.go b/tapfreighter/fund.go new file mode 100644 index 000000000..baa639aff --- /dev/null +++ b/tapfreighter/fund.go @@ -0,0 +1,393 @@ +package tapfreighter + +import ( + "bytes" + "context" + "errors" + "fmt" + + "github.com/btcsuite/btcd/btcutil/psbt" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/taproot-assets/address" + "github.com/lightninglabs/taproot-assets/asset" + "github.com/lightninglabs/taproot-assets/commitment" + "github.com/lightninglabs/taproot-assets/fn" + "github.com/lightninglabs/taproot-assets/proof" + "github.com/lightninglabs/taproot-assets/tappsbt" + "github.com/lightninglabs/taproot-assets/tapsend" +) + +// fundPacketWithInputs funds a virtual transaction with the given inputs. +func fundPacketWithInputs(ctx context.Context, exporter proof.Exporter, + keyRing KeyRing, addrBook AddrBook, fundDesc *tapsend.FundingDescriptor, + vPkt *tappsbt.VPacket, + selectedCommitments []*AnchoredCommitment) (*FundedVPacket, error) { + + if vPkt.ChainParams == nil { + return nil, errors.New("chain params not set in virtual packet") + } + + assetId, err := fundDesc.AssetSpecifier.UnwrapIdOrErr() + if err != nil { + return nil, err + } + + log.Infof("Selected %v asset inputs for send of %d to %x", + len(selectedCommitments), fundDesc.Amount, assetId[:]) + + assetType := selectedCommitments[0].Asset.Type + + totalInputAmt := uint64(0) + for _, anchorAsset := range selectedCommitments { + // We only use the sum of all assets of the same TAP commitment + // key to avoid counting passive assets as well. We'll filter + // out the passive assets from the selected commitments in a + // later step. + if anchorAsset.Asset.TapCommitmentKey() != + fundDesc.TapCommitmentKey() { + + continue + } + + totalInputAmt += anchorAsset.Asset.Amount + } + + inputCommitments, err := setVPacketInputs( + ctx, exporter, selectedCommitments, vPkt, + ) + if err != nil { + return nil, err + } + + fullValue, err := tapsend.ValidateInputs( + inputCommitments, assetType, fundDesc.AssetSpecifier, + fundDesc.Amount, + ) + if err != nil { + return nil, err + } + + // We want to know if we are sending to ourselves. We detect that by + // looking at the key descriptor of the script key. Because that is not + // part of addresses and might not be specified by the user through the + // PSBT interface, we now attempt to detect all local script keys and + // mark them as such by filling in the descriptor. + for idx := range vPkt.Outputs { + vOut := vPkt.Outputs[idx] + + tweakedKey, err := addrBook.FetchScriptKey( + ctx, vOut.ScriptKey.PubKey, + ) + switch { + case err == nil: + // We found a tweaked key for this output, so we'll + // update the key with the full descriptor info. + vOut.ScriptKey.TweakedScriptKey = tweakedKey + + case errors.Is(err, address.ErrScriptKeyNotFound): + // This is not a local key, or at least we don't know of + // it in the database. + continue + + default: + return nil, fmt.Errorf("cannot fetch script key: %w", + err) + } + } + + // We expect some change back, or have passive assets to commit to, so + // let's make sure we create a transfer output. + var changeOut *tappsbt.VOutput + if !fullValue { + // Do we need to add a change output? + changeOut, err = vPkt.SplitRootOutput() + if err != nil { + lastOut := vPkt.Outputs[len(vPkt.Outputs)-1] + splitOutIndex := lastOut.AnchorOutputIndex + 1 + changeOut = &tappsbt.VOutput{ + Type: tappsbt.TypeSplitRoot, + Interactive: lastOut.Interactive, + AnchorOutputIndex: splitOutIndex, + + // We want to handle deriving a real key in a + // generic manner, so we'll do that just below. + ScriptKey: asset.NUMSScriptKey, + } + + vPkt.Outputs = append(vPkt.Outputs, changeOut) + } + + // Since we know we're going to receive some change back, we + // need to make sure it is going to an address that we control. + // This should only be the case where we create the default + // change output with the NUMS key to avoid deriving too many + // keys prematurely. We don't need to derive a new key if we + // only have passive assets to commit to, since they all have + // their own script key and the output is more of a placeholder + // to attach the passive assets to. + unSpendable, err := changeOut.ScriptKey.IsUnSpendable() + if err != nil { + return nil, fmt.Errorf("cannot determine if script "+ + "key is spendable: %w", err) + } + if unSpendable && !fullValue { + changeScriptKey, err := keyRing.DeriveNextKey( + ctx, asset.TaprootAssetsKeyFamily, + ) + if err != nil { + return nil, err + } + + // We'll assume BIP-0086 everywhere, and use the tweaked + // key from here on out. + changeOut.ScriptKey = asset.NewScriptKeyBip86( + changeScriptKey, + ) + } + + // For existing change outputs, we'll just update the amount + // since we might not have known what coin would've been + // selected and how large the change would turn out to be. + changeOut.Amount = totalInputAmt - fundDesc.Amount + + // The asset version of the output should be the max of the set + // of input versions. We need to set this now as in + // PrepareOutputAssets locators are created which includes the + // version from the vOut. If we don't set it here, a v1 asset + // spent that becomes change will be a v0 if combined with such + // inputs. + // + // TODO(roasbeef): remove as not needed? + maxVersion := func(maxVersion asset.Version, + vInput *tappsbt.VInput) asset.Version { + + if vInput.Asset().Version > maxVersion { + return vInput.Asset().Version + } + + return maxVersion + } + changeOut.AssetVersion = fn.Reduce(vPkt.Inputs, maxVersion) + } + + // Before we can prepare output assets for our send, we need to generate + // a new internal key for the anchor outputs. We assume any output that + // hasn't got an internal key set is going to a local anchor, and we + // provide the internal key for that. + for idx := range vPkt.Outputs { + vOut := vPkt.Outputs[idx] + if vOut.AnchorOutputInternalKey != nil { + continue + } + + newInternalKey, err := keyRing.DeriveNextKey( + ctx, asset.TaprootAssetsKeyFamily, + ) + if err != nil { + return nil, err + } + vOut.SetAnchorInternalKey( + newInternalKey, vPkt.ChainParams.HDCoinType, + ) + } + + if err := tapsend.PrepareOutputAssets(ctx, vPkt); err != nil { + return nil, fmt.Errorf("unable to prepare outputs: %w", err) + } + + return &FundedVPacket{ + VPacket: vPkt, + InputCommitments: inputCommitments, + }, nil +} + +// setVPacketInputs sets the inputs of the given vPkt to the given send eligible +// commitments. It also returns the assets that were used as inputs. +func setVPacketInputs(ctx context.Context, exporter proof.Exporter, + eligibleCommitments []*AnchoredCommitment, + vPkt *tappsbt.VPacket) (tappsbt.InputCommitments, error) { + + vPkt.Inputs = make([]*tappsbt.VInput, len(eligibleCommitments)) + inputCommitments := make(tappsbt.InputCommitments) + + for idx := range eligibleCommitments { + // If the key found for the input UTXO cannot be identified as + // belonging to the lnd wallet, we won't be able to sign for it. + // This would happen if a user manually imported an asset that + // was issued/received for/on another node. We should probably + // not create asset entries for such imported assets in the + // first place, as we won't be able to spend it anyway. But for + // now we just put this check in place. + assetInput := eligibleCommitments[idx] + + // We'll also include an inclusion proof for the input asset in + // the virtual transaction. With that a signer can verify that + // the asset was actually committed to in the anchor output. + inputProof, err := fetchInputProof( + ctx, exporter, assetInput.Asset, assetInput.AnchorPoint, + ) + if err != nil { + return nil, fmt.Errorf("error fetching input proof: %w", + err) + } + + // Create the virtual packet input including the chain anchor + // information. + err = createAndSetInput( + vPkt, idx, assetInput, inputProof, + ) + if err != nil { + return nil, fmt.Errorf("unable to create and set "+ + "input: %w", err) + } + + prevID := vPkt.Inputs[idx].PrevID + inputCommitments[prevID] = assetInput.Commitment + } + + return inputCommitments, nil +} + +// createAndSetInput creates a virtual packet input for the given asset input +// and sets it on the given virtual packet. +func createAndSetInput(vPkt *tappsbt.VPacket, idx int, + assetInput *AnchoredCommitment, inputProof *proof.Proof) error { + + internalKey := assetInput.InternalKey + derivation, trDerivation := tappsbt.Bip32DerivationFromKeyDesc( + internalKey, vPkt.ChainParams.HDCoinType, + ) + + anchorPkScript, anchorMerkleRoot, _, err := tapsend.AnchorOutputScript( + internalKey.PubKey, assetInput.TapscriptSibling, + assetInput.Commitment, + ) + if err != nil { + return fmt.Errorf("cannot calculate input asset pk script: %w", + err) + } + + // Check if this is the anchorPkScript (and indirectly the + // anchorMerkleRoot) we expect. If not this might be a non-V2 + // commitment. + anchorTxOut := inputProof.AnchorTx.TxOut[assetInput.AnchorPoint.Index] + if !bytes.Equal(anchorTxOut.PkScript, anchorPkScript) { + var err error + + inputCommitment, err := assetInput.Commitment.Downgrade() + if err != nil { + return fmt.Errorf("cannot downgrade commitment: %w", + err) + } + + //nolint:lll + anchorPkScript, anchorMerkleRoot, _, err = tapsend.AnchorOutputScript( + internalKey.PubKey, assetInput.TapscriptSibling, + inputCommitment, + ) + if err != nil { + return fmt.Errorf("cannot calculate input asset "+ + "pkScript for commitment V0: %w", err) + } + + if !bytes.Equal(anchorTxOut.PkScript, anchorPkScript) { + // This matches neither version. + return fmt.Errorf("%w: anchor input script "+ + "mismatch for anchor outpoint %v", + tapsend.ErrInvalidAnchorInputInfo, + assetInput.AnchorPoint) + } + } + + // Add some trace logging for easier debugging of what we expect to be + // in the commitment we spend (we did the same when creating the output, + // so differences should be apparent when debugging). + tapsend.LogCommitment( + "Input", idx, assetInput.Commitment, internalKey.PubKey, + anchorPkScript, anchorMerkleRoot[:], + ) + + //nolint:lll + tapscriptSiblingBytes, _, err := commitment.MaybeEncodeTapscriptPreimage( + assetInput.TapscriptSibling, + ) + if err != nil { + return fmt.Errorf("cannot encode tapscript sibling: %w", err) + } + + // At this point, we have a valid "coin" to spend in the commitment, so + // we'll add the relevant information to the virtual TX's input. + prevID := asset.PrevID{ + OutPoint: assetInput.AnchorPoint, + ID: assetInput.Asset.ID(), + ScriptKey: asset.ToSerialized( + assetInput.Asset.ScriptKey.PubKey, + ), + } + vPkt.Inputs[idx] = &tappsbt.VInput{ + PrevID: prevID, + Anchor: tappsbt.Anchor{ + Value: assetInput.AnchorOutputValue, + PkScript: anchorPkScript, + InternalKey: internalKey.PubKey, + MerkleRoot: anchorMerkleRoot[:], + TapscriptSibling: tapscriptSiblingBytes, + Bip32Derivation: []*psbt.Bip32Derivation{derivation}, + TrBip32Derivation: []*psbt.TaprootBip32Derivation{ + trDerivation, + }, + }, + Proof: inputProof, + PInput: psbt.PInput{ + SighashType: txscript.SigHashDefault, + }, + } + vPkt.SetInputAsset(idx, assetInput.Asset) + + inputAltLeaves, err := assetInput.Commitment.FetchAltLeaves() + if err != nil { + return fmt.Errorf("cannot fetch alt leaves from input: %w", err) + } + + err = vPkt.Inputs[idx].SetAltLeaves(inputAltLeaves) + if err != nil { + return fmt.Errorf("cannot set alt leaves on vInput: %w", err) + } + + return nil +} + +// fetchInputProof fetches the proof for the given asset input from the archive. +func fetchInputProof(ctx context.Context, exporter proof.Exporter, + inputAsset *asset.Asset, anchorPoint wire.OutPoint) (*proof.Proof, + error) { + + assetID := inputAsset.ID() + proofLocator := proof.Locator{ + AssetID: &assetID, + ScriptKey: *inputAsset.ScriptKey.PubKey, + OutPoint: &anchorPoint, + } + if inputAsset.GroupKey != nil { + proofLocator.GroupKey = &inputAsset.GroupKey.GroupPubKey + } + inputProofBlob, err := exporter.FetchProof(ctx, proofLocator) + if err != nil { + return nil, fmt.Errorf("cannot fetch proof for input "+ + "asset: %w", err) + } + inputProofFile := &proof.File{} + err = inputProofFile.Decode(bytes.NewReader(inputProofBlob)) + if err != nil { + return nil, fmt.Errorf("cannot decode proof for input "+ + "asset: %w", err) + } + inputProof, err := inputProofFile.LastProof() + if err != nil { + return nil, fmt.Errorf("cannot get last proof for "+ + "input asset: %w", err) + } + + return inputProof, nil +} diff --git a/tapfreighter/wallet.go b/tapfreighter/wallet.go index 7859aff06..0668044f0 100644 --- a/tapfreighter/wallet.go +++ b/tapfreighter/wallet.go @@ -640,380 +640,6 @@ func hasOtherAssets(inputCommitments tappsbt.InputCommitments, return false, nil } -// fundPacketWithInputs funds a virtual transaction with the given inputs. -func fundPacketWithInputs(ctx context.Context, exporter proof.Exporter, - keyRing KeyRing, addrBook AddrBook, fundDesc *tapsend.FundingDescriptor, - vPkt *tappsbt.VPacket, - selectedCommitments []*AnchoredCommitment) (*FundedVPacket, error) { - - if vPkt.ChainParams == nil { - return nil, errors.New("chain params not set in virtual packet") - } - - assetId, err := fundDesc.AssetSpecifier.UnwrapIdOrErr() - if err != nil { - return nil, err - } - - log.Infof("Selected %v asset inputs for send of %d to %x", - len(selectedCommitments), fundDesc.Amount, assetId[:]) - - assetType := selectedCommitments[0].Asset.Type - - totalInputAmt := uint64(0) - for _, anchorAsset := range selectedCommitments { - // We only use the sum of all assets of the same TAP commitment - // key to avoid counting passive assets as well. We'll filter - // out the passive assets from the selected commitments in a - // later step. - if anchorAsset.Asset.TapCommitmentKey() != - fundDesc.TapCommitmentKey() { - - continue - } - - totalInputAmt += anchorAsset.Asset.Amount - } - - inputCommitments, err := setVPacketInputs( - ctx, exporter, selectedCommitments, vPkt, - ) - if err != nil { - return nil, err - } - - fullValue, err := tapsend.ValidateInputs( - inputCommitments, assetType, fundDesc.AssetSpecifier, - fundDesc.Amount, - ) - if err != nil { - return nil, err - } - - // We want to know if we are sending to ourselves. We detect that by - // looking at the key descriptor of the script key. Because that is not - // part of addresses and might not be specified by the user through the - // PSBT interface, we now attempt to detect all local script keys and - // mark them as such by filling in the descriptor. - for idx := range vPkt.Outputs { - vOut := vPkt.Outputs[idx] - - tweakedKey, err := addrBook.FetchScriptKey( - ctx, vOut.ScriptKey.PubKey, - ) - switch { - case err == nil: - // We found a tweaked key for this output, so we'll - // update the key with the full descriptor info. - vOut.ScriptKey.TweakedScriptKey = tweakedKey - - case errors.Is(err, address.ErrScriptKeyNotFound): - // This is not a local key, or at least we don't know of - // it in the database. - continue - - default: - return nil, fmt.Errorf("cannot fetch script key: %w", - err) - } - } - - // We expect some change back, or have passive assets to commit to, so - // let's make sure we create a transfer output. - var changeOut *tappsbt.VOutput - if !fullValue { - // Do we need to add a change output? - changeOut, err = vPkt.SplitRootOutput() - if err != nil { - lastOut := vPkt.Outputs[len(vPkt.Outputs)-1] - splitOutIndex := lastOut.AnchorOutputIndex + 1 - changeOut = &tappsbt.VOutput{ - Type: tappsbt.TypeSplitRoot, - Interactive: lastOut.Interactive, - AnchorOutputIndex: splitOutIndex, - - // We want to handle deriving a real key in a - // generic manner, so we'll do that just below. - ScriptKey: asset.NUMSScriptKey, - } - - vPkt.Outputs = append(vPkt.Outputs, changeOut) - } - - // Since we know we're going to receive some change back, we - // need to make sure it is going to an address that we control. - // This should only be the case where we create the default - // change output with the NUMS key to avoid deriving too many - // keys prematurely. We don't need to derive a new key if we - // only have passive assets to commit to, since they all have - // their own script key and the output is more of a placeholder - // to attach the passive assets to. - unSpendable, err := changeOut.ScriptKey.IsUnSpendable() - if err != nil { - return nil, fmt.Errorf("cannot determine if script "+ - "key is spendable: %w", err) - } - if unSpendable && !fullValue { - changeScriptKey, err := keyRing.DeriveNextKey( - ctx, asset.TaprootAssetsKeyFamily, - ) - if err != nil { - return nil, err - } - - // We'll assume BIP-0086 everywhere, and use the tweaked - // key from here on out. - changeOut.ScriptKey = asset.NewScriptKeyBip86( - changeScriptKey, - ) - } - - // For existing change outputs, we'll just update the amount - // since we might not have known what coin would've been - // selected and how large the change would turn out to be. - changeOut.Amount = totalInputAmt - fundDesc.Amount - - // The asset version of the output should be the max of the set - // of input versions. We need to set this now as in - // PrepareOutputAssets locators are created which includes the - // version from the vOut. If we don't set it here, a v1 asset - // spent that becomes change will be a v0 if combined with such - // inputs. - // - // TODO(roasbeef): remove as not needed? - maxVersion := func(maxVersion asset.Version, - vInput *tappsbt.VInput) asset.Version { - - if vInput.Asset().Version > maxVersion { - return vInput.Asset().Version - } - - return maxVersion - } - changeOut.AssetVersion = fn.Reduce(vPkt.Inputs, maxVersion) - } - - // Before we can prepare output assets for our send, we need to generate - // a new internal key for the anchor outputs. We assume any output that - // hasn't got an internal key set is going to a local anchor, and we - // provide the internal key for that. - for idx := range vPkt.Outputs { - vOut := vPkt.Outputs[idx] - if vOut.AnchorOutputInternalKey != nil { - continue - } - - newInternalKey, err := keyRing.DeriveNextKey( - ctx, asset.TaprootAssetsKeyFamily, - ) - if err != nil { - return nil, err - } - vOut.SetAnchorInternalKey( - newInternalKey, vPkt.ChainParams.HDCoinType, - ) - } - - if err := tapsend.PrepareOutputAssets(ctx, vPkt); err != nil { - return nil, fmt.Errorf("unable to prepare outputs: %w", err) - } - - return &FundedVPacket{ - VPacket: vPkt, - InputCommitments: inputCommitments, - }, nil -} - -// setVPacketInputs sets the inputs of the given vPkt to the given send eligible -// commitments. It also returns the assets that were used as inputs. -func setVPacketInputs(ctx context.Context, exporter proof.Exporter, - eligibleCommitments []*AnchoredCommitment, - vPkt *tappsbt.VPacket) (tappsbt.InputCommitments, error) { - - vPkt.Inputs = make([]*tappsbt.VInput, len(eligibleCommitments)) - inputCommitments := make(tappsbt.InputCommitments) - - for idx := range eligibleCommitments { - // If the key found for the input UTXO cannot be identified as - // belonging to the lnd wallet, we won't be able to sign for it. - // This would happen if a user manually imported an asset that - // was issued/received for/on another node. We should probably - // not create asset entries for such imported assets in the - // first place, as we won't be able to spend it anyway. But for - // now we just put this check in place. - assetInput := eligibleCommitments[idx] - - // We'll also include an inclusion proof for the input asset in - // the virtual transaction. With that a signer can verify that - // the asset was actually committed to in the anchor output. - inputProof, err := fetchInputProof( - ctx, exporter, assetInput.Asset, assetInput.AnchorPoint, - ) - if err != nil { - return nil, fmt.Errorf("error fetching input proof: %w", - err) - } - - // Create the virtual packet input including the chain anchor - // information. - err = createAndSetInput( - vPkt, idx, assetInput, inputProof, - ) - if err != nil { - return nil, fmt.Errorf("unable to create and set "+ - "input: %w", err) - } - - prevID := vPkt.Inputs[idx].PrevID - inputCommitments[prevID] = assetInput.Commitment - } - - return inputCommitments, nil -} - -// createAndSetInput creates a virtual packet input for the given asset input -// and sets it on the given virtual packet. -func createAndSetInput(vPkt *tappsbt.VPacket, idx int, - assetInput *AnchoredCommitment, inputProof *proof.Proof) error { - - internalKey := assetInput.InternalKey - derivation, trDerivation := tappsbt.Bip32DerivationFromKeyDesc( - internalKey, vPkt.ChainParams.HDCoinType, - ) - - anchorPkScript, anchorMerkleRoot, _, err := tapsend.AnchorOutputScript( - internalKey.PubKey, assetInput.TapscriptSibling, - assetInput.Commitment, - ) - if err != nil { - return fmt.Errorf("cannot calculate input asset pk script: %w", - err) - } - - // Check if this is the anchorPkScript (and indirectly the - // anchorMerkleRoot) we expect. If not this might be a non-V2 - // commitment. - anchorTxOut := inputProof.AnchorTx.TxOut[assetInput.AnchorPoint.Index] - if !bytes.Equal(anchorTxOut.PkScript, anchorPkScript) { - var err error - - inputCommitment, err := assetInput.Commitment.Downgrade() - if err != nil { - return fmt.Errorf("cannot downgrade commitment: %w", - err) - } - - //nolint:lll - anchorPkScript, anchorMerkleRoot, _, err = tapsend.AnchorOutputScript( - internalKey.PubKey, assetInput.TapscriptSibling, - inputCommitment, - ) - if err != nil { - return fmt.Errorf("cannot calculate input asset "+ - "pkScript for commitment V0: %w", err) - } - - if !bytes.Equal(anchorTxOut.PkScript, anchorPkScript) { - // This matches neither version. - return fmt.Errorf("%w: anchor input script "+ - "mismatch for anchor outpoint %v", - tapsend.ErrInvalidAnchorInputInfo, - assetInput.AnchorPoint) - } - } - - // Add some trace logging for easier debugging of what we expect to be - // in the commitment we spend (we did the same when creating the output, - // so differences should be apparent when debugging). - tapsend.LogCommitment( - "Input", idx, assetInput.Commitment, internalKey.PubKey, - anchorPkScript, anchorMerkleRoot[:], - ) - - //nolint:lll - tapscriptSiblingBytes, _, err := commitment.MaybeEncodeTapscriptPreimage( - assetInput.TapscriptSibling, - ) - if err != nil { - return fmt.Errorf("cannot encode tapscript sibling: %w", err) - } - - // At this point, we have a valid "coin" to spend in the commitment, so - // we'll add the relevant information to the virtual TX's input. - prevID := asset.PrevID{ - OutPoint: assetInput.AnchorPoint, - ID: assetInput.Asset.ID(), - ScriptKey: asset.ToSerialized( - assetInput.Asset.ScriptKey.PubKey, - ), - } - vPkt.Inputs[idx] = &tappsbt.VInput{ - PrevID: prevID, - Anchor: tappsbt.Anchor{ - Value: assetInput.AnchorOutputValue, - PkScript: anchorPkScript, - InternalKey: internalKey.PubKey, - MerkleRoot: anchorMerkleRoot[:], - TapscriptSibling: tapscriptSiblingBytes, - Bip32Derivation: []*psbt.Bip32Derivation{derivation}, - TrBip32Derivation: []*psbt.TaprootBip32Derivation{ - trDerivation, - }, - }, - Proof: inputProof, - PInput: psbt.PInput{ - SighashType: txscript.SigHashDefault, - }, - } - vPkt.SetInputAsset(idx, assetInput.Asset) - - inputAltLeaves, err := assetInput.Commitment.FetchAltLeaves() - if err != nil { - return fmt.Errorf("cannot fetch alt leaves from input: %w", err) - } - - err = vPkt.Inputs[idx].SetAltLeaves(inputAltLeaves) - if err != nil { - return fmt.Errorf("cannot set alt leaves on vInput: %w", err) - } - - return nil -} - -// fetchInputProof fetches the proof for the given asset input from the archive. -func fetchInputProof(ctx context.Context, exporter proof.Exporter, - inputAsset *asset.Asset, anchorPoint wire.OutPoint) (*proof.Proof, - error) { - - assetID := inputAsset.ID() - proofLocator := proof.Locator{ - AssetID: &assetID, - ScriptKey: *inputAsset.ScriptKey.PubKey, - OutPoint: &anchorPoint, - } - if inputAsset.GroupKey != nil { - proofLocator.GroupKey = &inputAsset.GroupKey.GroupPubKey - } - inputProofBlob, err := exporter.FetchProof(ctx, proofLocator) - if err != nil { - return nil, fmt.Errorf("cannot fetch proof for input "+ - "asset: %w", err) - } - inputProofFile := &proof.File{} - err = inputProofFile.Decode(bytes.NewReader(inputProofBlob)) - if err != nil { - return nil, fmt.Errorf("cannot decode proof for input "+ - "asset: %w", err) - } - inputProof, err := inputProofFile.LastProof() - if err != nil { - return nil, fmt.Errorf("cannot get last proof for "+ - "input asset: %w", err) - } - - return inputProof, nil -} - // SignVirtualPacketOptions is a set of functional options that allow callers to // further modify the virtual packet signing process. type SignVirtualPacketOptions struct { From 180e9f4a9fda0f98740c7d2fb154f45fde927d11 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:40 +0100 Subject: [PATCH 11/16] key_ring+tapgarden: remove DeriveKey from iface, improve mock The DeriveKey method is currently not used anywhere. To simplify the mock, we simply remove it. --- key_ring.go | 18 ---------- tapgarden/custodian_test.go | 15 +++----- tapgarden/interface.go | 7 ---- tapgarden/mock.go | 65 +++++++++++++++++++++------------- tapgarden/planter_test.go | 69 +++++-------------------------------- 5 files changed, 54 insertions(+), 120 deletions(-) diff --git a/key_ring.go b/key_ring.go index 1b9971dd7..0f4b5ef54 100644 --- a/key_ring.go +++ b/key_ring.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - "github.com/davecgh/go-spew/spew" "github.com/lightninglabs/lndclient" "github.com/lightninglabs/taproot-assets/asset" "github.com/lightninglabs/taproot-assets/tapgarden" @@ -60,23 +59,6 @@ func (l *LndRpcKeyRing) DeriveNextTaprootAssetKey( return *keyDesc, nil } -// DeriveKey attempts to derive an arbitrary key specified by the passed -// KeyLocator. This may be used in several recovery scenarios, or when manually -// rotating something like our current default node key. -func (l *LndRpcKeyRing) DeriveKey(ctx context.Context, - keyLoc keychain.KeyLocator) (keychain.KeyDescriptor, error) { - - tapdLog.Debugf("Deriving new key, key_loc=%v", spew.Sdump(keyLoc)) - - keyDesc, err := l.lnd.WalletKit.DeriveKey(ctx, &keyLoc) - if err != nil { - return keychain.KeyDescriptor{}, fmt.Errorf("unable to "+ - "derive key ring: %w", err) - } - - return *keyDesc, nil -} - // IsLocalKey returns true if the key is under the control of the wallet // and can be derived by it. func (l *LndRpcKeyRing) IsLocalKey(ctx context.Context, diff --git a/tapgarden/custodian_test.go b/tapgarden/custodian_test.go index 74c2da232..d905bc7a0 100644 --- a/tapgarden/custodian_test.go +++ b/tapgarden/custodian_test.go @@ -525,13 +525,6 @@ func TestCustodianNewAddr(t *testing.T) { }) h.assertStartup() - // Store a new random address to the store. We need to acknowledge the - // creation of two keys in a goroutine to unblock the underlying key - // ring. - go func() { - <-h.keyRing.ReqKeys - <-h.keyRing.ReqKeys - }() ctx := context.Background() addr, _ := randAddr(h) proofCourierAddr := address.RandProofCourierAddr(t) @@ -542,6 +535,8 @@ func TestCustodianNewAddr(t *testing.T) { ) require.NoError(t, err) + h.keyRing.AssertNumberOfCalls(t, "DeriveNextTaprootAssetKey", 2) + h.assertAddrsRegistered(dbAddr) h.eventually(func() bool { @@ -589,10 +584,6 @@ func TestBookAssetSyncer(t *testing.T) { // If we add the asset to the asset syncer, address creation should // succeed. h.syncer.AddAsset(*newAsset) - go func() { - <-h.keyRing.ReqKeys - <-h.keyRing.ReqKeys - }() addrVersion = test.RandFlip(address.V0, address.V1) newAddr, err := h.addrBook.NewAddress( ctx, addrVersion, newAsset.ID(), 1, nil, proofCourierAddr, @@ -600,6 +591,8 @@ func TestBookAssetSyncer(t *testing.T) { require.NoError(t, err) require.NotNil(t, newAddr) + h.keyRing.AssertNumberOfCalls(t, "DeriveNextTaprootAssetKey", 2) + numAddrs := 1 validAddrs := fn.MakeSlice(newAddr) diff --git a/tapgarden/interface.go b/tapgarden/interface.go index d2d3fbb8a..70d77cf99 100644 --- a/tapgarden/interface.go +++ b/tapgarden/interface.go @@ -393,13 +393,6 @@ type KeyRing interface { DeriveNextKey(context.Context, keychain.KeyFamily) (keychain.KeyDescriptor, error) - // DeriveKey attempts to derive an arbitrary key specified by the - // passed KeyLocator. This may be used in several recovery scenarios, - // or when manually rotating something like our current default node - // key. - DeriveKey(context.Context, - keychain.KeyLocator) (keychain.KeyDescriptor, error) - // IsLocalKey returns true if the key is under the control of the wallet // and can be derived by it. IsLocalKey(context.Context, keychain.KeyDescriptor) bool diff --git a/tapgarden/mock.go b/tapgarden/mock.go index 94cfa9712..6577db165 100644 --- a/tapgarden/mock.go +++ b/tapgarden/mock.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "fmt" "math/rand" + "sync" "sync/atomic" "testing" "time" @@ -29,6 +30,7 @@ import ( "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -602,19 +604,29 @@ func (m *MockAssetSyncer) EnableAssetSync(_ context.Context, } type MockKeyRing struct { - FamIndex keychain.KeyFamily + mock.Mock + + sync.RWMutex + KeyIndex uint32 Keys map[keychain.KeyLocator]*btcec.PrivateKey - - ReqKeys chan *keychain.KeyDescriptor } +var _ KeyRing = (*MockKeyRing)(nil) + func NewMockKeyRing() *MockKeyRing { - return &MockKeyRing{ - Keys: make(map[keychain.KeyLocator]*btcec.PrivateKey), - ReqKeys: make(chan *keychain.KeyDescriptor), + keyRing := &MockKeyRing{ + Keys: make(map[keychain.KeyLocator]*btcec.PrivateKey), } + + keyRing.On( + "DeriveNextKey", mock.Anything, + keychain.KeyFamily(asset.TaprootAssetsKeyFamily), + ).Return(nil) + keyRing.On("DeriveNextTaprootAssetKey", mock.Anything).Return(nil) + + return keyRing } // DeriveNextTaprootAssetKey attempts to derive the *next* key within the @@ -622,21 +634,26 @@ func NewMockKeyRing() *MockKeyRing { func (m *MockKeyRing) DeriveNextTaprootAssetKey( ctx context.Context) (keychain.KeyDescriptor, error) { + m.Called(ctx) + return m.DeriveNextKey(ctx, asset.TaprootAssetsKeyFamily) } func (m *MockKeyRing) DeriveNextKey(ctx context.Context, keyFam keychain.KeyFamily) (keychain.KeyDescriptor, error) { + m.Called(ctx, keyFam) + select { case <-ctx.Done(): return keychain.KeyDescriptor{}, fmt.Errorf("shutting down") default: } + m.Lock() defer func() { - m.FamIndex++ m.KeyIndex++ + m.Unlock() }() priv, err := btcec.NewPrivateKey() @@ -646,7 +663,7 @@ func (m *MockKeyRing) DeriveNextKey(ctx context.Context, loc := keychain.KeyLocator{ Index: m.KeyIndex, - Family: m.FamIndex, + Family: keyFam, } m.Keys[loc] = priv @@ -656,29 +673,29 @@ func (m *MockKeyRing) DeriveNextKey(ctx context.Context, KeyLocator: loc, } - select { - case m.ReqKeys <- &desc: - case <-ctx.Done(): - return keychain.KeyDescriptor{}, fmt.Errorf("shutting down") - } - return desc, nil } -func (m *MockKeyRing) DeriveKey(ctx context.Context, - _ keychain.KeyLocator) (keychain.KeyDescriptor, error) { +func (m *MockKeyRing) IsLocalKey(ctx context.Context, + d keychain.KeyDescriptor) bool { - select { - case <-ctx.Done(): - return keychain.KeyDescriptor{}, fmt.Errorf("shutting down") - default: + m.Called(ctx, d) + + m.RLock() + defer m.RUnlock() + + priv, ok := m.Keys[d.KeyLocator] + if ok && priv.PubKey().IsEqual(d.PubKey) { + return true } - return keychain.KeyDescriptor{}, nil -} + for _, key := range m.Keys { + if key.PubKey().IsEqual(d.PubKey) { + return true + } + } -func (m *MockKeyRing) IsLocalKey(context.Context, keychain.KeyDescriptor) bool { - return true + return false } type MockGenSigner struct { diff --git a/tapgarden/planter_test.go b/tapgarden/planter_test.go index a5c0905c0..b1442f645 100644 --- a/tapgarden/planter_test.go +++ b/tapgarden/planter_test.go @@ -164,7 +164,9 @@ func (t *mintingTestHarness) refreshChainPlanter() { // newRandSeedlings creates numSeedlings amount of seedlings with random // initialized values. -func (t *mintingTestHarness) newRandSeedlings(numSeedlings int) []*tapgarden.Seedling { +func (t *mintingTestHarness) newRandSeedlings( + numSeedlings int) []*tapgarden.Seedling { + seedlings := make([]*tapgarden.Seedling, numSeedlings) for i := 0; i < numSeedlings; i++ { var n [32]byte @@ -214,44 +216,9 @@ func (t *mintingTestHarness) assertBatchResumedBackground(wg *sync.WaitGroup, }() } -func (t *mintingTestHarness) assertKeyDerived() *keychain.KeyDescriptor { - t.Helper() - - key, err := fn.RecvOrTimeout(t.keyRing.ReqKeys, defaultTimeout) - require.NoError(t, err) - - return *key -} - -// assertKeyDerivedBackground unblocks key derivation with the test harness key -// ring. This is only needed when using the key ring from a unit test and not -// the planter or caretaker. -func (t *mintingTestHarness) assertKeyDerivedBackground( - wg *sync.WaitGroup) **keychain.KeyDescriptor { - - t.Helper() - - var ( - key **keychain.KeyDescriptor - err error - ) - - wg.Add(1) - go func() { - defer wg.Done() - - key, err = fn.RecvOrTimeout(t.keyRing.ReqKeys, defaultTimeout) - require.NoError(t, err) - }() - - // This return value will be unsafe to use until we confirm that the - // above goroutine has returned. - return key -} - -// createExternalBatch creates a new pending batch outside of the planter, which +// createExternalBatch creates a new pending batch outside the planter, which // can then be stored on disk. -func (t *mintingTestHarness) createExternalBatch(wg *sync.WaitGroup, +func (t *mintingTestHarness) createExternalBatch( numSeedlings int) *tapgarden.MintingBatch { t.Helper() @@ -266,13 +233,11 @@ func (t *mintingTestHarness) createExternalBatch(wg *sync.WaitGroup, // The group internal key should be from the key ring since we // expect the caretaker to sign with it later. if seedling.EnableEmission { - t.assertKeyDerivedBackground(wg) groupKey, err := t.keyRing.DeriveNextKey( context.Background(), asset.TaprootAssetsKeyFamily, ) require.NoError(t, err) - wg.Wait() seedling.GroupInternalKey = &groupKey } @@ -280,12 +245,10 @@ func (t *mintingTestHarness) createExternalBatch(wg *sync.WaitGroup, seedlingsWithKeys[seedling.AssetName] = seedling } - t.assertKeyDerivedBackground(wg) batchInternalKey, err := t.keyRing.DeriveNextKey( context.Background(), asset.TaprootAssetsKeyFamily, ) require.NoError(t, err) - wg.Wait() newBatch := &tapgarden.MintingBatch{ CreationTime: time.Now(), @@ -307,6 +270,7 @@ func (t *mintingTestHarness) queueSeedlingsInBatch(isFunded bool, for i, seedling := range seedlings { seedling := seedling keyCount := 0 + t.keyRing.Calls = nil // For the first seedling sent, we should get a new request, // representing the batch internal key. @@ -332,11 +296,6 @@ func (t *mintingTestHarness) queueSeedlingsInBatch(isFunded bool, updates, err := t.planter.QueueNewSeedling(seedling) require.NoError(t, err) - for keyCount != 0 { - t.assertKeyDerived() - keyCount-- - } - // We should get an update from the update channel that the // seedling is now pending. update, err := fn.RecvOrTimeout(updates, defaultTimeout) @@ -347,6 +306,8 @@ func (t *mintingTestHarness) queueSeedlingsInBatch(isFunded bool, // The received update should be a state of MintingStateSeed. require.Equal(t, tapgarden.MintingStateSeed, update.NewState) + + t.keyRing.AssertNumberOfCalls(t, "DeriveNextKey", keyCount) } } @@ -1745,7 +1706,6 @@ func testFundSealBeforeFinalize(t *mintingTestHarness) { } t.fundBatch(&wg, respChan, &fundReq) - t.assertKeyDerived() t.assertGenesisTxFunded(&manualFee) t.assertFundBatch(&wg, respChan, "") @@ -1986,7 +1946,7 @@ func testFundSealOnRestart(t *mintingTestHarness) { // We should also be able to resume one batch even when resuming another // batch fails. Since we can only queue one batch at a time, we'll // insert another pending batch on disk while the planter is shut down. - dbBatch := t.createExternalBatch(&wg, numSeedlings) + dbBatch := t.createExternalBatch(numSeedlings) batchCount++ err := t.store.CommitMintingBatch(context.Background(), dbBatch) require.NoError(t, err) @@ -2084,12 +2044,6 @@ func TestGroupKeyRevealV1WitnessWithCustomRoot(t *testing.T) { hashLockPreimage = []byte("foobar") ) - // We expect two keys to be derived from the mock. - go func() { - <-mockKeyRing.ReqKeys - <-mockKeyRing.ReqKeys - }() - // The internal key is for the actual internal key of the group. internalKeyDesc, err := mockKeyRing.DeriveNextTaprootAssetKey(ctx) require.NoError(t, err) @@ -2252,11 +2206,6 @@ func TestGroupKeyRevealV1WitnessNoScripts(t *testing.T) { txValidator = &tap.ValidatorV0{} ) - // We expect just one key to be derived from the mock. - go func() { - <-mockKeyRing.ReqKeys - }() - // The internal key is for the actual internal key of the group. internalKeyDesc, err := mockKeyRing.DeriveNextTaprootAssetKey(ctx) require.NoError(t, err) From 74e75475964872772f01d88b5310f8afdb6b89b4 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:41 +0100 Subject: [PATCH 12/16] tapgarden+tapfreighter: add unit test for funding code --- tapfreighter/fund_test.go | 439 ++++++++++++++++++++++++++++++++++++++ tapgarden/mock.go | 37 ++++ 2 files changed, 476 insertions(+) create mode 100644 tapfreighter/fund_test.go diff --git a/tapfreighter/fund_test.go b/tapfreighter/fund_test.go new file mode 100644 index 000000000..60457a98f --- /dev/null +++ b/tapfreighter/fund_test.go @@ -0,0 +1,439 @@ +package tapfreighter + +import ( + "bytes" + "context" + "encoding/hex" + "fmt" + "os" + "strings" + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcec/v2/schnorr" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/taproot-assets/address" + "github.com/lightninglabs/taproot-assets/asset" + "github.com/lightninglabs/taproot-assets/commitment" + "github.com/lightninglabs/taproot-assets/internal/test" + "github.com/lightninglabs/taproot-assets/proof" + "github.com/lightninglabs/taproot-assets/tapgarden" + "github.com/lightninglabs/taproot-assets/tappsbt" + "github.com/lightninglabs/taproot-assets/tapsend" + "github.com/lightningnetwork/lnd/keychain" + "github.com/stretchr/testify/require" +) + +var ( + testParams = &address.RegressionNetTap + + oddTxBlockHexFileName = "../proof/testdata/odd-block.hex" +) + +type mockExporter struct { + singleProof proof.Proof +} + +func (m *mockExporter) FetchProof(context.Context, + proof.Locator) (proof.Blob, error) { + + f, err := proof.NewFile(proof.V0, m.singleProof) + if err != nil { + return nil, err + } + + var buf bytes.Buffer + err = f.Encode(&buf) + if err != nil { + return nil, err + } + + return buf.Bytes(), nil +} + +var _ proof.Exporter = (*mockExporter)(nil) + +type mockAddrBook struct { + scriptKeys []asset.TweakedScriptKey +} + +func (m *mockAddrBook) FetchScriptKey(_ context.Context, + tweakedScriptKey *btcec.PublicKey) (*asset.TweakedScriptKey, error) { + + for _, key := range m.scriptKeys { + tweakedKey := txscript.ComputeTaprootOutputKey( + key.RawKey.PubKey, key.Tweak, + ) + if tweakedKey.IsEqual(tweakedScriptKey) { + return &key, nil + } + } + + return nil, address.ErrScriptKeyNotFound +} + +func (m *mockAddrBook) FetchInternalKeyLocator(_ context.Context, + _ *btcec.PublicKey) (keychain.KeyLocator, error) { + + panic("not implemented") +} + +var _ AddrBook = (*mockAddrBook)(nil) + +func randProof(t *testing.T, amount uint64, + internalKey keychain.KeyDescriptor) proof.Proof { + + oddTxBlockHex, err := os.ReadFile(oddTxBlockHexFileName) + require.NoError(t, err) + + oddTxBlockBytes, err := hex.DecodeString( + strings.Trim(string(oddTxBlockHex), "\n"), + ) + require.NoError(t, err) + + var oddTxBlock wire.MsgBlock + err = oddTxBlock.Deserialize(bytes.NewReader(oddTxBlockBytes)) + require.NoError(t, err) + + randGen := asset.RandGenesis(t, asset.Normal) + scriptKey := test.RandPubKey(t) + + txMerkleProof := proof.TxMerkleProof{} + mintCommitment, assets, err := commitment.Mint( + nil, randGen, nil, &commitment.AssetDetails{ + Type: randGen.Type, + ScriptKey: test.PubToKeyDesc(scriptKey), + Amount: &amount, + LockTime: 1337, + RelativeLockTime: 6, + }, + ) + require.NoError(t, err) + proofAsset := assets[0] + + // Empty the raw script key, since we only serialize the tweaked + // pubkey. We'll also force the main script key to be an x-only key as + // well. + proofAsset.ScriptKey.PubKey, err = schnorr.ParsePubKey( + schnorr.SerializePubKey(proofAsset.ScriptKey.PubKey), + ) + require.NoError(t, err) + + proofAsset.ScriptKey.TweakedScriptKey = nil + + _, commitmentProof, err := mintCommitment.Proof( + proofAsset.TapCommitmentKey(), proofAsset.AssetCommitmentKey(), + ) + require.NoError(t, err) + + pkScript, _, _, err := tapsend.AnchorOutputScript( + internalKey.PubKey, nil, mintCommitment, + ) + require.NoError(t, err) + + anchorTx := wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{{}}, + TxOut: []*wire.TxOut{{ + Value: 1000, + PkScript: pkScript, + }}, + } + + return proof.Proof{ + PrevOut: randGen.FirstPrevOut, + BlockHeight: 42, + AnchorTx: anchorTx, + TxMerkleProof: txMerkleProof, + Asset: *proofAsset, + InclusionProof: proof.TaprootProof{ + OutputIndex: 0, + InternalKey: test.RandPubKey(t), + CommitmentProof: &proof.CommitmentProof{ + Proof: *commitmentProof, + }, + }, + MetaReveal: &proof.MetaReveal{ + Data: []byte("quoth the raven nevermore"), + Type: proof.MetaOpaque, + }, + GenesisReveal: &randGen, + } +} + +func assertOutputsEqual(t *testing.T, pkts []*tappsbt.VPacket, + expectedOutputs [][]*tappsbt.VOutput) { + + t.Helper() + + require.Len(t, pkts, len(expectedOutputs)) + for i, pkt := range pkts { + require.Len(t, pkt.Outputs, len(expectedOutputs[i])) + require.Len(t, pkt.Inputs, 1) + + for j, out := range pkt.Outputs { + expected := expectedOutputs[i][j] + errCtx := fmt.Sprintf("packet %d, output %d", i, j) + + // We don't expect all fields to match, so we can't just + // compare the outputs directly. + require.Equal(t, expected.Amount, out.Amount, errCtx) + require.Equal( + t, expected.ScriptKey, out.ScriptKey, errCtx, + ) + require.Equal( + t, expected.Interactive, out.Interactive, + errCtx, + ) + require.Equal(t, expected.Type, out.Type, errCtx) + require.Equal( + t, expected.AnchorOutputIndex, + out.AnchorOutputIndex, errCtx, + ) + require.Equal( + t, expected.AnchorOutputInternalKey, + out.AnchorOutputInternalKey, errCtx, + ) + require.Equal( + t, expected.AltLeaves, out.AltLeaves, errCtx, + ) + require.Equal( + t, expected.AnchorOutputTapscriptSibling, + out.AnchorOutputTapscriptSibling, errCtx, + ) + require.Equal( + t, expected.AssetVersion, out.AssetVersion, + errCtx, + ) + require.Equal( + t, expected.LockTime, out.LockTime, errCtx, + ) + require.Equal( + t, expected.RelativeLockTime, + out.RelativeLockTime, errCtx, + ) + require.Equal( + t, expected.ProofDeliveryAddress, + out.ProofDeliveryAddress, errCtx, + ) + + // We do expect the BIP-0032 derivations to be set on + // the resulting anchor outputs. But we can't really + // assert their values, just that they're set. + require.Len( + t, out.AnchorOutputBip32Derivation, 1, errCtx, + ) + require.Len( + t, out.AnchorOutputTaprootBip32Derivation, 1, + errCtx, + ) + } + } +} + +// TestFundPacket tests that a virtual packet is created correctly from the +// combination of a funding template and the actual selected inputs. This +// includes anchor output key generation and assertion of a change or tombstone +// output. +func TestFundPacket(t *testing.T) { + ctx := context.Background() + + internalKey, _ := test.RandKeyDesc(t) + scriptKey := asset.RandScriptKey(t) + + const mintAmount = 500 + inputProof := randProof(t, mintAmount, internalKey) + inputAsset := inputProof.Asset + assetID := inputAsset.ID() + + inputPrevID := asset.PrevID{ + OutPoint: inputProof.OutPoint(), + ID: assetID, + ScriptKey: asset.ToSerialized(inputAsset.ScriptKey.PubKey), + } + + inputCommitment, err := commitment.FromAssets(nil, &inputProof.Asset) + require.NoError(t, err) + + testCases := []struct { + name string + fundDesc *tapsend.FundingDescriptor + vPkt *tappsbt.VPacket + selectedCommitments []*AnchoredCommitment + keysDerived int + expectedErr string + expectedInputCommitments tappsbt.InputCommitments + expectedOutputs func(*testing.T, + *tapgarden.MockKeyRing) [][]*tappsbt.VOutput + }{ + { + name: "single input, no change present", + fundDesc: &tapsend.FundingDescriptor{ + AssetSpecifier: asset.NewSpecifierFromId( + assetID, + ), + Amount: 20, + }, + vPkt: &tappsbt.VPacket{ + ChainParams: testParams, + Outputs: []*tappsbt.VOutput{{ + Amount: 20, + ScriptKey: scriptKey, + Interactive: false, + }}, + }, + selectedCommitments: []*AnchoredCommitment{{ + AnchorPoint: inputProof.OutPoint(), + InternalKey: internalKey, + Commitment: inputCommitment, + Asset: &inputAsset, + }}, + keysDerived: 3, + expectedInputCommitments: tappsbt.InputCommitments{ + inputPrevID: inputCommitment, + }, + expectedOutputs: func(t *testing.T, + r *tapgarden.MockKeyRing) [][]*tappsbt.VOutput { + + pkt0Outputs := []*tappsbt.VOutput{{ + Amount: 20, + Type: tappsbt.TypeSimple, + ScriptKey: scriptKey, + AnchorOutputInternalKey: r.PubKeyAt( + t, 1, + ), + AnchorOutputIndex: 0, + }, { + Amount: mintAmount - 20, + Type: tappsbt.TypeSplitRoot, + ScriptKey: r.ScriptKeyAt(t, 0), + AnchorOutputInternalKey: r.PubKeyAt( + t, 2, + ), + AnchorOutputIndex: 1, + }} + + return [][]*tappsbt.VOutput{pkt0Outputs} + }, + }, + { + name: "single input, full value, no change present", + fundDesc: &tapsend.FundingDescriptor{ + AssetSpecifier: asset.NewSpecifierFromId( + assetID, + ), + Amount: mintAmount, + }, + vPkt: &tappsbt.VPacket{ + ChainParams: testParams, + Outputs: []*tappsbt.VOutput{{ + Amount: mintAmount, + ScriptKey: scriptKey, + Interactive: false, + }}, + }, + selectedCommitments: []*AnchoredCommitment{{ + AnchorPoint: inputProof.OutPoint(), + InternalKey: internalKey, + Commitment: inputCommitment, + Asset: &inputAsset, + }}, + keysDerived: 1, + expectedErr: "single output must be interactive", + }, + { + name: "single input, full value, change present", + fundDesc: &tapsend.FundingDescriptor{ + AssetSpecifier: asset.NewSpecifierFromId( + assetID, + ), + Amount: mintAmount, + }, + vPkt: &tappsbt.VPacket{ + ChainParams: testParams, + Outputs: []*tappsbt.VOutput{{ + Type: tappsbt.TypeSplitRoot, + Amount: 0, + ScriptKey: asset.NUMSScriptKey, + Interactive: false, + }, { + Amount: mintAmount, + ScriptKey: scriptKey, + Interactive: false, + AnchorOutputIndex: 1, + }}, + }, + selectedCommitments: []*AnchoredCommitment{{ + AnchorPoint: inputProof.OutPoint(), + InternalKey: internalKey, + Commitment: inputCommitment, + Asset: &inputAsset, + }}, + keysDerived: 2, + expectedInputCommitments: tappsbt.InputCommitments{ + inputPrevID: inputCommitment, + }, + expectedOutputs: func(t *testing.T, + r *tapgarden.MockKeyRing) [][]*tappsbt.VOutput { + + pkt0Outputs := []*tappsbt.VOutput{{ + Amount: 0, + Type: tappsbt.TypeSplitRoot, + ScriptKey: asset.NUMSScriptKey, + AnchorOutputInternalKey: r.PubKeyAt( + t, 0, + ), + AnchorOutputIndex: 0, + }, { + Amount: mintAmount, + Type: tappsbt.TypeSimple, + ScriptKey: scriptKey, + AnchorOutputInternalKey: r.PubKeyAt( + t, 1, + ), + AnchorOutputIndex: 1, + }} + + return [][]*tappsbt.VOutput{pkt0Outputs} + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(tt *testing.T) { + exporter := &mockExporter{ + singleProof: inputProof, + } + addrBook := &mockAddrBook{} + keyRing := tapgarden.NewMockKeyRing() + + result, err := fundPacketWithInputs( + ctx, exporter, keyRing, addrBook, + tc.fundDesc, tc.vPkt, tc.selectedCommitments, + ) + + keyRing.AssertNumberOfCalls( + tt, "DeriveNextKey", tc.keysDerived, + ) + + if tc.expectedErr != "" { + require.ErrorContains(tt, err, tc.expectedErr) + + return + } + + require.NoError(tt, err) + require.NotNil(tt, result) + + require.Equal( + tt, tc.expectedInputCommitments, + result.InputCommitments, + ) + assertOutputsEqual( + tt, []*tappsbt.VPacket{result.VPacket}, + tc.expectedOutputs(tt, keyRing), + ) + }) + } +} diff --git a/tapgarden/mock.go b/tapgarden/mock.go index 6577db165..ea53f23c3 100644 --- a/tapgarden/mock.go +++ b/tapgarden/mock.go @@ -698,6 +698,43 @@ func (m *MockKeyRing) IsLocalKey(ctx context.Context, return false } +func (m *MockKeyRing) PubKeyAt(t *testing.T, idx uint32) *btcec.PublicKey { + m.RLock() + defer m.RUnlock() + + loc := keychain.KeyLocator{ + Index: idx, + Family: asset.TaprootAssetsKeyFamily, + } + + priv, ok := m.Keys[loc] + if !ok { + t.Fatalf("script key not found at index %d", idx) + } + + return priv.PubKey() +} + +func (m *MockKeyRing) ScriptKeyAt(t *testing.T, idx uint32) asset.ScriptKey { + m.RLock() + defer m.RUnlock() + + loc := keychain.KeyLocator{ + Index: idx, + Family: asset.TaprootAssetsKeyFamily, + } + + priv, ok := m.Keys[loc] + if !ok { + t.Fatalf("script key not found at index %d", idx) + } + + return asset.NewScriptKeyBip86(keychain.KeyDescriptor{ + KeyLocator: loc, + PubKey: priv.PubKey(), + }) +} + type MockGenSigner struct { KeyRing *MockKeyRing failSigning atomic.Bool From f3e4fbe6b64670d22176dff0455e968405a7a066 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:43 +0100 Subject: [PATCH 13/16] multi: return multiple vPackets from funding --- rpcserver.go | 24 ++++-- tapchannel/aux_funding_controller.go | 108 +++++++++++++++------------ tapfreighter/chain_porter.go | 4 +- tapfreighter/fund.go | 19 +++-- tapfreighter/fund_test.go | 4 +- tapfreighter/wallet.go | 23 ++++-- tappsbt/interface.go | 3 - 7 files changed, 105 insertions(+), 80 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index d27c05df3..0b7ff9555 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2235,8 +2235,7 @@ func (r *rpcServer) FundVirtualPsbt(ctx context.Context, // Extract the passive assets that are needed for the fully RPC driven // flow. passivePackets, err := r.cfg.AssetWallet.CreatePassiveAssets( - ctx, []*tappsbt.VPacket{fundedVPkt.VPacket}, - fundedVPkt.InputCommitments, + ctx, fundedVPkt.VPackets, fundedVPkt.InputCommitments, ) if err != nil { return nil, fmt.Errorf("error creating passive assets: %w", err) @@ -2257,7 +2256,12 @@ func (r *rpcServer) FundVirtualPsbt(ctx context.Context, } } - response.FundedPsbt, err = serialize(fundedVPkt.VPacket) + // TODO(guggero): Remove this once we support multiple packets. + if len(fundedVPkt.VPackets) > 1 { + return nil, fmt.Errorf("only one packet supported") + } + + response.FundedPsbt, err = serialize(fundedVPkt.VPackets[0]) if err != nil { return nil, fmt.Errorf("error serializing packet: %w", err) } @@ -3421,16 +3425,22 @@ func (r *rpcServer) BurnAsset(ctx context.Context, return nil, fmt.Errorf("error funding burn: %w", err) } + // We don't support burning by group key yet, so we only expect a single + // vPacket (which implies a single asset ID is involved). + if len(fundResp.VPackets) > 1 { + return nil, fmt.Errorf("only one packet supported") + } + // Now we can sign the packet and send it to the chain. - _, err = r.cfg.AssetWallet.SignVirtualPacket(fundResp.VPacket) + vPkt := fundResp.VPackets[0] + _, err = r.cfg.AssetWallet.SignVirtualPacket(vPkt) if err != nil { return nil, fmt.Errorf("error signing packet: %w", err) } resp, err := r.cfg.ChainPorter.RequestShipment( tapfreighter.NewPreSignedParcel( - []*tappsbt.VPacket{fundResp.VPacket}, - fundResp.InputCommitments, in.Note, + fundResp.VPackets, fundResp.InputCommitments, in.Note, ), ) if err != nil { @@ -3445,7 +3455,7 @@ func (r *rpcServer) BurnAsset(ctx context.Context, var burnProof *taprpc.DecodedProof for idx := range resp.Outputs { - vOut := fundResp.VPacket.Outputs[idx] + vOut := vPkt.Outputs[idx] tOut := resp.Outputs[idx] if vOut.Asset.IsBurn() { p, err := proof.Decode(tOut.ProofSuffix) diff --git a/tapchannel/aux_funding_controller.go b/tapchannel/aux_funding_controller.go index a37d08ff9..c605f32e1 100644 --- a/tapchannel/aux_funding_controller.go +++ b/tapchannel/aux_funding_controller.go @@ -810,13 +810,19 @@ func (f *FundingController) fundVirtualPacket(ctx context.Context, // sendInputOwnershipProofs sends the input ownership proofs to the remote // party during the validation phase of the funding process. func (f *FundingController) sendInputOwnershipProofs(peerPub btcec.PublicKey, - vPkt *tappsbt.VPacket, fundingState *pendingAssetFunding) error { + vPackets []*tappsbt.VPacket, fundingState *pendingAssetFunding) error { ctx, done := f.WithCtxQuit() defer done() - log.Infof("Generating input ownership proofs for %v inputs", - len(vPkt.Inputs)) + log.Infof("Generating input ownership proofs for %v packets", + len(vPackets)) + + // TODO(guggero): Remove once we add group key support. + if len(vPackets) > 1 { + return fmt.Errorf("only one vPacket supported for now") + } + vPkt := vPackets[0] // For each of the inputs we selected, we'll create a new ownership // proof for each of them. We'll send this to the peer, so they can @@ -948,25 +954,25 @@ func (f *FundingController) signAllVPackets(ctx context.Context, log.Infof("Signing all funding vPackets") - activePkt := fundingVpkt.VPacket - - encoded, err := tappsbt.Encode(activePkt) - if err != nil { - return nil, nil, nil, fmt.Errorf("unable to encode active "+ - "packet: %w", err) - } + activePackets := fundingVpkt.VPackets + for idx := range activePackets { + encoded, err := tappsbt.Encode(activePackets[idx]) + if err != nil { + return nil, nil, nil, fmt.Errorf("unable to encode "+ + "active packet: %w", err) + } - log.Debugf("Active packet: %x", encoded) + log.Debugf("Active packet %d: %x", idx, encoded) - _, err = f.cfg.AssetWallet.SignVirtualPacket(activePkt) - if err != nil { - return nil, nil, nil, fmt.Errorf("unable to sign and commit "+ - "virtual packet: %w", err) + _, err = f.cfg.AssetWallet.SignVirtualPacket(activePackets[idx]) + if err != nil { + return nil, nil, nil, fmt.Errorf("unable to sign and "+ + "commit virtual packet: %w", err) + } } passivePkts, err := f.cfg.AssetWallet.CreatePassiveAssets( - ctx, []*tappsbt.VPacket{activePkt}, - fundingVpkt.InputCommitments, + ctx, activePackets, fundingVpkt.InputCommitments, ) if err != nil { return nil, nil, nil, fmt.Errorf("unable to create passive "+ @@ -978,7 +984,7 @@ func (f *FundingController) signAllVPackets(ctx context.Context, "assets: %w", err) } - allPackets := append([]*tappsbt.VPacket{}, activePkt) + allPackets := append([]*tappsbt.VPacket{}, activePackets...) allPackets = append(allPackets, passivePkts...) err = tapsend.ValidateVPacketVersions(allPackets) @@ -986,7 +992,7 @@ func (f *FundingController) signAllVPackets(ctx context.Context, return nil, nil, nil, fmt.Errorf("signed packets: %w", err) } - return allPackets, []*tappsbt.VPacket{activePkt}, passivePkts, nil + return allPackets, activePackets, passivePkts, nil } // anchorVPackets anchors the vPackets to the funding PSBT, creating a @@ -1156,20 +1162,25 @@ func (f *FundingController) completeChannelFunding(ctx context.Context, return nil, fmt.Errorf("unable to parse internal key: %w", err) } - fundedVpkt.VPacket.Outputs[0].AnchorOutputBip32Derivation = nil - fundedVpkt.VPacket.Outputs[0].AnchorOutputTaprootBip32Derivation = nil - fundingInternalKeyDesc := keychain.KeyDescriptor{ - PubKey: fundingInternalKey, + // Overwrite the funding output's anchor information with the on-chain + // funding output internal key (MuSig2 key). + fundingPackets := fundedVpkt.VPackets + for idx := range fundingPackets { + fundingPkt := fundingPackets[idx] + fundingPkt.Outputs[0].AnchorOutputBip32Derivation = nil + fundingPkt.Outputs[0].AnchorOutputTaprootBip32Derivation = nil + fundingInternalKeyDesc := keychain.KeyDescriptor{ + PubKey: fundingInternalKey, + } + fundingPkt.Outputs[0].SetAnchorInternalKey( + fundingInternalKeyDesc, f.cfg.ChainParams.HDCoinType, + ) } - fundedVpkt.VPacket.Outputs[0].SetAnchorInternalKey( - fundingInternalKeyDesc, f.cfg.ChainParams.HDCoinType, - ) // Given the asset inputs selected in the prior step, we'll now // construct a template packet that maps our asset inputs to actual // inputs in the PSBT packet. - fundingVPkts := []*tappsbt.VPacket{fundedVpkt.VPacket} - fundingPsbt, err := tapsend.PrepareAnchoringTemplate(fundingVPkts) + fundingPsbt, err := tapsend.PrepareAnchoringTemplate(fundingPackets) if err != nil { return nil, err } @@ -1551,10 +1562,14 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex, // Now that we've funded the vPk, keep track of the set of inputs we // locked to ensure we unlock them later. - fundingState.lockedAssetInputs = fn.Map( - fundingVpkt.VPacket.Inputs, - func(in *tappsbt.VInput) wire.OutPoint { - return in.PrevID.OutPoint + fundingState.lockedAssetInputs = fn.FlatMap( + fundingVpkt.VPackets, func(p *tappsbt.VPacket) []wire.OutPoint { + return fn.Map( + p.Inputs, + func(in *tappsbt.VInput) wire.OutPoint { + return in.PrevID.OutPoint + }, + ) }, ) @@ -1573,8 +1588,8 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex, } } - // Register a defer to execute if none of the set up below succeeds. - // This ensure we always unlock the UTXO. + // Register a defer to execute if none of the setup below succeeds. + // This ensures we always unlock the UTXO. var setupSuccess bool defer func() { if !setupSuccess { @@ -1586,15 +1601,11 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex, // we allow to be commited to a single channel. This is to make sure we // have a decent number of HTLCs available. See Godoc of maxNumAssetIDs // for more information. - // - // TODO(guggero): This following code is obviously wrong and needs to be - // changed when we support committing fungible assets into a channel. To - // avoid this TODO from being overlooked, we add a dummy implementation - // with a condition that currently will never be true (since there's - // only a single vPacket being selected currently anyway). assetIDSet := lfn.NewSet[asset.ID]() - for _, out := range fundingVpkt.VPacket.Outputs { - assetIDSet.Add(out.Asset.ID()) + for _, fundingPacket := range fundingVpkt.VPackets { + for _, out := range fundingPacket.Outputs { + assetIDSet.Add(out.Asset.ID()) + } } if assetIDSet.Size() > maxNumAssetIDs { return fmt.Errorf("too many different asset IDs in channel "+ @@ -1606,17 +1617,20 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex, // we can derive the tapscript root that'll be used alongside the // internal key (which we'll only learn from lnd later as we finalize // the funding PSBT). - fundingOutput := fundingVpkt.VPacket.Outputs[0] + fundingAssets := fn.Map( + fundingVpkt.VPackets, func(pkt *tappsbt.VPacket) *asset.Asset { + return pkt.Outputs[0].Asset.Copy() + }, + ) fundingCommitVersion, err := tappsbt.CommitmentVersion( - fundingVpkt.VPacket.Version, + fundingVpkt.VPackets[0].Version, ) if err != nil { return fmt.Errorf("unable to create commitment: %w", err) } fundingCommitment, err := commitment.FromAssets( - fundingCommitVersion, - fundingOutput.Asset.Copy(), + fundingCommitVersion, fundingAssets..., ) if err != nil { return fmt.Errorf("unable to create commitment: %w", err) @@ -1633,7 +1647,7 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex, // need to derive then send a series of ownership // proofs to the remote party. err = f.sendInputOwnershipProofs( - fundReq.PeerPub, fundingVpkt.VPacket, fundingState, + fundReq.PeerPub, fundingVpkt.VPackets, fundingState, ) if err != nil { return fmt.Errorf("unable to send input ownership "+ diff --git a/tapfreighter/chain_porter.go b/tapfreighter/chain_porter.go index 9dce8c5ed..922c22671 100644 --- a/tapfreighter/chain_porter.go +++ b/tapfreighter/chain_porter.go @@ -1090,9 +1090,7 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) { "%w", err) } - currentPkg.VirtualPackets = []*tappsbt.VPacket{ - fundSendRes.VPacket, - } + currentPkg.VirtualPackets = fundSendRes.VPackets currentPkg.InputCommitments = fundSendRes.InputCommitments currentPkg.SendState = SendStateVirtualSign diff --git a/tapfreighter/fund.go b/tapfreighter/fund.go index baa639aff..04f5f7ef5 100644 --- a/tapfreighter/fund.go +++ b/tapfreighter/fund.go @@ -18,8 +18,11 @@ import ( "github.com/lightninglabs/taproot-assets/tapsend" ) -// fundPacketWithInputs funds a virtual transaction with the given inputs. -func fundPacketWithInputs(ctx context.Context, exporter proof.Exporter, +// createFundedPacketWithInputs funds a set of virtual transaction with the +// given inputs. A new vPacket is created for each tranche of the active asset +// (which is the one specified in the funding descriptor, which is either a +// single asset ID/tranche or group key with multiple tranches). +func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, keyRing KeyRing, addrBook AddrBook, fundDesc *tapsend.FundingDescriptor, vPkt *tappsbt.VPacket, selectedCommitments []*AnchoredCommitment) (*FundedVPacket, error) { @@ -28,13 +31,9 @@ func fundPacketWithInputs(ctx context.Context, exporter proof.Exporter, return nil, errors.New("chain params not set in virtual packet") } - assetId, err := fundDesc.AssetSpecifier.UnwrapIdOrErr() - if err != nil { - return nil, err - } - - log.Infof("Selected %v asset inputs for send of %d to %x", - len(selectedCommitments), fundDesc.Amount, assetId[:]) + log.Infof("Selected %v asset inputs for send of %d to %s", + len(selectedCommitments), fundDesc.Amount, + &fundDesc.AssetSpecifier) assetType := selectedCommitments[0].Asset.Type @@ -197,7 +196,7 @@ func fundPacketWithInputs(ctx context.Context, exporter proof.Exporter, } return &FundedVPacket{ - VPacket: vPkt, + VPackets: []*tappsbt.VPacket{vPkt}, InputCommitments: inputCommitments, }, nil } diff --git a/tapfreighter/fund_test.go b/tapfreighter/fund_test.go index 60457a98f..ed200cf5d 100644 --- a/tapfreighter/fund_test.go +++ b/tapfreighter/fund_test.go @@ -408,7 +408,7 @@ func TestFundPacket(t *testing.T) { addrBook := &mockAddrBook{} keyRing := tapgarden.NewMockKeyRing() - result, err := fundPacketWithInputs( + result, err := createFundedPacketWithInputs( ctx, exporter, keyRing, addrBook, tc.fundDesc, tc.vPkt, tc.selectedCommitments, ) @@ -431,7 +431,7 @@ func TestFundPacket(t *testing.T) { result.InputCommitments, ) assertOutputsEqual( - tt, []*tappsbt.VPacket{result.VPacket}, + tt, result.VPackets, tc.expectedOutputs(tt, keyRing), ) }) diff --git a/tapfreighter/wallet.go b/tapfreighter/wallet.go index 0668044f0..6d952dc7c 100644 --- a/tapfreighter/wallet.go +++ b/tapfreighter/wallet.go @@ -224,9 +224,9 @@ func NewAssetWallet(cfg *WalletConfig) *AssetWallet { // FundedVPacket is the result from an attempt to fund a given Taproot Asset // address send request via a call to FundAddressSend. type FundedVPacket struct { - // VPacket is the virtual transaction that was created to fund the - // transfer. - VPacket *tappsbt.VPacket + // VPackets is a list of virtual transaction that was created to fund + // the transfer. + VPackets []*tappsbt.VPacket // InputCommitments is a map from virtual package input index to its // associated Taproot Asset commitment. @@ -453,7 +453,7 @@ func (f *AssetWallet) FundPacket(ctx context.Context, } }() - pkt, err := fundPacketWithInputs( + pkt, err := createFundedPacketWithInputs( ctx, f.cfg.AssetProofs, f.cfg.KeyRing, f.cfg.AddrBook, fundDesc, vPkt, selectedCommitments, ) @@ -566,7 +566,7 @@ func (f *AssetWallet) FundBurn(ctx context.Context, // The virtual transaction is now ready to be further enriched with the // split commitment and other data. - fundedPkt, err := fundPacketWithInputs( + fundedPkt, err := createFundedPacketWithInputs( ctx, f.cfg.AssetProofs, f.cfg.KeyRing, f.cfg.AddrBook, fundDesc, vPkt, selectedCommitments, ) @@ -574,12 +574,19 @@ func (f *AssetWallet) FundBurn(ctx context.Context, return nil, err } + // We don't support burning by group key yet, so we only expect a single + // vPacket (which implies a single asset ID is involved). + if len(fundedPkt.VPackets) != 1 { + return nil, fmt.Errorf("expected a single vPacket, got %d", + len(fundedPkt.VPackets)) + } + // We want to avoid a BTC output being created that just sits there // without an actual commitment in it. So if we are not getting any // change or passive assets in this output, we'll not want to go through // with it. - firstOut := fundedPkt.VPacket.Outputs[0] - if len(fundedPkt.VPacket.Outputs) == 1 && + firstOut := fundedPkt.VPackets[0].Outputs[0] + if len(fundedPkt.VPackets[0].Outputs) == 1 && firstOut.Amount == fundDesc.Amount { // A burn is an interactive transfer. So we don't expect there @@ -589,7 +596,7 @@ func (f *AssetWallet) FundBurn(ctx context.Context, // case, we'll return as burning all assets in an anchor output // is not supported. otherAssets, err := hasOtherAssets( - fundedPkt.InputCommitments, []*tappsbt.VPacket{vPkt}, + fundedPkt.InputCommitments, fundedPkt.VPackets, ) if err != nil { return nil, err diff --git a/tappsbt/interface.go b/tappsbt/interface.go index 579a06abc..006eb267a 100644 --- a/tappsbt/interface.go +++ b/tappsbt/interface.go @@ -167,9 +167,6 @@ var ( // split again in this virtual transaction). Therefore, if an anchor output // carries commitments for multiple assets, a virtual transaction needs to be // created, signed and then anchored for each asset ID separately. -// -// TODO(guggero): Actually support merging multiple virtual transactions into a -// single BTC transaction. type VPacket struct { // Inputs is the list of asset inputs that are being spent. Inputs []*VInput From 08d0354f6e52b06a45048bb35b6f3a578aac7218 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:44 +0100 Subject: [PATCH 14/16] tapfreighter: extract functions --- tapfreighter/fund.go | 106 ++++++++++++++++++++++++++++--------------- 1 file changed, 69 insertions(+), 37 deletions(-) diff --git a/tapfreighter/fund.go b/tapfreighter/fund.go index 04f5f7ef5..d724e4ec7 100644 --- a/tapfreighter/fund.go +++ b/tapfreighter/fund.go @@ -67,6 +67,56 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, return nil, err } + // Make sure we'll recognize local script keys in the virtual packet + // later on in the process by annotating them with the full descriptor + // information. + if err := annotateLocalScriptKeys(ctx, vPkt, addrBook); err != nil { + return nil, err + } + + // If we don't spend the full value, we need to create a change output. + changeAmount := totalInputAmt - fundDesc.Amount + err = createChangeOutput(ctx, vPkt, keyRing, fullValue, changeAmount) + if err != nil { + return nil, err + } + + // Before we can prepare output assets for our send, we need to generate + // a new internal key for the anchor outputs. We assume any output that + // hasn't got an internal key set is going to a local anchor, and we + // provide the internal key for that. + for idx := range vPkt.Outputs { + vOut := vPkt.Outputs[idx] + if vOut.AnchorOutputInternalKey != nil { + continue + } + + newInternalKey, err := keyRing.DeriveNextKey( + ctx, asset.TaprootAssetsKeyFamily, + ) + if err != nil { + return nil, err + } + vOut.SetAnchorInternalKey( + newInternalKey, vPkt.ChainParams.HDCoinType, + ) + } + + if err := tapsend.PrepareOutputAssets(ctx, vPkt); err != nil { + return nil, fmt.Errorf("unable to prepare outputs: %w", err) + } + + return &FundedVPacket{ + VPackets: []*tappsbt.VPacket{vPkt}, + InputCommitments: inputCommitments, + }, nil +} + +// annotateLocalScriptKeys annotates the local script keys in the given virtual +// packet with the full descriptor information. +func annotateLocalScriptKeys(ctx context.Context, vPkt *tappsbt.VPacket, + addrBook AddrBook) error { + // We want to know if we are sending to ourselves. We detect that by // looking at the key descriptor of the script key. Because that is not // part of addresses and might not be specified by the user through the @@ -90,14 +140,24 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, continue default: - return nil, fmt.Errorf("cannot fetch script key: %w", - err) + return fmt.Errorf("cannot fetch script key: %w", err) } } + return nil +} + +// createChangeOutput creates a change output for the given virtual packet if +// it isn't fully spent. +func createChangeOutput(ctx context.Context, vPkt *tappsbt.VPacket, + keyRing KeyRing, fullValue bool, changeAmount uint64) error { + // We expect some change back, or have passive assets to commit to, so // let's make sure we create a transfer output. - var changeOut *tappsbt.VOutput + var ( + changeOut *tappsbt.VOutput + err error + ) if !fullValue { // Do we need to add a change output? changeOut, err = vPkt.SplitRootOutput() @@ -127,15 +187,15 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, // to attach the passive assets to. unSpendable, err := changeOut.ScriptKey.IsUnSpendable() if err != nil { - return nil, fmt.Errorf("cannot determine if script "+ - "key is spendable: %w", err) + return fmt.Errorf("cannot determine if script key is "+ + "spendable: %w", err) } - if unSpendable && !fullValue { + if unSpendable { changeScriptKey, err := keyRing.DeriveNextKey( ctx, asset.TaprootAssetsKeyFamily, ) if err != nil { - return nil, err + return err } // We'll assume BIP-0086 everywhere, and use the tweaked @@ -148,7 +208,7 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, // For existing change outputs, we'll just update the amount // since we might not have known what coin would've been // selected and how large the change would turn out to be. - changeOut.Amount = totalInputAmt - fundDesc.Amount + changeOut.Amount = changeAmount // The asset version of the output should be the max of the set // of input versions. We need to set this now as in @@ -170,35 +230,7 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, changeOut.AssetVersion = fn.Reduce(vPkt.Inputs, maxVersion) } - // Before we can prepare output assets for our send, we need to generate - // a new internal key for the anchor outputs. We assume any output that - // hasn't got an internal key set is going to a local anchor, and we - // provide the internal key for that. - for idx := range vPkt.Outputs { - vOut := vPkt.Outputs[idx] - if vOut.AnchorOutputInternalKey != nil { - continue - } - - newInternalKey, err := keyRing.DeriveNextKey( - ctx, asset.TaprootAssetsKeyFamily, - ) - if err != nil { - return nil, err - } - vOut.SetAnchorInternalKey( - newInternalKey, vPkt.ChainParams.HDCoinType, - ) - } - - if err := tapsend.PrepareOutputAssets(ctx, vPkt); err != nil { - return nil, fmt.Errorf("unable to prepare outputs: %w", err) - } - - return &FundedVPacket{ - VPackets: []*tappsbt.VPacket{vPkt}, - InputCommitments: inputCommitments, - }, nil + return nil } // setVPacketInputs sets the inputs of the given vPkt to the given send eligible From b47fe32d5bb5589cb8f60f9aa971895bdc4f7c0a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:45 +0100 Subject: [PATCH 15/16] tapfreighter: simplify createChangeOutput function --- tapfreighter/fund.go | 141 ++++++++++++++++++++++--------------------- 1 file changed, 73 insertions(+), 68 deletions(-) diff --git a/tapfreighter/fund.go b/tapfreighter/fund.go index d724e4ec7..334e593b4 100644 --- a/tapfreighter/fund.go +++ b/tapfreighter/fund.go @@ -152,83 +152,88 @@ func annotateLocalScriptKeys(ctx context.Context, vPkt *tappsbt.VPacket, func createChangeOutput(ctx context.Context, vPkt *tappsbt.VPacket, keyRing KeyRing, fullValue bool, changeAmount uint64) error { + // If we're spending the full value, we don't need a change output. We + // currently assume that if it's a full-value non-interactive spend that + // the packet was created with the correct function in the tappsbt + // packet that adds the NUMS script key output for the tombstone. If + // the user doesn't set that, then an error will be returned from the + // tapsend.PrepareOutputAssets function. But we should probably change + // that and allow the user to specify a minimum packet template and add + // whatever else is needed to it automatically. + if fullValue { + return nil + } + // We expect some change back, or have passive assets to commit to, so // let's make sure we create a transfer output. - var ( - changeOut *tappsbt.VOutput - err error - ) - if !fullValue { - // Do we need to add a change output? - changeOut, err = vPkt.SplitRootOutput() - if err != nil { - lastOut := vPkt.Outputs[len(vPkt.Outputs)-1] - splitOutIndex := lastOut.AnchorOutputIndex + 1 - changeOut = &tappsbt.VOutput{ - Type: tappsbt.TypeSplitRoot, - Interactive: lastOut.Interactive, - AnchorOutputIndex: splitOutIndex, - - // We want to handle deriving a real key in a - // generic manner, so we'll do that just below. - ScriptKey: asset.NUMSScriptKey, - } - - vPkt.Outputs = append(vPkt.Outputs, changeOut) + changeOut, err := vPkt.SplitRootOutput() + if err != nil { + lastOut := vPkt.Outputs[len(vPkt.Outputs)-1] + splitOutIndex := lastOut.AnchorOutputIndex + 1 + changeOut = &tappsbt.VOutput{ + Type: tappsbt.TypeSplitRoot, + Interactive: lastOut.Interactive, + AnchorOutputIndex: splitOutIndex, + + // We want to handle deriving a real key in a + // generic manner, so we'll do that just below. + ScriptKey: asset.NUMSScriptKey, } - // Since we know we're going to receive some change back, we - // need to make sure it is going to an address that we control. - // This should only be the case where we create the default - // change output with the NUMS key to avoid deriving too many - // keys prematurely. We don't need to derive a new key if we - // only have passive assets to commit to, since they all have - // their own script key and the output is more of a placeholder - // to attach the passive assets to. - unSpendable, err := changeOut.ScriptKey.IsUnSpendable() + vPkt.Outputs = append(vPkt.Outputs, changeOut) + } + + // Since we know we're going to receive some change back, we + // need to make sure it is going to an address that we control. + // This should only be the case where we create the default + // change output with the NUMS key to avoid deriving too many + // keys prematurely. We don't need to derive a new key if we + // only have passive assets to commit to, since they all have + // their own script key and the output is more of a placeholder + // to attach the passive assets to. + unSpendable, err := changeOut.ScriptKey.IsUnSpendable() + if err != nil { + return fmt.Errorf("cannot determine if script key is "+ + "spendable: %w", err) + } + if unSpendable { + changeScriptKey, err := keyRing.DeriveNextKey( + ctx, asset.TaprootAssetsKeyFamily, + ) if err != nil { - return fmt.Errorf("cannot determine if script key is "+ - "spendable: %w", err) - } - if unSpendable { - changeScriptKey, err := keyRing.DeriveNextKey( - ctx, asset.TaprootAssetsKeyFamily, - ) - if err != nil { - return err - } - - // We'll assume BIP-0086 everywhere, and use the tweaked - // key from here on out. - changeOut.ScriptKey = asset.NewScriptKeyBip86( - changeScriptKey, - ) + return err } - // For existing change outputs, we'll just update the amount - // since we might not have known what coin would've been - // selected and how large the change would turn out to be. - changeOut.Amount = changeAmount - - // The asset version of the output should be the max of the set - // of input versions. We need to set this now as in - // PrepareOutputAssets locators are created which includes the - // version from the vOut. If we don't set it here, a v1 asset - // spent that becomes change will be a v0 if combined with such - // inputs. - // - // TODO(roasbeef): remove as not needed? - maxVersion := func(maxVersion asset.Version, - vInput *tappsbt.VInput) asset.Version { - - if vInput.Asset().Version > maxVersion { - return vInput.Asset().Version - } - - return maxVersion + // We'll assume BIP-0086 everywhere, and use the tweaked + // key from here on out. + changeOut.ScriptKey = asset.NewScriptKeyBip86( + changeScriptKey, + ) + } + + // For existing change outputs, we'll just update the amount + // since we might not have known what coin would've been + // selected and how large the change would turn out to be. + changeOut.Amount = changeAmount + + // The asset version of the output should be the max of the set + // of input versions. We need to set this now as in + // PrepareOutputAssets locators are created which includes the + // version from the vOut. If we don't set it here, a v1 asset + // spent that becomes change will be a v0 if combined with such + // inputs. + // + // TODO(roasbeef): remove as not needed? + maxVersion := func(maxVersion asset.Version, + vInput *tappsbt.VInput) asset.Version { + + if vInput.Asset().Version > maxVersion { + return vInput.Asset().Version } - changeOut.AssetVersion = fn.Reduce(vPkt.Inputs, maxVersion) + + return maxVersion } + changeOut.AssetVersion = fn.Reduce(vPkt.Inputs, maxVersion) return nil } From d416a6fb2d785cf8219975bd0ac1beb88e0b59a4 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Feb 2025 16:34:46 +0100 Subject: [PATCH 16/16] tapfreighter: create anchor output keys correctly Once we have multiple asset IDs (and with that multiple distinct virtual packets), we need to make sure that we create our anchor outputs correctly. We need to make sure that the anchor outputs for the same index are actually the same (e.g. same internal key). If we just blindly loop over them and assign new keys, then two virtual outputs that reference the same anchor output index would have different keys and we'd fail a check then attempting to commit those packets to a BTC anchor transaction. --- tapfreighter/fund.go | 151 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 135 insertions(+), 16 deletions(-) diff --git a/tapfreighter/fund.go b/tapfreighter/fund.go index 334e593b4..eb2f6396f 100644 --- a/tapfreighter/fund.go +++ b/tapfreighter/fund.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" + "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcutil/psbt" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" @@ -85,21 +86,11 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, // a new internal key for the anchor outputs. We assume any output that // hasn't got an internal key set is going to a local anchor, and we // provide the internal key for that. - for idx := range vPkt.Outputs { - vOut := vPkt.Outputs[idx] - if vOut.AnchorOutputInternalKey != nil { - continue - } - - newInternalKey, err := keyRing.DeriveNextKey( - ctx, asset.TaprootAssetsKeyFamily, - ) - if err != nil { - return nil, err - } - vOut.SetAnchorInternalKey( - newInternalKey, vPkt.ChainParams.HDCoinType, - ) + packets := []*tappsbt.VPacket{vPkt} + err = generateOutputAnchorInternalKeys(ctx, packets, keyRing) + if err != nil { + return nil, fmt.Errorf("unable to generate output anchor "+ + "internal keys: %w", err) } if err := tapsend.PrepareOutputAssets(ctx, vPkt); err != nil { @@ -107,7 +98,7 @@ func createFundedPacketWithInputs(ctx context.Context, exporter proof.Exporter, } return &FundedVPacket{ - VPackets: []*tappsbt.VPacket{vPkt}, + VPackets: packets, InputCommitments: inputCommitments, }, nil } @@ -238,6 +229,134 @@ func createChangeOutput(ctx context.Context, vPkt *tappsbt.VPacket, return nil } +// vOutAnchor is a helper struct that holds the anchor output information that +// might be set on a virtual output. +type vOutAnchor struct { + internalKey *btcec.PublicKey + derivation []*psbt.Bip32Derivation + trDerivation []*psbt.TaprootBip32Derivation + siblingPreimage *commitment.TapscriptPreimage +} + +// newVOutAnchor creates a new vOutAnchor from the given virtual output. +func newVOutAnchor(vOut *tappsbt.VOutput) vOutAnchor { + return vOutAnchor{ + internalKey: vOut.AnchorOutputInternalKey, + derivation: vOut.AnchorOutputBip32Derivation, + trDerivation: vOut.AnchorOutputTaprootBip32Derivation, + siblingPreimage: vOut.AnchorOutputTapscriptSibling, + } +} + +// applyFields applies the anchor output information from the given vOutAnchor +// to the given virtual output. +func (a vOutAnchor) applyFields(vOut *tappsbt.VOutput) { + vOut.AnchorOutputInternalKey = a.internalKey + vOut.AnchorOutputBip32Derivation = a.derivation + vOut.AnchorOutputTaprootBip32Derivation = a.trDerivation + vOut.AnchorOutputTapscriptSibling = a.siblingPreimage +} + +// generateOutputAnchorInternalKeys generates internal keys for the anchor +// outputs of the given virtual packets. If an output already has an internal +// key set, it will be used. If not, a new key will be derived and set. +// At the same time we make sure that we don't use different keys for the same +// anchor output index in case there are multiple packets. +func generateOutputAnchorInternalKeys(ctx context.Context, + packets []*tappsbt.VPacket, keyRing KeyRing) error { + + // We need to make sure we don't use different keys for the same anchor + // output index in case there are multiple packets. So we'll keep track + // of any set keys here. This will be a merged set of existing and new + // keys. + anchorKeys := make(map[uint32]vOutAnchor) + + // extractAnchorKey is a helper function that extracts the anchor key + // from a virtual output and makes sure it is consistent with the + // existing anchor keys from previous outputs of the same or different + // packets. + extractAnchorKey := func(vOut *tappsbt.VOutput) error { + if vOut.AnchorOutputInternalKey == nil { + return nil + } + + anchorIndex := vOut.AnchorOutputIndex + anchorKey := vOut.AnchorOutputInternalKey + + // Handle the case where we already have an anchor defined for + // this index. + if _, ok := anchorKeys[anchorIndex]; ok { + existingPubKey := anchorKeys[anchorIndex].internalKey + if !existingPubKey.IsEqual(anchorKey) { + return fmt.Errorf("anchor output index %d "+ + "already has a different internal key "+ + "set: %x", anchorIndex, + existingPubKey.SerializeCompressed()) + } + + // The keys are the same, so this is already correct. + return nil + } + + // There is no anchor yet, so we add it to the map. + anchorKeys[anchorIndex] = newVOutAnchor(vOut) + + return nil + } + + // Do a first pass through all packets to collect all existing anchor + // keys. At the same time we make sure we don't already have diverging + // information. + for _, vPkt := range packets { + for _, vOut := range vPkt.Outputs { + if err := extractAnchorKey(vOut); err != nil { + return err + } + } + } + + // We now do a second pass through all packets and set the internal keys + // for all outputs that don't have one yet. If we don't have any key for + // an output index, we create a new one. + // nolint: lll + for _, vPkt := range packets { + for idx := range vPkt.Outputs { + vOut := vPkt.Outputs[idx] + anchorIndex := vOut.AnchorOutputIndex + + // Skip any outputs that already have an internal key. + if vOut.AnchorOutputInternalKey != nil { + continue + } + + // Check if we can use an existing key for this output + // index. + existingAnchor, ok := anchorKeys[anchorIndex] + if ok { + existingAnchor.applyFields(vOut) + + continue + } + + newInternalKey, err := keyRing.DeriveNextKey( + ctx, asset.TaprootAssetsKeyFamily, + ) + if err != nil { + return err + } + vOut.SetAnchorInternalKey( + newInternalKey, vPkt.ChainParams.HDCoinType, + ) + + // Store this anchor information in case we have other + // outputs in other packets that need it. + anchorKeys[anchorIndex] = newVOutAnchor(vOut) + } + } + + return nil +} + // setVPacketInputs sets the inputs of the given vPkt to the given send eligible // commitments. It also returns the assets that were used as inputs. func setVPacketInputs(ctx context.Context, exporter proof.Exporter,