fix(arkd-wallet): don't silently sign a required leaf with the wrong signer key

pkg/arkd-wallet/core/application/wallet/service.go

@@ -677,7 +677,13 @@ func (w *wallet) SignTransaction(
 		if len(input.TaprootLeafScript) > 0 {
 			var signingKey *btcec.PrivateKey
 			if signMode == application.SignModeSigner {
-				signingKey = w.signerKeyForLeaf(input.TaprootLeafScript[0].Script)
+				leafKey, err := w.signerKeyForLeaf(
+					input.TaprootLeafScript[0].Script, len(inputIndexes) > 0,
+				)
+				if err != nil {
+					return "", err
+				}
+				signingKey = leafKey
 			} else {
 				signingKey = w.keyMgr.forfeitPrvkey
 			}
@@ -794,38 +800,68 @@ func (w *wallet) SignTransaction(
 	return ptx.B64Encode()
 }
 
-// signerKeyForLeaf returns the deprecated signer key referenced by the leaf, or the current SignerKey.
-func (w *wallet) signerKeyForLeaf(leafScript []byte) *btcec.PrivateKey {
-	if len(w.DeprecatedSignerKeys) == 0 {
-		return w.SignerKey
+// signerKeyForLeaf returns the wallet key that signs the given tapscript leaf:
+// the current SignerKey or a deprecated one, whichever the leaf's multisig closure
+// references. When the leaf references none of the wallet's keys, a required input
+// (caller passed explicit indexes) is a hard error rather than a silent wrong-key
+// signature, while a best-effort one falls back to the current key. Non-multisig
+// leaves also fall back to the current key.
+func (w *wallet) signerKeyForLeaf(leafScript []byte, required bool) (*btcec.PrivateKey, error) {
+	leafKeys, ok := multisigClosureKeys(leafScript)
+	if !ok {
+		return w.SignerKey, nil
 	}
 
+	if keyInLeaf(w.SignerKey, leafKeys) {
+		return w.SignerKey, nil
+	}
+	for _, k := range w.DeprecatedSignerKeys {
+		if keyInLeaf(k.Key, leafKeys) {
+			return k.Key, nil
+		}
+	}
+
+	if required {
+		return nil, fmt.Errorf(
+			"no signer key for tapscript leaf: it references none of the wallet's keys " +
+				"(current or deprecated); a rotated signer key may not have been retained " +
+				"as a deprecated key",
+		)
+	}
+	return w.SignerKey, nil
+}
+
+// multisigClosureKeys returns the public keys of a multisig-bearing closure and
+// whether the leaf could be decoded as one.
+func multisigClosureKeys(leafScript []byte) ([]*btcec.PublicKey, bool) {
 	closure, err := script.DecodeClosure(leafScript)
 	if err != nil {
-		return w.SignerKey
+		return nil, false
 	}
-
-	leafKeys := make([]*btcec.PublicKey, 0)
 	switch c := closure.(type) {
 	case *script.MultisigClosure:
-		leafKeys = c.PubKeys
+		return c.PubKeys, true
 	case *script.CLTVMultisigClosure:
-		leafKeys = c.PubKeys
+		return c.PubKeys, true
 	case *script.ConditionMultisigClosure:
-		leafKeys = c.PubKeys
+		return c.PubKeys, true
 	default:
-		return w.SignerKey
+		return nil, false
 	}
+}
 
-	for _, k := range w.DeprecatedSignerKeys {
-		want := schnorr.SerializePubKey(k.Key.PubKey())
-		for _, pubkey := range leafKeys {
-			if bytes.Equal(schnorr.SerializePubKey(pubkey), want) {
-				return k.Key
-			}
+// keyInLeaf reports whether key is one of the leaf's multisig public keys.
+func keyInLeaf(key *btcec.PrivateKey, leafKeys []*btcec.PublicKey) bool {
+	if key == nil {
+		return false
+	}
+	want := schnorr.SerializePubKey(key.PubKey())
+	for _, pubkey := range leafKeys {
+		if bytes.Equal(schnorr.SerializePubKey(pubkey), want) {
+			return true
 		}
 	}
-	return w.SignerKey
+	return false
 }
 
 // WithdrawAll withdraws all available balance including connectors account funds

pkg/arkd-wallet/core/application/wallet/signer_keys_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	arklib "github.com/arkade-os/arkd/pkg/ark-lib"
 	"github.com/arkade-os/arkd/pkg/ark-lib/script"
 	"github.com/arkade-os/arkd/pkg/arkd-wallet/core/application"
 	"github.com/btcsuite/btcd/btcec/v2"
@@ -133,6 +134,81 @@ func leafScript(t *testing.T, owner, signer *btcec.PublicKey) []byte {
 	return s
 }
 
+// TestSignerKeyForLeaf covers the key selection used when signing tapscript leaves:
+// the wallet signs with whichever of its keys (current or deprecated) the leaf's
+// multisig closure references; a required leaf that references none of the wallet's
+// keys is a hard error instead of a silent wrong-key signature.
+func TestSignerKeyForLeaf(t *testing.T) {
+	mustKey := func() *btcec.PrivateKey {
+		k, err := btcec.NewPrivateKey()
+		require.NoError(t, err)
+		return k
+	}
+	current, deprecated, user, stranger := mustKey(), mustKey(), mustKey(), mustKey()
+
+	multisigLeaf := func(keys ...*btcec.PublicKey) []byte {
+		s, err := (&script.MultisigClosure{
+			PubKeys: keys, Type: script.MultisigTypeChecksig,
+		}).Script()
+		require.NoError(t, err)
+		return s
+	}
+	pub := func(k *btcec.PrivateKey) []byte { return schnorr.SerializePubKey(k.PubKey()) }
+
+	w := &wallet{WalletOptions: WalletOptions{
+		SignerKey:            current,
+		DeprecatedSignerKeys: []DeprecatedSignerKey{{Key: deprecated}},
+	}}
+
+	t.Run("current key in multisig leaf", func(t *testing.T) {
+		key, err := w.signerKeyForLeaf(multisigLeaf(user.PubKey(), current.PubKey()), true)
+		require.NoError(t, err)
+		require.Equal(t, pub(current), pub(key))
+	})
+
+	// The fix must select the deprecated key for every multisig closure type the
+	// server co-signs (plain, CLTV, and condition multisig), which previously fell
+	// through to the current key (the regression surface).
+	t.Run("deprecated key matched across all closure types", func(t *testing.T) {
+		ms := script.MultisigClosure{
+			PubKeys: []*btcec.PublicKey{user.PubKey(), deprecated.PubKey()},
+			Type:    script.MultisigTypeChecksig,
+		}
+		cond := []byte{txscript.OP_TRUE}
+		closures := map[string]script.Closure{
+			"multisig":  &ms,
+			"cltv":      &script.CLTVMultisigClosure{MultisigClosure: ms, Locktime: arklib.AbsoluteLocktime(1000)},
+			"condition": &script.ConditionMultisigClosure{MultisigClosure: ms, Condition: cond},
+		}
+		for name, c := range closures {
+			t.Run(name, func(t *testing.T) {
+				leaf, err := c.Script()
+				require.NoError(t, err)
+				key, err := w.signerKeyForLeaf(leaf, true)
+				require.NoError(t, err)
+				require.Equal(t, pub(deprecated), pub(key))
+			})
+		}
+	})
+
+	t.Run("required leaf with no held key errors", func(t *testing.T) {
+		_, err := w.signerKeyForLeaf(multisigLeaf(user.PubKey(), stranger.PubKey()), true)
+		require.ErrorContains(t, err, "no signer key for tapscript leaf")
+	})
+
+	t.Run("best-effort leaf with no held key falls back to current", func(t *testing.T) {
+		key, err := w.signerKeyForLeaf(multisigLeaf(user.PubKey(), stranger.PubKey()), false)
+		require.NoError(t, err)
+		require.Equal(t, pub(current), pub(key))
+	})
+
+	t.Run("non-multisig leaf falls back to current", func(t *testing.T) {
+		key, err := w.signerKeyForLeaf([]byte{0x01, 0x02, 0x03}, true)
+		require.NoError(t, err)
+		require.Equal(t, pub(current), pub(key))
+	})
+}
+
 func TestSignTransactionRejectsPartialSigHashTypes(t *testing.T) {
 	owner, err := btcec.NewPrivateKey()
 	require.NoError(t, err)