fix(wallets): keep MuSig2 secret nonce inside the signer

Closes #111.

The IArkadeWalletSigner / IRemoteSignerTransport contracts used to return
MusigPrivNonce from GenerateNonces and require the caller to pass it back into
SignMusig. That made the secret nonce cross the signer boundary — for the
remote-signer transport that meant the "remote signer" wasn't actually one
(secret material round-tripped over the wire), and for in-process signers it
encouraged callers to hold a MuSig2 secret in their own data structures where
accidental reuse leaks the private key (MuSig2 nonce reuse is catastrophic).

Reshape the signer contract so:

  - GenerateNonces returns MusigPubNonce only. The signer derives the secret
    nonce internally, stores it indexed by MusigContext.AggregatePubKey
    (the tweaked aggregate is a content fingerprint of the cosigner set +
    taproot tweaks), and returns the public half.
  - SignMusig drops the MusigPrivNonce parameter. The signer looks the secret
    nonce back up by the same context and consumes it on use. Throws if no
    matching nonce was generated for this context.
  - GenerateNonces throws if a nonce is already stored for the same context
    (calling twice without an intervening SignMusig would orphan secret
    material; the user is almost certainly making a mistake).

NSecWalletSigner / HierarchicalDeterministicWalletSigner / SimpleSeedWallet
each hold a per-instance ConcurrentDictionary<string, MusigPrivNonce> keyed by
the hex of MusigContext.AggregatePubKey.ToBytes(); SignMusig TryRemoves on
consume so the store is self-evicting on the happy path. RemoteArkadeWalletSigner
is a pure passthrough, so it just drops the nonce param.

IRemoteSignerTransport now documents that long-lived transports need an
eviction policy (TTL or bounded count) for abandoned nonces — in-process
signers get away with remove-on-consume because their lifetime is the batch
session, but a server-side transport could leak unconsumed entries if a client
generates a nonce and never signs.

TreeSignerSession's _myNonces collapses from
Dictionary<uint256, (MusigPrivNonce, MusigPubNonce)> to
Dictionary<uint256, MusigPubNonce> — the secret half no longer round-trips
through the batch coordinator.

docs/articles/wallets.md updates the remote-signer example and adds a section
on the MuSig2 nonce lifecycle and eviction responsibility.

Author: Kukks

NArk.Abstractions/Wallets/IArkadeWalletSigner.cs

@@ -5,17 +5,38 @@
 
 namespace NArk.Abstractions.Wallets;
 
+/// <summary>
+/// Wallet-side signer used during MuSig2 batch participation. The MuSig2 nonce flow is
+/// designed so the secret half never leaves the signer:
+/// <list type="number">
+///   <item><description><see cref="GenerateNonces"/> derives the secret nonce internally and returns only the public half.</description></item>
+///   <item><description>The signer keeps the secret nonce indexed by <paramref name="context"/>'s aggregate pubkey.</description></item>
+///   <item><description><see cref="SignMusig"/> looks the secret nonce back up by the same context, uses it, and consumes it.</description></item>
+/// </list>
+/// Callers pass the same <see cref="MusigContext"/> instance (or one with an identical
+/// <see cref="MusigContext.AggregatePubKey"/>) to both calls. Calling <see cref="SignMusig"/>
+/// without a prior matching <see cref="GenerateNonces"/> on the same signer throws.
+/// </summary>
 public interface IArkadeWalletSigner
 {
     /// <summary>
     /// Gets the compressed public key for the given descriptor, preserving parity.
     /// </summary>
     Task<ECPubKey> GetPubKey(OutputDescriptor descriptor, CancellationToken cancellationToken = default);
 
+    /// <summary>
+    /// Produces a MuSig2 partial signature for the given context using the descriptor's
+    /// private key and the secret nonce generated for the same context by a prior call
+    /// to <see cref="GenerateNonces"/>. The secret nonce is consumed and cannot be reused
+    /// for another <see cref="SignMusig"/> call (MuSig2 nonce reuse leaks the private key).
+    /// </summary>
+    /// <exception cref="InvalidOperationException">
+    /// No secret nonce is stored for <paramref name="context"/> — <see cref="GenerateNonces"/>
+    /// was not called for this context on this signer instance, or the nonce was already consumed.
+    /// </exception>
     Task<MusigPartialSignature> SignMusig(
         OutputDescriptor descriptor,
         MusigContext context,
-        MusigPrivNonce nonce,
         CancellationToken cancellationToken = default);
 
 
@@ -24,7 +45,14 @@ Task<MusigPartialSignature> SignMusig(
         uint256 hash,
         CancellationToken cancellationToken = default);
 
-    Task<MusigPrivNonce> GenerateNonces(
+    /// <summary>
+    /// Generates a fresh MuSig2 nonce pair for <paramref name="context"/>, retains the
+    /// secret half indexed by the context's aggregate pubkey, and returns the public half.
+    /// Calling twice for the same context (without an intervening <see cref="SignMusig"/>
+    /// to consume the prior nonce) throws — generating a fresh nonce on top of an unused
+    /// one would orphan secret material in the signer's store.
+    /// </summary>
+    Task<MusigPubNonce> GenerateNonces(
         OutputDescriptor descriptor,
         MusigContext context,
         CancellationToken cancellationToken = default

NArk.Abstractions/Wallets/IRemoteSignerTransport.cs

@@ -19,15 +19,12 @@ namespace NArk.Abstractions.Wallets;
 /// watch-only (<see cref="IWalletProvider.GetSignerAsync"/> returns <c>null</c>). Capability is
 /// answered by this interface, not by a flag on the wallet record.
 /// <para>
-/// <b>SECURITY LIMITATION (phase 1):</b> <see cref="GenerateNoncesAsync"/> currently returns
-/// the MuSig2 secret nonce to the SDK and <see cref="SignMusigAsync"/> accepts it back across
-/// the transport. The cryptographic argument for remote signing — that private material never
-/// leaves the signer — does <i>not</i> hold under this interface. The intended phase-2 shape
-/// has <see cref="GenerateNoncesAsync"/> returning only the public nonce and
-/// <see cref="SignMusigAsync"/> taking no nonce parameter (the signer remembers it indexed by
-/// context). Tracked in <see href="https://github.com/arkade-os/dotnet-sdk/issues/111">#111</see>;
-/// do not rely on nonce confidentiality of an <see cref="IRemoteSignerTransport"/>
-/// implementation until that is fixed.
+/// The MuSig2 nonce flow keeps the secret half on the transport side:
+/// <see cref="GenerateNoncesAsync"/> returns only the public nonce, the implementation retains
+/// the secret half indexed by the context, and <see cref="SignMusigAsync"/> consumes it on use.
+/// Implementations need an eviction policy for abandoned nonces (TTL or bounded count) so the
+/// store does not grow without bound — the SDK-side <see cref="IArkadeWalletSigner"/> contract
+/// requires SignMusig to throw if no matching nonce is found.
 /// </para>
 /// </remarks>
 public interface IRemoteSignerTransport
@@ -53,22 +50,23 @@ Task<ECPubKey> GetPubKeyAsync(
         CancellationToken cancellationToken = default);
 
     /// <summary>
-    /// Produces a MuSig2 partial signature for the given context and nonce
-    /// using the descriptor's private key.
+    /// Produces a MuSig2 partial signature for the given context using the descriptor's
+    /// private key and the secret nonce the transport retained when <see cref="GenerateNoncesAsync"/>
+    /// was called for the same context. The secret nonce is consumed on this call.
     /// </summary>
     /// <param name="walletId">The wallet whose key signs.</param>
     /// <param name="descriptor">The descriptor identifying the signing key.</param>
-    /// <param name="context">The MuSig2 context (cosigner set + sighash).</param>
-    /// <param name="nonce">The secret nonce produced earlier by <see cref="GenerateNoncesAsync"/>.</param>
+    /// <param name="context">The MuSig2 context (cosigner set + sighash) the nonce was generated for.</param>
     /// <param name="cancellationToken">Cancellation token.</param>
-    // TODO: SECURITY — drop the `nonce` parameter and have the signer dereference
-    // the private nonce it kept after GenerateNoncesAsync via a context key. Phase-2,
-    // tracked at https://github.com/arkade-os/dotnet-sdk/issues/111.
+    /// <exception cref="InvalidOperationException">
+    /// No secret nonce is stored for <paramref name="walletId"/> + <paramref name="context"/>
+    /// (it was never generated, was already consumed, or has been evicted by the transport's
+    /// eviction policy).
+    /// </exception>
     Task<MusigPartialSignature> SignMusigAsync(
         string walletId,
         OutputDescriptor descriptor,
         MusigContext context,
-        MusigPrivNonce nonce,
         CancellationToken cancellationToken = default);
 
     /// <summary>
@@ -87,19 +85,17 @@ Task<MusigPartialSignature> SignMusigAsync(
         CancellationToken cancellationToken = default);
 
     /// <summary>
-    /// Generates the secret nonce for the supplied MuSig2 context. The returned
-    /// nonce is bound to <paramref name="context"/> and must be passed back
-    /// into <see cref="SignMusigAsync"/> on the same context.
+    /// Generates a fresh MuSig2 nonce pair for the supplied context, retains the secret
+    /// half on the transport side indexed by <paramref name="walletId"/> +
+    /// <paramref name="context"/>, and returns the public half so the caller can complete
+    /// nonce aggregation with cosigners. The secret half never crosses the transport
+    /// boundary — that is the cryptographic claim of remote signing.
     /// </summary>
     /// <param name="walletId">The wallet whose key contributes the nonce.</param>
     /// <param name="descriptor">The descriptor identifying the signing key.</param>
     /// <param name="context">The MuSig2 context the nonce is generated for.</param>
     /// <param name="cancellationToken">Cancellation token.</param>
-    // TODO: SECURITY — return MusigPubNonce instead and keep the private nonce on
-    // the signer indexed by context. Today the SDK receives the secret half, which
-    // breaks the cryptographic argument for remote signing. Phase-2, tracked at
-    // https://github.com/arkade-os/dotnet-sdk/issues/111.
-    Task<MusigPrivNonce> GenerateNoncesAsync(
+    Task<MusigPubNonce> GenerateNoncesAsync(
         string walletId,
         OutputDescriptor descriptor,
         MusigContext context,

NArk.Core/Batches/TreeSignerSession.cs

@@ -16,7 +16,9 @@ namespace NArk.Core.Batches;
 /// </summary>
 public class TreeSignerSession
 {
-    private Dictionary<uint256, (MusigPrivNonce secNonce, MusigPubNonce pubNonce)>? _myNonces;
+    // Public nonces only — the secret half stays inside the signer and is consumed
+    // when SignMusig is called for the same MusigContext.
+    private Dictionary<uint256, MusigPubNonce>? _myNonces;
     private Dictionary<uint256, MusigContext>? _musigContexts;
     private readonly string _walletId;
     private readonly IWalletProvider _walletProvider;
@@ -113,7 +115,7 @@ public async Task<Dictionary<uint256, MusigPubNonce>> GetNoncesAsync(Cancellatio
     {
         _myNonces ??= await GenerateNoncesAsync(cancellationToken);
 
-        return _myNonces.ToDictionary(pair => pair.Key, pair => pair.Value.pubNonce);
+        return new Dictionary<uint256, MusigPubNonce>(_myNonces);
     }
 
     public void VerifyAggregatedNonces(Dictionary<uint256, MusigPubNonce> expectedAggregateNonces,
@@ -150,7 +152,7 @@ public async Task<Dictionary<uint256, MusigPartialSignature>> SignAsync(Cancella
         return sigs;
     }
 
-    private async Task<Dictionary<uint256, (MusigPrivNonce secNonce, MusigPubNonce pubNonce)>> GenerateNoncesAsync(CancellationToken cancellationToken = default)
+    private async Task<Dictionary<uint256, MusigPubNonce>> GenerateNoncesAsync(CancellationToken cancellationToken = default)
     {
         if (_musigContexts == null)
             await CreateMusigContexts(cancellationToken);
@@ -163,12 +165,12 @@ public async Task<Dictionary<uint256, MusigPartialSignature>> SignAsync(Cancella
                      ?? throw new InvalidOperationException(
                          $"Wallet '{_walletId}' has no signer; watch-only wallets cannot participate in batch signing.");
 
-        var res = new Dictionary<uint256, (MusigPrivNonce secNonce, MusigPubNonce pubNonce)>();
+        var res = new Dictionary<uint256, MusigPubNonce>();
         foreach (var (txid, musigContext) in _musigContexts!)
         {
-            // Generate nonce tied to this specific context
-            var nonce = await signer.GenerateNonces(_descriptor, musigContext, cancellationToken);
-            res[txid] = (nonce, nonce.CreatePubNonce());
+            // Signer stores the secret half indexed by musigContext.AggregatePubKey;
+            // SignPartialAsync passes the same context back so the signer can look it up.
+            res[txid] = await signer.GenerateNonces(_descriptor, musigContext, cancellationToken);
         }
 
         return res;
@@ -182,8 +184,8 @@ private async Task<MusigPartialSignature> SignPartialAsync(TxTree g, Cancellatio
 
         var txid = g.Root.GetGlobalTransaction().GetHash();
 
-        if (!_myNonces.TryGetValue(txid, out var myNonce))
-            throw new InvalidOperationException("missing private nonce");
+        if (!_myNonces.ContainsKey(txid))
+            throw new InvalidOperationException("missing nonce for this txid — GetNoncesAsync must run first");
 
         if (!_musigContexts.TryGetValue(txid, out var musigContext))
             throw new InvalidOperationException("missing musig context");
@@ -196,9 +198,9 @@ private async Task<MusigPartialSignature> SignPartialAsync(TxTree g, Cancellatio
                      ?? throw new InvalidOperationException(
                          $"Wallet '{_walletId}' has no signer; watch-only wallets cannot participate in batch signing.");
 
-        // Use the wallet signer to create a MUSIG2 partial signature
-        // The context already has the correct sighash from nonce generation
-        var partialSig = await signer.SignMusig(_descriptor, musigContext, myNonce.secNonce, cancellationToken);
+        // Signer looks up the secret nonce internally by musigContext.AggregatePubKey
+        // (set when GenerateNonces was called for the same context) and consumes it.
+        var partialSig = await signer.SignMusig(_descriptor, musigContext, cancellationToken);
 
         return partialSig;
     }
@@ -258,10 +260,10 @@ public Task AggregateNonces(uint256 txid, MusigPubNonce[] nonces, CancellationTo
         if (!_musigContexts.TryGetValue(txid, out var musigContext))
             return Task.CompletedTask;
 
-        if (_myNonces is null || !_myNonces.TryGetValue(txid, out var myNonce))
-            throw new InvalidOperationException("missing private nonce");
+        if (_myNonces is null || !_myNonces.TryGetValue(txid, out var myPubNonce))
+            throw new InvalidOperationException("missing my public nonce");
 
-        if (!nonces.Any(nonce => nonce.ToBytes().SequenceEqual(myNonce.pubNonce.ToBytes())))
+        if (!nonces.Any(nonce => nonce.ToBytes().SequenceEqual(myPubNonce.ToBytes())))
         {
             throw new InvalidOperationException("missing my nonce");
         }

NArk.Core/Wallet/HierarchicalDeterministicWalletSigner.cs

@@ -21,6 +21,10 @@ public class HierarchicalDeterministicWalletSigner(ArkWalletInfo wallet) : IArka
     // wallet object, so this introduces no new exposure surface.
     private static readonly System.Collections.Concurrent.ConcurrentDictionary<string, ExtKey> _extKeyCache = new();
 
+    // Per-instance secret nonce store; see NSecWalletSigner for the rationale.
+    // Keyed by the (tweaked) aggregate pubkey hex of the MuSig2 context.
+    private readonly System.Collections.Concurrent.ConcurrentDictionary<string, MusigPrivNonce> _secNonces = new();
+
     private Task<ECPrivKey> DerivePrivateKey(OutputDescriptor descriptor)
     {
         var fullPath = descriptor.Extract().FullPath ?? throw new InvalidOperationException();
@@ -36,9 +40,14 @@ public async Task<ECPubKey> GetPubKey(OutputDescriptor descriptor, CancellationT
         return privKey.CreatePubKey();
     }
 
-    public async Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor, MusigContext context, MusigPrivNonce nonce,
+    public async Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor, MusigContext context,
         CancellationToken cancellationToken = default)
     {
+        var key = ContextKey(context);
+        if (!_secNonces.TryRemove(key, out var nonce))
+            throw new InvalidOperationException(
+                $"No secret nonce stored for the given MuSig2 context (aggregate pubkey {key}). " +
+                "Call GenerateNonces for this context first; nonces are consumed on use and cannot be replayed.");
         var privKey = await DerivePrivateKey(descriptor);
         return context.Sign(privKey, nonce);
     }
@@ -49,9 +58,19 @@ public async Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor,
         return (privKey.CreateXOnlyPubKey(), privKey.SignBIP340(hash.ToBytes()));
     }
 
-    public async Task<MusigPrivNonce> GenerateNonces(OutputDescriptor descriptor, MusigContext context, CancellationToken cancellationToken = default)
+    public async Task<MusigPubNonce> GenerateNonces(OutputDescriptor descriptor, MusigContext context, CancellationToken cancellationToken = default)
     {
         var privKey = await DerivePrivateKey(descriptor);
-        return context.GenerateNonce(privKey);
+        var nonce = context.GenerateNonce(privKey);
+        var key = ContextKey(context);
+        if (!_secNonces.TryAdd(key, nonce))
+            throw new InvalidOperationException(
+                $"A secret nonce is already stored for this MuSig2 context (aggregate pubkey {key}). " +
+                "Call SignMusig to consume it before generating a fresh nonce for the same context; " +
+                "MuSig2 nonce reuse leaks the private key.");
+        return nonce.CreatePubNonce();
     }
+
+    private static string ContextKey(MusigContext context) =>
+        Convert.ToHexString(context.AggregatePubKey.ToBytes()).ToLowerInvariant();
 }

NArk.Core/Wallet/NSecWalletSigner.cs

@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using Microsoft.Extensions.Logging;
 using NArk.Abstractions.Extensions;
 using NArk.Abstractions.Wallets;
@@ -14,6 +15,12 @@ public class NSecWalletSigner(ECPrivKey privateKey, ILogger? logger = null) : IA
     private readonly ECPubKey _publicKey = privateKey.CreatePubKey();
     private readonly ECXOnlyPubKey _xOnlyPubKey = privateKey.CreateXOnlyPubKey();
 
+    // Per-context secret nonce store. Keyed by the (tweaked) aggregate pubkey hex —
+    // a content fingerprint of the cosigner set + taproot tweaks. ConcurrentDictionary
+    // lets GenerateNonces / SignMusig race safely if a caller parallelises across
+    // contexts; each entry is removed on SignMusig consumption so the store doesn't grow.
+    private readonly ConcurrentDictionary<string, MusigPrivNonce> _secNonces = new();
+
     public static NSecWalletSigner FromNsec(string nsec, ILogger? logger = null)
     {
         var encoder2 = Bech32Encoder.ExtractEncoderFromString(nsec);
@@ -56,9 +63,15 @@ public Task<ECPubKey> GetPubKey(OutputDescriptor descriptor, CancellationToken c
         return Task.FromResult(_publicKey);
     }
 
-    public Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor, MusigContext context, MusigPrivNonce nonce,
+    public Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor, MusigContext context,
         CancellationToken cancellationToken = default)
     {
+        var key = ContextKey(context);
+        if (!_secNonces.TryRemove(key, out var nonce))
+            throw new InvalidOperationException(
+                $"No secret nonce stored for the given MuSig2 context (aggregate pubkey {key}). " +
+                "Call GenerateNonces for this context first; nonces are consumed on use and cannot be replayed.");
+
         logger?.LogInformation(
             "SignMusig called. Descriptor={Descriptor}, SignerCompressed={SignerCompressed}",
             descriptor.ToString(),
@@ -105,14 +118,24 @@ public Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor, MusigC
         return Task.FromResult((_xOnlyPubKey, sig));
     }
 
-    public Task<MusigPrivNonce> GenerateNonces(OutputDescriptor descriptor, MusigContext context, CancellationToken cancellationToken = default)
+    public Task<MusigPubNonce> GenerateNonces(OutputDescriptor descriptor, MusigContext context, CancellationToken cancellationToken = default)
     {
+        var key = ContextKey(context);
         logger?.LogInformation(
-            "GenerateNonces called. Descriptor={Descriptor}, SignerCompressed={SignerCompressed}",
+            "GenerateNonces called. Descriptor={Descriptor}, SignerCompressed={SignerCompressed}, ContextKey={ContextKey}",
             descriptor.ToString(),
-            Convert.ToHexString(_publicKey.ToBytes()).ToLowerInvariant());
+            Convert.ToHexString(_publicKey.ToBytes()).ToLowerInvariant(),
+            key);
         var nonce = context.GenerateNonce(privateKey);
+        if (!_secNonces.TryAdd(key, nonce))
+            throw new InvalidOperationException(
+                $"A secret nonce is already stored for this MuSig2 context (aggregate pubkey {key}). " +
+                "Call SignMusig to consume it before generating a fresh nonce for the same context; " +
+                "MuSig2 nonce reuse leaks the private key.");
         logger?.LogInformation("GenerateNonces produced nonce successfully");
-        return Task.FromResult(nonce);
+        return Task.FromResult(nonce.CreatePubNonce());
     }
+
+    private static string ContextKey(MusigContext context) =>
+        Convert.ToHexString(context.AggregatePubKey.ToBytes()).ToLowerInvariant();
 }