Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[tapsend]: Enforce unique script keys #1181

Merged
merged 10 commits into from
Nov 21, 2024
5 changes: 4 additions & 1 deletion itest/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,10 @@ func AssertAssetOutboundTransferWithOutputs(t *testing.T,
out := transfer.Outputs[idx]
require.Contains(t, outpoints, out.Anchor.Outpoint)
require.Contains(t, scripts, string(out.ScriptKey))
require.Equal(t, expectedAmounts[idx], out.Amount)
require.Equalf(
t, expectedAmounts[idx], out.Amount,
"expected amounts: %v, transfer: %v",
expectedAmounts, toJSON(t, transfer))
}

firstIn := transfer.Inputs[0]
Expand Down
24 changes: 13 additions & 11 deletions itest/fee_estimation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ func testFeeEstimation(t *harnessTest) {
rpcAssets := MintAssetsConfirmBatch(
t.t, t.lndHarness.Miner().Client, t.tapd, simpleAssets,
)
normalAsset := rpcAssets[0]
normalAssetId := normalAsset.AssetGenesis.AssetId

// Check the final fee rate of the mint TX.
rpcMintOutpoint := rpcAssets[0].ChainAnchor.AnchorOutpoint
rpcMintOutpoint := normalAsset.ChainAnchor.AnchorOutpoint
mintOutpoint, err := wire.NewOutPointFromString(rpcMintOutpoint)
require.NoError(t.t, err)

Expand All @@ -81,16 +83,15 @@ func testFeeEstimation(t *harnessTest) {
)

// Split the normal asset to create a transfer with two anchor outputs.
normalAssetId := rpcAssets[0].AssetGenesis.AssetId
splitAmount := rpcAssets[0].Amount / 2
splitAmount := normalAsset.Amount / 2
addr, stream := NewAddrWithEventStream(
t.t, t.tapd, &taprpc.NewAddrRequest{
AssetId: normalAssetId,
Amt: splitAmount,
},
)

AssertAddrCreated(t.t, t.tapd, rpcAssets[0], addr)
AssertAddrCreated(t.t, t.tapd, normalAsset, addr)
sendResp, sendEvents := sendAssetsToAddr(t, t.tapd, addr)

transferIdx := 0
Expand Down Expand Up @@ -121,7 +122,7 @@ func testFeeEstimation(t *harnessTest) {
},
)

AssertAddrCreated(t.t, t.tapd, rpcAssets[0], addr2)
AssertAddrCreated(t.t, t.tapd, normalAsset, addr2)
sendResp, sendEvents = sendAssetsToAddr(t, t.tapd, addr2)

ConfirmAndAssertOutboundTransfer(
Expand Down Expand Up @@ -157,7 +158,7 @@ func testFeeEstimation(t *harnessTest) {
[]*UTXORequest{initialUTXOs[3]},
)

AssertAddrCreated(t.t, t.tapd, rpcAssets[0], addr3)
AssertAddrCreated(t.t, t.tapd, normalAsset, addr3)
_, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
TapAddrs: []string{addr3.Encoded},
})
Expand All @@ -166,23 +167,24 @@ func testFeeEstimation(t *harnessTest) {
)

// The transfer should also be rejected if the manually-specified
// feerate fails the sanity check against the fee estimator's fee floor
// fee rate fails the sanity check against the fee estimator's fee floor
// of 253 sat/kw, or 1.012 sat/vB.
_, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
TapAddrs: []string{addr3.Encoded},
FeeRate: uint32(chainfee.FeePerKwFloor) - 1,
})
require.ErrorContains(t.t, err, "manual fee rate below floor")

// After failure at the high feerate, we should still be able to make a
// transfer at a very low feerate.
// After failure at the high fee rate, we should still be able to make a
// transfer at a very low fee rate.
t.lndHarness.SetFeeEstimateWithConf(lowFeeRate, 6)
sendResp, sendEvents = sendAssetsToAddr(t, t.tapd, addr3)

ConfirmAndAssertOutboundTransfer(
t.t, t.lndHarness.Miner().Client, t.tapd, sendResp,
normalAssetId, []uint64{thirdSplitAmount, thirdSplitAmount},
transferIdx, transferIdx+1,
normalAssetId, []uint64{
splitAmount - thirdSplitAmount, thirdSplitAmount,
}, transferIdx, transferIdx+1,
)
transferIdx += 1
AssertNonInteractiveRecvComplete(t.t, t.tapd, transferIdx)
Expand Down
11 changes: 11 additions & 0 deletions itest/send_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/lightninglabs/taproot-assets/taprpc/mintrpc"
"github.com/lightninglabs/taproot-assets/taprpc/tapdevrpc"
unirpc "github.com/lightninglabs/taproot-assets/taprpc/universerpc"
"github.com/lightninglabs/taproot-assets/tapsend"
"github.com/lightningnetwork/lnd/lntest/wait"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -102,6 +103,16 @@ func testBasicSendUnidirectional(t *harnessTest) {
})
require.NoError(t.t, err)

// Before we start sending, we test that we aren't allowed to send to
// the same address more than once within the same transfer.
_, err = t.tapd.SendAsset(ctxb, &taprpc.SendAssetRequest{
guggero marked this conversation as resolved.
Show resolved Hide resolved
TapAddrs: []string{
bobAddr.Encoded,
bobAddr.Encoded,
},
})
require.ErrorContains(t.t, err, tapsend.ErrDuplicateScriptKeys.Error())

for i := 0; i < numSends; i++ {
t.t.Logf("Performing send procedure: %d", i)

Expand Down
7 changes: 7 additions & 0 deletions proof/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,8 @@ func TestProofVerification(t *testing.T) {

t.Logf("Proof asset JSON: %s", assetJSON)

// If we have a challenge witness, we can verify that without having the
// previous proof.
if len(p.ChallengeWitness) > 0 {
_, err = p.Verify(
context.Background(), nil, MockHeaderVerifier,
Expand All @@ -766,6 +768,11 @@ func TestProofVerification(t *testing.T) {
require.NoError(t, err)
}

// Verifying the inclusion and exclusion proofs can also be done without
// the previous proof.
_, err = p.VerifyProofs()
require.NoError(t, err)

// Ensure that verification of a proof of unknown version fails.
p.Version = TransitionVersion(212)

Expand Down
51 changes: 50 additions & 1 deletion tapfreighter/chain_porter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1075,9 +1075,14 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
// At this point, we have everything we need to sign our _virtual_
// transaction on the Taproot Asset layer.
case SendStateVirtualSign:
ctx, cancel := p.WithCtxQuitNoTimeout()
defer cancel()

vPackets := currentPkg.VirtualPackets
err := tapsend.ValidateVPacketVersions(vPackets)
if err != nil {
p.unlockInputs(ctx, &currentPkg)

return nil, err
}

Expand All @@ -1091,6 +1096,8 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {

_, err := p.cfg.AssetWallet.SignVirtualPacket(vPkt)
if err != nil {
p.unlockInputs(ctx, &currentPkg)

return nil, fmt.Errorf("unable to sign and "+
"commit virtual packet: %w", err)
}
Expand Down Expand Up @@ -1125,6 +1132,8 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
ctx, tapsend.SendConfTarget,
)
if err != nil {
p.unlockInputs(ctx, &currentPkg)

return nil, fmt.Errorf("unable to estimate "+
"fee: %w", err)
}
Expand All @@ -1148,6 +1157,8 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
currentPkg.InputCommitments,
)
if err != nil {
p.unlockInputs(ctx, &currentPkg)

return nil, fmt.Errorf("unable to create passive "+
"assets: %w", err)
}
Expand All @@ -1156,6 +1167,8 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
len(currentPkg.PassiveAssets))
err = wallet.SignPassiveAssets(currentPkg.PassiveAssets)
if err != nil {
p.unlockInputs(ctx, &currentPkg)

return nil, fmt.Errorf("unable to sign passive "+
"assets: %w", err)
}
Expand All @@ -1168,6 +1181,8 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
},
)
if err != nil {
p.unlockInputs(ctx, &currentPkg)

return nil, fmt.Errorf("unable to anchor virtual "+
"transactions: %w", err)
}
Expand Down Expand Up @@ -1382,10 +1397,44 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {

// unlockInputs unlocks the inputs that were locked for the given package.
func (p *ChainPorter) unlockInputs(ctx context.Context, pkg *sendPackage) {
if pkg == nil || pkg.AnchorTx == nil || pkg.AnchorTx.FundedPsbt == nil {
// Impossible state, but catch it anyway.
if pkg == nil {
return
}

// If we haven't even attempted to broadcast yet, we're still in a state
// where we give feedback to the user synchronously, as we haven't
// created an on-chain transaction that we need to await confirmation.
// We also haven't written the transfer to disk yet, so we can just
// release/unlock the _asset_ level UTXOs so the user can try again. We
// sanity-check that we have known input commitments to unlock, since
// that might not always be the case (for example if another party
// contributes inputs).
if pkg.SendState < SendStateStorePreBroadcast &&
guggero marked this conversation as resolved.
Show resolved Hide resolved
len(pkg.InputCommitments) > 0 {

for prevID := range pkg.InputCommitments {
log.Debugf("Unlocking input %v", prevID.OutPoint)

err := p.cfg.AssetWallet.ReleaseCoins(
ctx, prevID.OutPoint,
)
if err != nil {
log.Warnf("Unable to unlock input %v: %v",
prevID.OutPoint, err)
}
}
}

// If we're in another state, the anchor transaction has been created,
// and we can't simply unlock the asset level inputs. This will likely
// require manual intervention.
if pkg.AnchorTx == nil || pkg.AnchorTx.FundedPsbt == nil {
return
}

// We need to unlock any _BTC_ level inputs we locked for the anchor
// transaction.
for _, op := range pkg.AnchorTx.FundedPsbt.LockedUTXOs {
err := p.cfg.Wallet.UnlockInput(ctx, op)
if err != nil {
Expand Down
10 changes: 8 additions & 2 deletions tapfreighter/coin_select.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,14 @@ func (s *CoinSelect) SelectCoins(ctx context.Context,
return nil, ErrMatchingAssetsNotFound
}

log.Infof("Identified %v eligible asset inputs for send of %d to %v",
len(eligibleCommitments), constraints.MinAmt, constraints)
anchorInputs := fn.Map(
eligibleCommitments, func(c *AnchoredCommitment) string {
return c.AnchorPoint.String()
},
)
log.Infof("Identified %v eligible asset inputs for send of %d to %v: "+
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is counting anchor inputs and not asset inputs?

IIUC the anchor inputs may contain multiple eligible asset inputs, which we will select from later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think if we have multiple eligible assets in the same anchor, we'd still get multiple *AnchoredCommitment structs returned. So IMO this is correct.

"%v", len(anchorInputs), constraints.MinAmt,
constraints.String(), anchorInputs)

// Only select coins anchored in a compatible commitment.
compatibleCommitments := fn.Filter(
Expand Down
31 changes: 27 additions & 4 deletions tapfreighter/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/lightninglabs/taproot-assets/tapsend"
"github.com/lightningnetwork/lnd/keychain"
"github.com/lightningnetwork/lnd/lnwallet/chainfee"
"golang.org/x/exp/maps"
)

const (
Expand Down Expand Up @@ -129,6 +130,10 @@ type Wallet interface {
// address.ErrInternalKeyNotFound is returned.
FetchInternalKeyLocator(ctx context.Context,
rawKey *btcec.PublicKey) (keychain.KeyLocator, error)

// ReleaseCoins releases/unlocks coins that were previously leased and
// makes them available for coin selection again.
ReleaseCoins(ctx context.Context, utxoOutpoints ...wire.OutPoint) error
}

// AddrBook is an interface that provides access to the address book.
Expand Down Expand Up @@ -752,7 +757,7 @@ func (f *AssetWallet) fundPacketWithInputs(ctx context.Context,
}

if err := tapsend.PrepareOutputAssets(ctx, vPkt); err != nil {
return nil, fmt.Errorf("unable to create split commit: %w", err)
return nil, fmt.Errorf("unable to prepare outputs: %w", err)
}

return &FundedVPacket{
Expand Down Expand Up @@ -1206,7 +1211,7 @@ func (f *AssetWallet) CreatePassiveAssets(ctx context.Context,
}

// Gather passive assets found in each input Taproot Asset commitment.
var passivePackets []*tappsbt.VPacket
passivePackets := make(map[asset.PrevID]*tappsbt.VPacket)
for prevID := range inputCommitments {
tapCommitment := inputCommitments[prevID]

Expand Down Expand Up @@ -1244,6 +1249,16 @@ func (f *AssetWallet) CreatePassiveAssets(ctx context.Context,
"proof: %w", err)
}

scriptKey := passiveAsset.ScriptKey.PubKey
passivePrevID := asset.PrevID{
OutPoint: prevID.OutPoint,
ID: passiveAsset.ID(),
ScriptKey: asset.ToSerialized(scriptKey),
}
log.Tracef("Adding passive packet for asset_id=%v, "+
"script_key=%x", passiveAsset.ID().String(),
scriptKey.SerializeCompressed())

passivePacket, err := createPassivePacket(
f.cfg.ChainParams, passiveAsset, activePackets,
anchorOutIdx, *anchorOutDesc, prevID.OutPoint,
Expand All @@ -1254,11 +1269,11 @@ func (f *AssetWallet) CreatePassiveAssets(ctx context.Context,
"passive packet: %w", err)
}

passivePackets = append(passivePackets, passivePacket)
passivePackets[passivePrevID] = passivePacket
}
}

return passivePackets, nil
return maps.Values(passivePackets), nil
}

// SignPassiveAssets signs the given passive asset packets.
Expand Down Expand Up @@ -1462,6 +1477,14 @@ func (f *AssetWallet) FetchInternalKeyLocator(ctx context.Context,
return f.cfg.AddrBook.FetchInternalKeyLocator(ctx, rawKey)
}

// ReleaseCoins releases/unlocks coins that were previously leased and makes
// them available for coin selection again.
func (f *AssetWallet) ReleaseCoins(ctx context.Context,
utxoOutpoints ...wire.OutPoint) error {

return f.cfg.CoinSelector.ReleaseCoins(ctx, utxoOutpoints...)
}

// addAnchorPsbtInputs adds anchor information from all inputs to the PSBT
// packet. This is called after the PSBT has been funded, but before signing.
func addAnchorPsbtInputs(btcPkt *psbt.Packet, vPackets []*tappsbt.VPacket) {
Expand Down
19 changes: 19 additions & 0 deletions tappsbt/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,25 @@ func (p *VPacket) AssetID() (asset.ID, error) {
return firstID, nil
}

// AssetSpecifier returns the asset specifier for the asset being spent by the
// first input of the virtual transaction. It returns an error if the asset ID
// of the virtual transaction is not set or if the asset of the first input is
// not set.
func (p *VPacket) AssetSpecifier() (asset.Specifier, error) {
assetID, err := p.AssetID()
if err != nil {
return asset.Specifier{}, err
}

if p.Inputs[0].Asset() == nil {
return asset.Specifier{}, fmt.Errorf("no asset set for input 0")
}

return asset.NewSpecifier(
&assetID, nil, p.Inputs[0].Asset().GroupKey, true,
)
}

// Anchor is a struct that contains all the information about an anchor output.
type Anchor struct {
// Value is output value of the anchor output.
Expand Down
Loading
Loading