Sign forfeit txs at collection-time

internal/core/application/fraud.go

@@ -211,9 +211,22 @@ func (s *service) broadcastForfeitTx(ctx context.Context, vtxo domain.Vtxo) erro
 		return fmt.Errorf("failed to encode forfeit tx: %s", err)
 	}
 
-	signedForfeitTx, err := s.signer.SignTransactionTapscript(ctx, forfeitTxB64, nil)
-	if err != nil {
-		return fmt.Errorf("failed to sign forfeit tx: %s", err)
+	// Forfeit txs are signed by the operator at collection time, so the stored tx
+	// is usually already broadcast-ready. Re-signing would append a duplicate
+	// operator signature and produce an invalid PSBT (duplicate key), so we only
+	// sign here when a signature is still missing, as on a legacy forfeit stored
+	// without the operator's half.
+	//
+	// Readiness is decided from the psbt alone, without consulting the signer or
+	// the operator's current key set: a pre-signed forfeit must stay broadcastable
+	// even when the signer is down or its key has since been rotated away, which is
+	// the whole point of signing at collection time.
+	signedForfeitTx := forfeitTxB64
+	if !domain.ForfeitTxReadyToBroadcast(forfeitTx) {
+		signedForfeitTx, err = s.signer.SignTransactionTapscript(ctx, forfeitTxB64, nil)
+		if err != nil {
+			return fmt.Errorf("failed to sign forfeit tx: %s", err)
+		}
 	}
 
 	forfeitTxHex, err := s.builder.FinalizeAndExtract(signedForfeitTx)

internal/core/application/service.go

@@ -2306,11 +2306,80 @@ func (s *service) ConfirmRegistration(ctx context.Context, intentId string) erro
 	return nil
 }
 
+// signForfeitTxs adds the operator signature to each collected forfeit tx and
+// returns them as domain.ForfeitTx ready to be persisted. Signing only adds
+// witness data, so the txid is read from the signed psbt's unsigned tx and is
+// identical to the txid of the user-submitted forfeit tx.
+func (s *service) signForfeitTxs(
+	ctx context.Context, forfeitTxs []string,
+) ([]domain.ForfeitTx, error) {
+	signed := make([]domain.ForfeitTx, 0, len(forfeitTxs))
+	for _, tx := range forfeitTxs {
+		signedTx, err := s.signer.SignTransactionTapscript(ctx, tx, nil)
+		if err != nil {
+			return nil, fmt.Errorf("failed to sign forfeit tx: %w", err)
+		}
+		ptx, err := psbt.NewFromRawBytes(strings.NewReader(signedTx), true)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse signed forfeit tx: %w", err)
+		}
+		signed = append(signed, domain.ForfeitTx{
+			Txid: ptx.UnsignedTx.TxID(),
+			Tx:   signedTx,
+		})
+	}
+	return signed, nil
+}
+
+// operatorXOnlyKeys returns the x-only encoding of every signer key the operator
+// signs forfeit txs with: the current one and any deprecated one still accepted.
+func (s *service) operatorXOnlyKeys(ctx context.Context) ([][]byte, error) {
+	settings, err := s.cache.Settings().Get(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get settings: %w", err)
+	}
+	if settings == nil {
+		return nil, fmt.Errorf("settings not available")
+	}
+
+	keys := make([][]byte, 0, 1+len(settings.DeprecatedSignerPubkeys))
+	if settings.SignerPubkey != nil {
+		keys = append(keys, schnorr.SerializePubKey(settings.SignerPubkey))
+	}
+	for _, deprecated := range settings.DeprecatedSignerPubkeys {
+		if deprecated.PubKey != nil {
+			keys = append(keys, schnorr.SerializePubKey(deprecated.PubKey))
+		}
+	}
+	// SignerPubkey is nillable, so an empty set is reachable. Callers use this to
+	// reject forfeits carrying a signature under one of these keys, and an empty
+	// set would make that check quietly pass everything, so fail instead. arkd
+	// cannot sign a forfeit at all in this state.
+	if len(keys) <= 0 {
+		return nil, fmt.Errorf("no operator signer key available")
+	}
+	return keys, nil
+}
+
 func (s *service) SubmitForfeitTxs(ctx context.Context, forfeitTxs []string) errors.Error {
 	if len(forfeitTxs) <= 0 {
 		return nil
 	}
 
+	// A client signs only its own half of a forfeit: the operator's signature is
+	// added by arkd at collection time, and the connector is spent with a wallet
+	// key the client does not hold. A submitted forfeit that already carries
+	// either can only be an attempt to poison the psbt, and it is not harmless:
+	// a planted tapscript sig under an operator key over the same leaf makes the
+	// collection-time signer append a second entry for that (key, leaf) pair,
+	// yielding a duplicate-key psbt that fails to parse and takes the whole round
+	// down with it. Reject it here, before it is ever stored.
+	operatorKeys, keysErr := s.operatorXOnlyKeys(ctx)
+	if keysErr != nil {
+		log.WithError(keysErr).Error("failed to get operator signer keys")
+		return errors.INTERNAL_ERROR.New("something went wrong")
+	}
+
 	for _, b64 := range forfeitTxs {
 		forfeitPtx, err := psbt.NewFromRawBytes(strings.NewReader(b64), true)
 		if err != nil {
@@ -2322,6 +2391,13 @@ func (s *service) SubmitForfeitTxs(ctx context.Context, forfeitTxs []string) err
 		); err != nil {
 			return errors.INVALID_FORFEIT_TXS.Wrap(err)
 		}
+
+		if domain.ForfeitTxCarriesOperatorSignature(forfeitPtx, operatorKeys) {
+			return errors.INVALID_FORFEIT_TXS.New(
+				"forfeit tx %s carries a signature reserved to the operator",
+				forfeitPtx.UnsignedTx.TxID(),
+			)
+		}
 	}
 
 	round, err := s.cache.CurrentRound().Get(ctx)
@@ -3575,14 +3651,15 @@ func (s *service) finalizeRound(roundId string, roundTiming roundTiming, setting
 			}
 		}
 
-		for _, tx := range forfeitTxList {
-			// nolint
-			ptx, _ := psbt.NewFromRawBytes(strings.NewReader(tx), true)
-			forfeitTxid := ptx.UnsignedTx.TxID()
-			forfeitTxs = append(forfeitTxs, domain.ForfeitTx{
-				Txid: forfeitTxid,
-				Tx:   tx,
-			})
+		// Add the operator signature to each forfeit tx at collection time, so the
+		// stored forfeit tx is broadcast-ready without needing to be signed later
+		// at fraud-reaction time.
+		forfeitTxs, err = s.signForfeitTxs(ctx, forfeitTxList)
+		if err != nil {
+			changes = round.Fail(errors.INTERNAL_ERROR.New(
+				"failed to sign forfeit txs: %s", err,
+			))
+			return
 		}
 	}