descriptors/miniscript: detect time lock mixing

An expression that requires both a height-based and a time-based time
lock of the same kind on one spending path has a path that can never be
satisfied, because the one nSequence (or nLockTime) of the input can only
be interpreted as one of the two. Such an expression is legal miniscript
but a mistake in practice, so this pass tracks which kinds a satisfaction
may need and marks the combination.

descriptors/miniscript/miniscript.go

@@ -273,6 +273,8 @@ func (p properties) String() string {
 //  2. computeOpCount: Counts the amount of opcodes the script contains.
 //  3. computeStackSize: Computes the maximum witness stack size needed to
 //     (dis)satisfy the script.
+//  1. computeTimelocks: Computes the time lock info used to detect time lock
+//     mixing.
 func ParseInsane(miniscript string, ctx Context) (*AST, error) {
 	node, err := createAST(miniscript, ctx)
 	if err != nil {
@@ -302,6 +304,7 @@ func ParseInsane(miniscript string, ctx Context) (*AST, error) {
 		computeStackSize,
 		computeSatSize,
 		computeExecStack,
+		computeTimelocks,
 	}
 	for _, transform := range transformers {
 		node, err = node.apply(transform)
@@ -353,6 +356,11 @@ type AST struct {
 	// execution (beyond the initial witness) to satisfy or dissatisfy this
 	// node. It is only used in the P2TR context.
 	execStack execSize
+
+	// timelock tracks the height- and time-based time locks that may be
+	// encountered when satisfying this node, used to detect time lock
+	// mixing.
+	timelock timelockInfo
 }
 
 // formattedType returns the basic type (B, V, K or W) followed by all type

descriptors/miniscript/timelocks.go

@@ -0,0 +1,154 @@
+package miniscript
+
+import (
+	"github.com/btcsuite/btcd/txscript/v2"
+	"github.com/btcsuite/btcd/wire/v2"
+)
+
+// timelockInfo tracks which kinds of time locks may be encountered while
+// satisfying a (sub)expression. Absolute time locks come from the `after`
+// fragment (OP_CHECKLOCKTIMEVERIFY), relative time locks from the `older`
+// fragment (OP_CHECKSEQUENCEVERIFY). Each is either expressed in block heights
+// or in units of time.
+//
+// It is a port of rust-miniscript's `TimelockInfo` and is used to detect time
+// lock mixing: an expression that requires both a height-based and a time-based
+// lock of the same kind on a single spending path has a branch that can never
+// be satisfied, see
+// https://medium.com/blockstream/dont-mix-your-timelocks-d9939b665094.
+type timelockInfo struct {
+	// csvWithHeight is set if the expression contains a relative,
+	// block-height-based time lock (`older` with the type flag unset).
+	csvWithHeight bool
+
+	// csvWithTime is set if the expression contains a relative, time-based
+	// time lock (`older` with the type flag set).
+	csvWithTime bool
+
+	// cltvWithHeight is set if the expression contains an absolute,
+	// block-height-based time lock (`after` with a value below the
+	// threshold).
+	cltvWithHeight bool
+
+	// cltvWithTime is set if the expression contains an absolute,
+	// time-based time lock (`after` with a value at or above the
+	// threshold).
+	cltvWithTime bool
+
+	// containsCombination is set if some spending path within the
+	// expression mixes a height-based and a time-based lock of the same
+	// kind, which makes that path unspendable.
+	containsCombination bool
+}
+
+// combineTimelocks folds the time lock info of the sub expressions of a
+// fragment into a single value. If more than one of the sub expressions can be
+// required simultaneously (k > 1, i.e. a conjunction), height and time locks of
+// the same kind that end up on the same spending path are flagged in
+// containsCombination.
+//
+// This is a port of rust-miniscript's `TimelockInfo::combine_threshold`.
+func combineTimelocks(k int, subs ...timelockInfo) timelockInfo {
+	var acc timelockInfo
+	for _, t := range subs {
+		// If more than one branch may be taken at once, and this branch
+		// has a requirement that conflicts with one already
+		// accumulated, the combined path is unspendable.
+		if k > 1 {
+			heightAndTime := (acc.csvWithHeight && t.csvWithTime) ||
+				(acc.csvWithTime && t.csvWithHeight) ||
+				(acc.cltvWithTime && t.cltvWithHeight) ||
+				(acc.cltvWithHeight && t.cltvWithTime)
+
+			acc.containsCombination =
+				acc.containsCombination || heightAndTime
+		}
+		acc.csvWithHeight = acc.csvWithHeight || t.csvWithHeight
+		acc.csvWithTime = acc.csvWithTime || t.csvWithTime
+		acc.cltvWithHeight = acc.cltvWithHeight || t.cltvWithHeight
+		acc.cltvWithTime = acc.cltvWithTime || t.cltvWithTime
+		acc.containsCombination =
+			acc.containsCombination || t.containsCombination
+	}
+
+	return acc
+}
+
+// combineTimelocksAnd combines the time lock info of two sub expressions that
+// are both required (logical and).
+func combineTimelocksAnd(a, b timelockInfo) timelockInfo {
+	return combineTimelocks(2, a, b)
+}
+
+// combineTimelocksOr combines the time lock info of two sub expressions of
+// which only one is required (logical or).
+func combineTimelocksOr(a, b timelockInfo) timelockInfo {
+	return combineTimelocks(1, a, b)
+}
+
+// computeTimelocks computes the timelockInfo of a node from that of its
+// children. It is applied bottom-up as part of Parse.
+func computeTimelocks(node *AST) (*AST, error) {
+	switch node.identifier {
+	case f_after:
+		// Absolute time lock (OP_CHECKLOCKTIMEVERIFY, BIP65). Values
+		// below the threshold are block heights, values at or above it
+		// are Unix time stamps.
+		n := node.args[0].num
+		node.timelock = timelockInfo{
+			cltvWithHeight: n < uint64(txscript.LockTimeThreshold),
+			cltvWithTime:   n >= uint64(txscript.LockTimeThreshold),
+		}
+
+	case f_older:
+		// Relative time lock (OP_CHECKSEQUENCEVERIFY, BIP68). The type
+		// flag (bit 22) selects time-based over height-based locks.
+		n := node.args[0].num
+		isTime := n&uint64(wire.SequenceLockTimeIsSeconds) != 0
+		node.timelock = timelockInfo{
+			csvWithHeight: !isTime,
+			csvWithTime:   isTime,
+		}
+
+	case f_and_v, f_and_b:
+		node.timelock = combineTimelocksAnd(
+			node.args[0].timelock, node.args[1].timelock,
+		)
+
+	case f_or_b, f_or_c, f_or_d, f_or_i:
+		node.timelock = combineTimelocksOr(
+			node.args[0].timelock, node.args[1].timelock,
+		)
+
+	case f_andor:
+		// andor(X, Y, Z) is or(and(X, Y), Z).
+		node.timelock = combineTimelocksOr(
+			combineTimelocksAnd(
+				node.args[0].timelock, node.args[1].timelock,
+			),
+			node.args[2].timelock,
+		)
+
+	case f_thresh:
+		k := int(node.args[0].num)
+		subs := make([]timelockInfo, 0, len(node.args)-1)
+		for _, arg := range node.args[1:] {
+			subs = append(subs, arg.timelock)
+		}
+		node.timelock = combineTimelocks(k, subs...)
+
+	case f_wrap_a, f_wrap_s, f_wrap_c, f_wrap_d, f_wrap_v, f_wrap_j,
+		f_wrap_n:
+
+		// Wrappers do not change the time lock semantics of their
+		// child.
+		node.timelock = node.args[0].timelock
+
+	default:
+		// The remaining leaves (0, 1, pk_k, pk_h, multi and the hash
+		// fragments) contain no time locks.
+		node.timelock = timelockInfo{}
+	}
+
+	return node, nil
+}

descriptors/miniscript/timelocks_test.go

@@ -0,0 +1,123 @@
+package miniscript
+
+import (
+	"testing"
+
+	"github.com/btcsuite/btcd/txscript/v2"
+	"github.com/btcsuite/btcd/wire/v2"
+	"github.com/stretchr/testify/require"
+)
+
+// TestCombineTimelocks checks the time-lock folding used to detect time-lock
+// mixing. A conjunction (k > 1) that puts a height-based and a time-based lock
+// of the same kind on one spending path flags an unspendable combination; a
+// disjunction (k == 1) never does. The individual lock flags are always OR'd
+// across the sub expressions.
+func TestCombineTimelocks(t *testing.T) {
+	t.Parallel()
+
+	csvHeight := timelockInfo{csvWithHeight: true}
+	csvTime := timelockInfo{csvWithTime: true}
+	cltvHeight := timelockInfo{cltvWithHeight: true}
+	cltvTime := timelockInfo{cltvWithTime: true}
+	combined := timelockInfo{containsCombination: true}
+
+	tests := []struct {
+		name            string
+		k               int
+		subs            []timelockInfo
+		wantCombination bool
+	}{{
+		name:            "and mixes relative height and time",
+		k:               2,
+		subs:            []timelockInfo{csvHeight, csvTime},
+		wantCombination: true,
+	}, {
+		name:            "and mixes absolute height and time",
+		k:               2,
+		subs:            []timelockInfo{cltvHeight, cltvTime},
+		wantCombination: true,
+	}, {
+		name:            "or does not mix",
+		k:               1,
+		subs:            []timelockInfo{csvHeight, csvTime},
+		wantCombination: false,
+	}, {
+		name:            "relative and absolute do not conflict",
+		k:               2,
+		subs:            []timelockInfo{csvHeight, cltvTime},
+		wantCombination: false,
+	}, {
+		name:            "same subtype does not conflict",
+		k:               2,
+		subs:            []timelockInfo{csvHeight, csvHeight},
+		wantCombination: false,
+	}, {
+		name:            "threshold k>1 mixes",
+		k:               3,
+		subs:            []timelockInfo{csvHeight, csvTime, {}},
+		wantCombination: true,
+	}, {
+		name:            "existing combination propagates under or",
+		k:               1,
+		subs:            []timelockInfo{combined, {}},
+		wantCombination: true,
+	}}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := combineTimelocks(tc.k, tc.subs...)
+			require.Equal(
+				t, tc.wantCombination,
+				got.containsCombination,
+			)
+		})
+	}
+
+	// The and/or helpers are thin wrappers over combineTimelocks with k=2
+	// and k=1 respectively.
+	require.True(t, combineTimelocksAnd(
+		csvHeight, csvTime,
+	).containsCombination)
+	require.False(t, combineTimelocksOr(
+		csvHeight, csvTime,
+	).containsCombination)
+
+	// The individual lock flags are OR'd across all sub expressions.
+	got := combineTimelocksOr(csvHeight, csvTime)
+	require.True(t, got.csvWithHeight)
+	require.True(t, got.csvWithTime)
+}
+
+// TestComputeTimelocksLeaf checks that the after and older leaves land in the
+// right lock category: after uses the absolute-locktime threshold to
+// distinguish block heights from Unix time, older uses the BIP68 seconds flag.
+func TestComputeTimelocksLeaf(t *testing.T) {
+	t.Parallel()
+
+	leaf := func(id string, num uint64) timelockInfo {
+		node := &AST{identifier: id, args: []*AST{{num: num}}}
+		out, err := computeTimelocks(node)
+		require.NoError(t, err)
+		return out.timelock
+	}
+
+	threshold := uint64(txscript.LockTimeThreshold)
+	secondsBit := uint64(wire.SequenceLockTimeIsSeconds)
+
+	require.Equal(
+		t, timelockInfo{cltvWithHeight: true}, leaf(f_after, 100),
+	)
+	require.Equal(
+		t, timelockInfo{cltvWithTime: true}, leaf(f_after, threshold),
+	)
+	require.Equal(
+		t, timelockInfo{csvWithHeight: true}, leaf(f_older, 100),
+	)
+	require.Equal(
+		t, timelockInfo{csvWithTime: true},
+		leaf(f_older, secondsBit|100),
+	)
+}