contractcourt: validate timeout second-level outputs

contractcourt/channel_arbitrator_test.go

@@ -984,8 +984,6 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) {
 			},
 		},
 	}
-	closeTxid := closeTx.TxHash()
-
 	htlcOp := wire.OutPoint{
 		Hash:  closeTx.TxHash(),
 		Index: 0,
@@ -1115,10 +1113,12 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) {
 
 	// Notify resolver that the HTLC output of the commitment has been
 	// spent.
+	timeoutTx := outgoingRes.SignedTimeoutTx
+	timeoutTxid := timeoutTx.TxHash()
 	oldNotifier.SpendChan <- &chainntnfs.SpendDetail{
-		SpendingTx:    closeTx,
-		SpentOutPoint: &wire.OutPoint{},
-		SpenderTxHash: &closeTxid,
+		SpendingTx:    timeoutTx,
+		SpentOutPoint: &htlcOp,
+		SpenderTxHash: &timeoutTxid,
 	}
 
 	// Finally, we should also receive a resolution message instructing the
@@ -1147,6 +1147,7 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) {
 	}
 
 	// Notify resolver that the output of the timeout tx has been spent.
+	closeTxid := closeTx.TxHash()
 	oldNotifier.SpendChan <- &chainntnfs.SpendDetail{
 		SpendingTx:    closeTx,
 		SpentOutPoint: &wire.OutPoint{},

contractcourt/htlc_success_resolver.go

@@ -31,6 +31,9 @@ var errInvalidSpendDetails = errors.New("invalid spend details")
 // errInvalidSuccessResolver identifies malformed success resolver state.
 var errInvalidSuccessResolver = errors.New("invalid success resolver")
 
+// errInvalidSecondLevelOutput identifies malformed second-level output data.
+var errInvalidSecondLevelOutput = errors.New("invalid second-level output")
+
 // htlcSuccessResolver is a resolver that's capable of sweeping an incoming
 // HTLC output on-chain. If this is the remote party's commitment, we'll sweep
 // it directly from the commitment output *immediately*. If this is our
@@ -639,9 +642,8 @@ func (h *htlcSuccessResolver) validatedSpendInput(
 //
 // The HTLC input uses SINGLE|ANYONECANPAY, so it commits to the transaction
 // output at the same index. A match returns that output's actual outpoint.
-func (h *htlcSuccessResolver) matchSecondLevelOutput(
-	spendingTx *wire.MsgTx,
-	outputIndex uint32) (wire.OutPoint, bool, error) {
+func matchSecondLevelOutput(spendingTx *wire.MsgTx, outputIndex uint32,
+	expected *wire.TxOut) (wire.OutPoint, bool, error) {
 
 	var zeroOutpoint wire.OutPoint
 	if spendingTx == nil {
@@ -650,11 +652,10 @@ func (h *htlcSuccessResolver) matchSecondLevelOutput(
 		)
 	}
 
-	expected := h.htlcResolution.SweepSignDesc.Output
 	if expected == nil {
 		return zeroOutpoint, false, fmt.Errorf(
-			"%w: missing expected output for %v",
-			errInvalidSuccessResolver, h.outpoint(),
+			"%w: missing expected output",
+			errInvalidSecondLevelOutput,
 		)
 	}
 
@@ -822,8 +823,9 @@ func (h *htlcSuccessResolver) sweepSuccessTxOutput() error {
 	if err != nil {
 		return err
 	}
-	secondLevelOutpoint, matches, err := h.matchSecondLevelOutput(
+	secondLevelOutpoint, matches, err := matchSecondLevelOutput(
 		commitSpend.SpendingTx, commitSpend.SpenderInputIndex,
+		h.htlcResolution.SweepSignDesc.Output,
 	)
 	if err != nil {
 		return err
@@ -978,8 +980,9 @@ func (h *htlcSuccessResolver) resolveSuccessTx() error {
 	if err != nil {
 		return err
 	}
-	secondLevelOutpoint, matches, err := h.matchSecondLevelOutput(
+	secondLevelOutpoint, matches, err := matchSecondLevelOutput(
 		commitSpend.SpendingTx, commitSpend.SpenderInputIndex,
+		h.htlcResolution.SweepSignDesc.Output,
 	)
 	if err != nil {
 		return err

contractcourt/htlc_success_resolver_test.go

@@ -528,11 +528,11 @@ func TestHtlcSuccessSingleStageClassification(t *testing.T) {
 	require.Empty(t, ctx.htlcNotifier.finalHtlcEvents)
 }
 
-// TestHtlcSuccessMatchSecondLevelOutput tests matching the success transaction
-// output against the sweep descriptor.
-func TestHtlcSuccessMatchSecondLevelOutput(t *testing.T) {
+// TestMatchSecondLevelOutput tests matching a second-level transaction output
+// against an expected sweep output.
+func TestMatchSecondLevelOutput(t *testing.T) {
 	claim := wire.OutPoint{Index: 2}
-	newMatch := func() (*htlcSuccessResolver, *wire.MsgTx) {
+	newMatch := func() (*wire.TxOut, *wire.MsgTx) {
 		resolution := newSuccessTestResolution(claim)
 		tx := &wire.MsgTx{
 			TxIn: []*wire.TxIn{
@@ -547,14 +547,14 @@ func TestHtlcSuccessMatchSecondLevelOutput(t *testing.T) {
 			},
 		}
 
-		return &htlcSuccessResolver{
-			htlcResolution: resolution,
-		}, tx
+		return resolution.SweepSignDesc.Output, tx
 	}
 
 	testCases := []struct {
-		name        string
-		prepare     func(*htlcSuccessResolver, *wire.MsgTx) *wire.MsgTx
+		name    string
+		prepare func(
+			*wire.TxOut, *wire.MsgTx,
+		) (*wire.TxOut, *wire.MsgTx)
 		matches     bool
 		expectedErr error
 	}{
@@ -564,73 +564,87 @@ func TestHtlcSuccessMatchSecondLevelOutput(t *testing.T) {
 		},
 		{
 			name: "commitment descriptor decoy",
-			prepare: func(resolver *htlcSuccessResolver,
-				tx *wire.MsgTx) *wire.MsgTx {
+			prepare: func(expected *wire.TxOut,
+				tx *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
 
 				// This decoy proves the matcher uses the sweep
 				// descriptor, not the commitment descriptor.
-				resolution := &resolver.htlcResolution
-				signDetails := resolution.SignDetails
-				tx.TxOut[1] = cloneTxOut(
-					signDetails.SignDesc.Output,
-				)
+				tx.TxOut[1] = cloneTxOut(testSignDesc.Output)
+
+				return expected, tx
+			},
+		},
+		{
+			name: "value mismatch",
+			prepare: func(expected *wire.TxOut,
+				tx *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
 
-				return tx
+				tx.TxOut[1].Value++
+
+				return expected, tx
+			},
+		},
+		{
+			name: "script mismatch",
+			prepare: func(expected *wire.TxOut,
+				tx *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
+
+				tx.TxOut[1].PkScript = []byte{txscript.OP_FALSE}
+
+				return expected, tx
 			},
 		},
 		{
 			name: "missing indexed output",
-			prepare: func(_ *htlcSuccessResolver,
-				tx *wire.MsgTx) *wire.MsgTx {
+			prepare: func(expected *wire.TxOut,
+				tx *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
 
 				tx.TxOut = tx.TxOut[:1]
 
-				return tx
+				return expected, tx
 			},
 		},
 		{
 			name: "missing expected output",
-			prepare: func(resolver *htlcSuccessResolver,
-				tx *wire.MsgTx) *wire.MsgTx {
+			prepare: func(_ *wire.TxOut,
+				tx *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
 
-				resolution := &resolver.htlcResolution
-				resolution.SweepSignDesc.Output = nil
-
-				return tx
+				return nil, tx
 			},
-			expectedErr: errInvalidSuccessResolver,
+			expectedErr: errInvalidSecondLevelOutput,
 		},
 		{
 			name: "nil indexed output",
-			prepare: func(_ *htlcSuccessResolver,
-				tx *wire.MsgTx) *wire.MsgTx {
+			prepare: func(expected *wire.TxOut,
+				tx *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
 
 				tx.TxOut[1] = nil
 
-				return tx
+				return expected, tx
 			},
 			expectedErr: errInvalidSpendDetails,
 		},
 		{
 			name: "nil transaction",
-			prepare: func(_ *htlcSuccessResolver,
-				_ *wire.MsgTx) *wire.MsgTx {
+			prepare: func(expected *wire.TxOut,
+				_ *wire.MsgTx) (*wire.TxOut, *wire.MsgTx) {
 
-				return nil
+				return expected, nil
 			},
 			expectedErr: errInvalidSpendDetails,
 		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			resolver, tx := newMatch()
+			expected, tx := newMatch()
 			if testCase.prepare != nil {
-				tx = testCase.prepare(resolver, tx)
+				expected, tx = testCase.prepare(expected, tx)
 			}
 
-			outpoint, matches, err :=
-				resolver.matchSecondLevelOutput(tx, 1)
+			outpoint, matches, err := matchSecondLevelOutput(
+				tx, 1, expected,
+			)
 			if testCase.expectedErr != nil {
 				require.ErrorIs(t, err, testCase.expectedErr)
 				return