Merge pull request #1103 from lightninglabs/fix-split-locktime

commitment: splits shouldn't inherit locktime from inputs

Author: Roasbeef

asset/asset.go

@@ -1831,6 +1831,26 @@ func (a *Asset) Copy() *Asset {
 	return &assetCopy
 }
 
+// CopySpendTemplate is similar to Copy, but should be used when wanting to
+// spend an input asset in a new transaction. Compared to Copy, this method
+// also blanks out some other fields that shouldn't always be carried along for
+// a dependent spend.
+func (a *Asset) CopySpendTemplate() *Asset {
+	assetCopy := a.Copy()
+
+	// We nil out the split commitment root, as we don't need to carry that
+	// into the next spend.
+	assetCopy.SplitCommitmentRoot = nil
+
+	// We'll also make sure to clear out the lock time and relative lock
+	// time from the input. The input at this point is already valid, so we
+	// don't need to inherit the time lock encumbrance.
+	assetCopy.RelativeLockTime = 0
+	assetCopy.LockTime = 0
+
+	return assetCopy
+}
+
 // DeepEqual returns true if this asset is equal with the given asset.
 func (a *Asset) DeepEqual(o *Asset) bool {
 	return a.deepEqual(false, o)

asset/asset_test.go

@@ -1194,3 +1194,25 @@ func TestNewAssetWithCustomVersion(t *testing.T) {
 
 	require.Equal(t, int(assetCustomVersion.Version), newVersion)
 }
+
+// TestCopySpendTemplate tests that the spend template is copied correctly.
+func TestCopySpendTemplate(t *testing.T) {
+	newAsset := RandAsset(t, Normal)
+	newAsset.SplitCommitmentRoot = mssmt.NewComputedNode(hashBytes1, 1337)
+	newAsset.RelativeLockTime = 1
+	newAsset.LockTime = 2
+
+	// The template should have the relevant set of fields blanked.
+	spendTemplate := newAsset.CopySpendTemplate()
+	require.Zero(t, spendTemplate.SplitCommitmentRoot)
+	require.Zero(t, spendTemplate.RelativeLockTime)
+	require.Zero(t, spendTemplate.LockTime)
+
+	// If blank these fields of the OG asset, then things should be
+	// identical.
+	newAsset.SplitCommitmentRoot = nil
+	newAsset.RelativeLockTime = 0
+	newAsset.LockTime = 0
+
+	require.True(t, newAsset.DeepEqual(spendTemplate))
+}

commitment/commitment_test.go

@@ -656,6 +656,43 @@ func TestSplitCommitment(t *testing.T) {
 			},
 			err: nil,
 		},
+		{
+			name: "single input split commitment lock time input",
+			f: func() (*asset.Asset, *SplitLocator,
+				[]*SplitLocator) {
+
+				input := randAsset(
+					t, genesisNormal, groupKeyNormal,
+				)
+				input.Amount = 3
+				input.RelativeLockTime = 1
+				input.LockTime = 1
+
+				root := &SplitLocator{
+					OutputIndex: 0,
+					AssetID:     genesisNormal.ID(),
+					ScriptKey: asset.ToSerialized(
+						input.ScriptKey.PubKey,
+					),
+					Amount: 1,
+				}
+				external := []*SplitLocator{{
+					OutputIndex: 1,
+					AssetID:     genesisNormal.ID(),
+					ScriptKey:   asset.RandSerializedKey(t),
+					Amount:      1,
+				}, {
+
+					OutputIndex: 2,
+					AssetID:     genesisNormal.ID(),
+					ScriptKey:   asset.RandSerializedKey(t),
+					Amount:      1,
+				}}
+
+				return input, root, external
+			},
+			err: nil,
+		},
 		{
 			name: "no external splits",
 			f: func() (*asset.Asset, *SplitLocator, []*SplitLocator) {
@@ -871,6 +908,11 @@ func TestSplitCommitment(t *testing.T) {
 				require.Contains(t, split.SplitAssets, *l)
 				splitAsset := split.SplitAssets[*l]
 
+				// Make sure that the splits don't inherit lock
+				// time information from the root asset.
+				require.Zero(t, splitAsset.LockTime)
+				require.Zero(t, splitAsset.RelativeLockTime)
+
 				// If this is a leaf split, then we need to
 				// ensure that the prev ID is zero.
 				if splitAsset.SplitCommitmentRoot == nil {

commitment/split.go

@@ -194,7 +194,7 @@ func NewSplitCommitment(ctx context.Context, inputs []SplitCommitmentInput,
 	remainingAmount := totalInputAmount
 	rootIdx := len(locators) - 1
 	addAssetSplit := func(locator *SplitLocator) error {
-		assetSplit := inputs[0].Asset.Copy()
+		assetSplit := inputs[0].Asset.CopySpendTemplate()
 		assetSplit.Amount = locator.Amount
 		assetSplit.Version = locator.AssetVersion
 
@@ -208,7 +208,6 @@ func NewSplitCommitment(ctx context.Context, inputs []SplitCommitmentInput,
 			TxWitness:       nil,
 			SplitCommitment: nil,
 		}}
-		assetSplit.SplitCommitmentRoot = nil
 
 		splitAssets[*locator] = &SplitAsset{
 			Asset:       *assetSplit,

proof/courier.go

@@ -1473,7 +1473,11 @@ func (c *UniverseRpcCourier) publishSubscriberEvent(event fn.Event) {
 
 // Close closes the courier's connection to the remote gRPC service.
 func (c *UniverseRpcCourier) Close() error {
-	return c.rawConn.Close()
+	if c.rawConn != nil {
+		return c.rawConn.Close()
+	}
+
+	return nil
 }
 
 // A compile-time assertion to ensure the UniverseRpcCourier meets the

proof/verifier.go

@@ -375,18 +375,20 @@ func CreateOwnershipProofAsset(ownedAsset *asset.Asset,
 		),
 	}
 
-	outputAsset := ownedAsset.Copy()
-	outputAsset.ScriptKey = address.GenChallengeNUMS(challengeBytes)
-	outputAsset.PrevWitnesses = []asset.Witness{{
-		PrevID: &prevId,
-	}}
-
 	// The ownership proof needs to be a 1-in-1-out transaction. So it will
 	// definitely not have a split commitment. Keeping the split commitment
 	// of the copied owned asset would lead to an issue with the
 	// non-inflation check we have in the VM that takes the split commitment
-	// root sum as the expected total output amount.
-	outputAsset.SplitCommitmentRoot = nil
+	// root sum as the expected total output amount. We also clear any time
+	// locks, as they don't apply to the ownership proof.
+	//
+	// This is handled by CopySpendTemplate.
+	outputAsset := ownedAsset.CopySpendTemplate()
+
+	outputAsset.ScriptKey = address.GenChallengeNUMS(challengeBytes)
+	outputAsset.PrevWitnesses = []asset.Witness{{
+		PrevID: &prevId,
+	}}
 
 	return prevId, outputAsset
 }

tapfreighter/wallet.go

@@ -309,11 +309,7 @@ func createPassivePacket(params *address.ChainParams, passiveAsset *asset.Asset,
 	}
 
 	// Specify virtual output.
-	outputAsset := passiveAsset.Copy()
-
-	// Clear the split commitment root, as we'll be transferring the whole
-	// asset.
-	outputAsset.SplitCommitmentRoot = nil
+	outputAsset := passiveAsset.CopySpendTemplate()
 
 	// Clear the output asset witness data. We'll be creating a new witness.
 	outputAsset.PrevWitnesses = []asset.Witness{{