From 523d7232cf8618c3977be9640a52eb0c83ece7fc Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Thu, 22 Aug 2024 18:24:10 -0700
Subject: [PATCH] commitment: splits shouldn't inherit locktime from inputs

In this commit, we fix an existing bug related to lock times and split
commitments. Before this commit, if an input had a relative lock time,
then when we went to make the new split assets, we would _copy_ that
value into the split. This isn't correct as the input is
valid/confirmed, so we don't need to copy over the lock time
information.

The prior behavior would cause certain classes of spends to fail, as
we would be validating a new root asset that has no lock time, but the
root asset split inserted into the split commitment would be carrying
the old lock time. When verifying the split, we would set the lock times
of the split to that of the new asset:
https://github.com/lightninglabs/taproot-assets/blob/e893dee87e9d8f0de53b8ee9e2527add80df6491/vm/vm.go#L305-L307.

As we copied over the lock time from the input, we would now effectively
invalid the split commitment.

Fixes https://github.com/lightninglabs/taproot-assets/issues/1099
---
 commitment/commitment_test.go | 40 +++++++++++++++++++++++++++++++++++
 commitment/split.go           |  6 ++++++
 2 files changed, 46 insertions(+)

diff --git a/commitment/commitment_test.go b/commitment/commitment_test.go
index 0325001d4..db0fd4625 100644
--- a/commitment/commitment_test.go
+++ b/commitment/commitment_test.go
@@ -656,6 +656,41 @@ 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 +906,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 {
diff --git a/commitment/split.go b/commitment/split.go
index 91e0e32e4..bb4d20b97 100644
--- a/commitment/split.go
+++ b/commitment/split.go
@@ -210,6 +210,12 @@ func NewSplitCommitment(ctx context.Context, inputs []SplitCommitmentInput,
 		}}
 		assetSplit.SplitCommitmentRoot = nil
 
+		// We'll also make sure to clear out the lock time and relative
+		// from the input. The input at this point is already valid, so
+		// we don't need to inherit the time lock encumbrance.
+		assetSplit.RelativeLockTime = 0
+		assetSplit.LockTime = 0
+
 		splitAssets[*locator] = &SplitAsset{
 			Asset:       *assetSplit,
 			OutputIndex: locator.OutputIndex,