fix(frost): refuse scaffold key-group by default; tighten witness and message hygiene

pkg/bitcoin/transaction_builder.go

@@ -354,18 +354,40 @@ func (tb *TransactionBuilder) ReplaceUnsignedTransaction(
 			)
 		}
 
+		// The replacement's SignatureScript and Witness are both empty here
+		// because of the two refusals above, so the per-input restore below
+		// only has to decide what to copy *from* the previous input.
 		if tb.sigHashArgs[i].witness {
-			if len(replacedInput.Witness) == 0 && len(previousInput.Witness) == 1 {
+			// Witness inputs may carry a single-element pre-signing witness
+			// that holds a P2WSH-style redeem script. Multi-element witnesses
+			// belong to P2TR script-path spends or other workflows that are
+			// not in scope for the current FROST migration, and silently
+			// dropping them produced malformed transactions later — refuse
+			// instead so the unsupported case fails loudly. Lifting this to
+			// support multi-element witnesses requires a per-input policy
+			// rather than a blanket copy because the replacement could
+			// legitimately differ in witness shape from the previous input.
+			switch len(previousInput.Witness) {
+			case 0:
+				// Nothing to restore (typical P2TR key-path or P2WPKH).
+			case 1:
 				redeemScript := append([]byte{}, previousInput.Witness[0]...)
 				replacedInput.Witness = wire.TxWitness{redeemScript}
-			}
-		} else {
-			if len(replacedInput.SignatureScript) == 0 && len(previousInput.SignatureScript) > 0 {
-				replacedInput.SignatureScript = append(
-					[]byte{},
-					previousInput.SignatureScript...,
+			default:
+				return fmt.Errorf(
+					"replacement transaction input [%d] previous witness has "+
+						"[%d] elements; only zero- or single-element "+
+						"pre-signing witnesses are currently supported for "+
+						"restoration",
+					i,
+					len(previousInput.Witness),
 				)
 			}
+		} else if len(previousInput.SignatureScript) > 0 {
+			replacedInput.SignatureScript = append(
+				[]byte{},
+				previousInput.SignatureScript...,
+			)
 		}
 	}
 

pkg/bitcoin/transaction_builder_test.go

@@ -451,6 +451,58 @@ func TestTransactionBuilder_ReplaceUnsignedTransaction_RejectsNonEmptyReplacemen
 	}
 }
 
+func TestTransactionBuilder_ReplaceUnsignedTransaction_RejectsMultiElementPreviousWitness(
+	t *testing.T,
+) {
+	builder := NewTransactionBuilder(nil)
+
+	var txHash chainhash.Hash
+	previousInput := wire.NewTxIn(wire.NewOutPoint(&txHash, 0), nil, nil)
+	// Pre-signing witness that mimics a P2TR script-path spend: [script,
+	// controlBlock]. The restoration path supports only zero- or
+	// single-element previous witnesses today; the multi-element case must
+	// fail loudly rather than silently dropping data later in signing.
+	previousInput.Witness = wire.TxWitness{
+		[]byte{0x51, 0x52},
+		[]byte{0xc0, 0xab, 0xcd},
+	}
+	builder.internal.AddTxIn(previousInput)
+	builder.sigHashArgs = append(
+		builder.sigHashArgs,
+		&inputSigHashArgs{value: 1, scriptCode: []byte{0x51}, witness: true},
+	)
+
+	err := builder.ReplaceUnsignedTransaction(
+		&Transaction{
+			Inputs: []*TransactionInput{
+				{
+					Outpoint: &TransactionOutpoint{
+						TransactionHash: Hash(txHash),
+						OutputIndex:     0,
+					},
+					Sequence: 0xffffffff,
+				},
+			},
+		},
+	)
+	if err == nil {
+		t.Fatal("expected multi-element witness restoration error")
+	}
+
+	if !strings.Contains(
+		err.Error(),
+		"previous witness has [2] elements",
+	) {
+		t.Fatalf("unexpected error: [%v]", err)
+	}
+	if !strings.Contains(
+		err.Error(),
+		"only zero- or single-element",
+	) {
+		t.Fatalf("unexpected error: [%v]", err)
+	}
+}
+
 func TestTransactionBuilder_UnsignedTransactionIO(t *testing.T) {
 	builder := NewTransactionBuilder(nil)
 

pkg/frost/signing/native_ffi_primitive_registration.go

@@ -9,6 +9,7 @@ import (
 
 var (
 	registrationLogger    = log.Logger("keep-frost-signing-registration")
+	protocolLogger        = log.Logger("keep-frost-signing-protocol")
 	registrationErrorMu   sync.RWMutex
 	lastRegistrationError error
 )

pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go

@@ -3,6 +3,7 @@
 package signing
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -466,6 +467,23 @@ func buildTaggedTBTCSignerRoundKeyGroup(
 	if payload.KeyGroupSource == NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey {
 		// Scaffold compatibility: legacy-wallet-pubkey key groups are
 		// placeholder-only and expected to diverge from coarse RunDKG output.
+		// Refuse the substitution by default so a production deployment that
+		// somehow ended up with placeholder material does not silently route
+		// signing through whatever key group the Rust side happens to return.
+		// The operator must explicitly opt into the scaffold path via
+		// AcceptScaffoldKeyGroupEnvVar; the env-var check is per-call (not
+		// cached) so flipping it off recovers fail-closed behavior without a
+		// restart.
+		if !AcceptScaffoldKeyGroupEnabled() {
+			return "", false, fmt.Errorf(
+				"tbtc-signer key group source %q is scaffold-era placeholder "+
+					"material and may not be silently substituted with the "+
+					"RunDKG output; set %s=true to opt in for local/CI use "+
+					"only, never in production",
+				NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey,
+				AcceptScaffoldKeyGroupEnvVar,
+			)
+		}
 		return dkgResult.KeyGroup, true, nil
 	}
 
@@ -902,13 +920,44 @@ func collectBuildTaggedTBTCSignerRoundContributionMessages(
 			)
 
 		case message := <-messageChan:
-			receivedMessages[message.SenderID()] = message
+			// First-write-wins / equal-or-reject. A peer that retransmits the
+			// same contribution is idempotent; a peer that mutates its own
+			// contribution after the first send is a ROAST evidence concern
+			// and must not be allowed to overwrite the persisted view.
+			senderID := message.SenderID()
+			if existing, ok := receivedMessages[senderID]; ok {
+				if !buildTaggedTBTCSignerRoundContributionMessagesEqual(
+					existing,
+					message,
+				) {
+					protocolLogger.Warnf(
+						"dropping conflicting tbtc-signer round contribution "+
+							"from sender [%d]; first-write-wins keeps the "+
+							"originally accepted contribution",
+						senderID,
+					)
+				}
+				continue
+			}
+			receivedMessages[senderID] = message
 		}
 	}
 
 	return receivedMessages, nil
 }
 
+func buildTaggedTBTCSignerRoundContributionMessagesEqual(
+	left, right *buildTaggedTBTCSignerRoundContributionMessage,
+) bool {
+	if left == nil || right == nil {
+		return left == right
+	}
+	return left.SenderIDValue == right.SenderIDValue &&
+		left.SessionIDValue == right.SessionIDValue &&
+		left.ContributionIdentifier == right.ContributionIdentifier &&
+		bytes.Equal(left.ContributionData, right.ContributionData)
+}
+
 func buildTaggedTBTCSignerSyntheticRoundContributions(
 	roundState *NativeTBTCSignerRoundState,
 	includedMembersIndexes []group.MemberIndex,

pkg/frost/signing/native_ffi_primitive_transitional_frost_native_test.go

@@ -1289,12 +1289,14 @@ func TestExecuteBuildTaggedTBTCSignerBootstrapCoarseRound_FailsWhenRoundStateSig
 
 func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) {
 	testCases := []struct {
-		name        string
-		payload     *NativeTBTCSignerMaterialPayload
-		dkgResult   *NativeTBTCSignerDKGResult
-		expected    string
-		substituted bool
-		expectError bool
+		name                 string
+		payload              *NativeTBTCSignerMaterialPayload
+		dkgResult            *NativeTBTCSignerDKGResult
+		acceptScaffoldOptIn  bool
+		expected             string
+		substituted          bool
+		expectError          bool
+		expectScaffoldRefuse bool
 	}{
 		{
 			name: "exact match",
@@ -1308,16 +1310,29 @@ func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) {
 			substituted: false,
 		},
 		{
-			name: "legacy source mismatch uses dkg key group",
+			name: "legacy source mismatch refused without opt-in",
+			payload: &NativeTBTCSignerMaterialPayload{
+				KeyGroup:       "legacy-group",
+				KeyGroupSource: NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey,
+			},
+			dkgResult: &NativeTBTCSignerDKGResult{
+				KeyGroup: "dkg-group",
+			},
+			expectError:          true,
+			expectScaffoldRefuse: true,
+		},
+		{
+			name: "legacy source mismatch uses dkg key group with opt-in",
 			payload: &NativeTBTCSignerMaterialPayload{
 				KeyGroup:       "legacy-group",
 				KeyGroupSource: NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey,
 			},
 			dkgResult: &NativeTBTCSignerDKGResult{
 				KeyGroup: "dkg-group",
 			},
-			expected:    "dkg-group",
-			substituted: true,
+			acceptScaffoldOptIn: true,
+			expected:            "dkg-group",
+			substituted:         true,
 		},
 		{
 			name: "non-legacy source mismatch rejects",
@@ -1334,12 +1349,30 @@ func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
+			if tc.acceptScaffoldOptIn {
+				t.Setenv(AcceptScaffoldKeyGroupEnvVar, "true")
+			} else {
+				// Force the env to "" so a stray external value from a
+				// containing process cannot suppress the scaffold refusal
+				// during this test case.
+				t.Setenv(AcceptScaffoldKeyGroupEnvVar, "")
+			}
+
 			actual, substituted, err := buildTaggedTBTCSignerRoundKeyGroup(tc.payload, tc.dkgResult)
 			if tc.expectError {
 				if err == nil {
 					t.Fatal("expected error")
 				}
 
+				if tc.expectScaffoldRefuse &&
+					!strings.Contains(err.Error(), AcceptScaffoldKeyGroupEnvVar) {
+					t.Fatalf(
+						"expected scaffold-refusal error referencing %s; got: [%v]",
+						AcceptScaffoldKeyGroupEnvVar,
+						err,
+					)
+				}
+
 				return
 			}
 
@@ -1802,6 +1835,12 @@ func TestBuildTaggedLegacyCompatibleNativeExecutionFFISigningPrimitive_Sign_TBTC
 func TestBuildTaggedLegacyCompatibleNativeExecutionFFISigningPrimitive_Sign_TBTCSignerPath_BootstrapVersion_LegacyKeyGroupSourceUsesRunDKGResult(
 	t *testing.T,
 ) {
+	// Scaffold-era path: legacy-wallet-pubkey signer material is refused by
+	// default; the operator opt-in via AcceptScaffoldKeyGroupEnvVar is what
+	// lets this test exercise the substitution. Production deployments must
+	// never set this.
+	t.Setenv(AcceptScaffoldKeyGroupEnvVar, "true")
+
 	engine := &mockBuildTaggedTBTCSignerEngine{
 		version: "tbtc-signer/0.1.0-bootstrap",
 		runDKGResult: &NativeTBTCSignerDKGResult{

pkg/frost/signing/native_frost_protocol_frost_native.go

@@ -3,6 +3,7 @@
 package signing
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -582,13 +583,39 @@ func collectNativeFROSTRoundOneMessages(
 			)
 
 		case message := <-messageChan:
-			receivedMessages[message.SenderID()] = message
+			// First-write-wins / equal-or-reject. See the matching comment in
+			// native_ffi_primitive_transitional_frost_native.go.
+			senderID := message.SenderID()
+			if existing, ok := receivedMessages[senderID]; ok {
+				if !nativeFROSTRoundOneCommitmentMessagesEqual(existing, message) {
+					protocolLogger.Warnf(
+						"dropping conflicting native FROST round one "+
+							"commitment from sender [%d]; first-write-wins "+
+							"keeps the originally accepted commitment",
+						senderID,
+					)
+				}
+				continue
+			}
+			receivedMessages[senderID] = message
 		}
 	}
 
 	return receivedMessages, nil
 }
 
+func nativeFROSTRoundOneCommitmentMessagesEqual(
+	left, right *nativeFROSTRoundOneCommitmentMessage,
+) bool {
+	if left == nil || right == nil {
+		return left == right
+	}
+	return left.SenderIDValue == right.SenderIDValue &&
+		left.SessionIDValue == right.SessionIDValue &&
+		left.ParticipantIdentifier == right.ParticipantIdentifier &&
+		bytes.Equal(left.CommitmentData, right.CommitmentData)
+}
+
 func collectNativeFROSTRoundTwoMessages(
 	ctx context.Context,
 	request *NativeExecutionFFISigningRequest,
@@ -638,13 +665,41 @@ func collectNativeFROSTRoundTwoMessages(
 			)
 
 		case message := <-messageChan:
-			receivedMessages[message.SenderID()] = message
+			// First-write-wins / equal-or-reject. See round one above.
+			senderID := message.SenderID()
+			if existing, ok := receivedMessages[senderID]; ok {
+				if !nativeFROSTRoundTwoSignatureShareMessagesEqual(
+					existing,
+					message,
+				) {
+					protocolLogger.Warnf(
+						"dropping conflicting native FROST round two "+
+							"signature share from sender [%d]; first-write-wins "+
+							"keeps the originally accepted share",
+						senderID,
+					)
+				}
+				continue
+			}
+			receivedMessages[senderID] = message
 		}
 	}
 
 	return receivedMessages, nil
 }
 
+func nativeFROSTRoundTwoSignatureShareMessagesEqual(
+	left, right *nativeFROSTRoundTwoSignatureShareMessage,
+) bool {
+	if left == nil || right == nil {
+		return left == right
+	}
+	return left.SenderIDValue == right.SenderIDValue &&
+		left.SessionIDValue == right.SessionIDValue &&
+		left.ParticipantIdentifier == right.ParticipantIdentifier &&
+		bytes.Equal(left.SignatureShareData, right.SignatureShareData)
+}
+
 func shouldAcceptNativeFROSTMessage(
 	request *NativeExecutionFFISigningRequest,
 	includedMembersSet map[group.MemberIndex]struct{},