Skip to content

Commit

Permalink
commitment: splits shouldn't inherit locktime from inputs
Browse files Browse the repository at this point in the history
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 #1099
  • Loading branch information
Roasbeef committed Aug 23, 2024
1 parent 8eb6c53 commit 523d723
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
40 changes: 40 additions & 0 deletions commitment/commitment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions commitment/split.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 523d723

Please sign in to comment.