Skip to content

Commit 6b69b9e

Browse files
KukksKukks
authored
feat(wallets): compose signers from IDescriptorSigningSources (#114)
* feat(wallets): compose signers from IPrivateKeyProviders Closes #112. Pushes signer composition down one layer: an IArkadeWalletSigner is now always a CompositeArkadeWalletSigner built from one or more IPrivateKeyProviders. Each provider answers CanProvideAsync(descriptor) and exposes the signing operations rooted in the key it owns; the composite dispatches each call to the first provider that claims the descriptor. The three previous concrete signers collapse into providers behind the same composite: - Bip39KeyProvider (was HierarchicalDeterministicWalletSigner) claim by master fingerprint match on the descriptor's origin. - NsecKeyProvider (was NSecWalletSigner) claim by x-only pubkey match on tr() descriptors. - RemoteTransportKeyProvider (was RemoteArkadeWalletSigner) claim by IRemoteSignerTransport.KnowsWalletAsync; passthrough proxy. DefaultWalletProvider builds the composition automatically — adds the matching local provider if Wallet.Secret is set, then a remote provider if an IRemoteSignerTransport claims the wallet. Order is significant: local providers get first refusal, remote is fallback. This is what makes hybrid setups (e.g. local view-key paths + HWI spend path) expressible at all — the previous binary local/remote split foreclosed them. The interface keeps IArkadeWalletSigner unchanged for SDK consumers (the TreeSignerSession path is unaffected); the new abstraction is added one layer below. The three old concrete signer classes are deleted since they're equivalent to constructing a CompositeArkadeWalletSigner with the matching single provider — NsecKeyProvider.FromNsec is the new entry point that mirrors the old NSecWalletSigner.FromNsec static. The IPrivateKeyProvider shape is operation-level (Sign/SignMusig/ GenerateNonces) rather than returning a raw ECPrivKey, so a remote provider can implement it honestly without round-tripping secret material over the wire — the original sketch in #112 had IPrivateKeyProvider return ECPrivKey, which would have defeated the point of remote signing. Each local provider keeps its own per-session ConcurrentDictionary nonce store with the same sessionId-keyed semantics established in #111. * refactor(wallets): rename IPrivateKeyProvider → IDescriptorSigningSource, simplify cache key, drop hybrid claim Three review-driven changes on top of the initial #112 sketch: - Renamed IPrivateKeyProvider → IDescriptorSigningSource. The interface never exposes ECPrivKey — it exposes the signing operations rooted in one. "PrivateKeyProvider" oversells what the abstraction returns; "DescriptorSigningSource" describes what it does. Renames track through: Bip39KeyProvider → Bip39SigningSource NsecKeyProvider → NsecSigningSource RemoteTransportKeyProvider → RemoteTransportSigningSource PrivateKeyProviders/ → SigningSources/ NsecKeyProviderParityTests → NsecSigningSourceParityTests - DefaultWalletProvider signer cache key collapses to wallet.Id only. The previous key folded in hashes of wallet.Secret and the transport instance to handle re-import; in practice signers are singleton-lifetime and re-import is an out-of-band event handled by host restart or explicit cache invalidation. Documented in the field comment. - Dropped "hybrid (local + remote on the same wallet)" claim from README + wallets.md. RemoteTransportSigningSource.CanProvideAsync delegates to KnowsWalletAsync, which is wallet-grained, not descriptor-grained, so "local view-path + remote spend-path" is not actually expressible without a per-descriptor probe on the transport. Filing that as a follow-up; meanwhile the docs only promise what the current contract delivers. No behaviour change beyond the cache-key simplification. * perf(wallets): cache-hit short-circuit in GetSignerAsync Addresses the remaining 🟡 from Arkana's PR #114 review. GetSignerAsync is called per-VTXO during batch participation; previously every call constructed a fresh Bip39SigningSource (master-fingerprint derivation) and awaited KnowsWalletAsync (potentially a network round-trip for remote transports) even when _signerCache would hit immediately after. Move the cache check up to right after LoadWallet — if a signer is cached for this wallet.Id we return it without touching the source-construction path. Cache miss still runs the full composition. The fingerprint derivation hits the static ExtKey cache so the in-process overhead is small, but the cost matters once anyone plugs in a remote transport whose KnowsWalletAsync is actually a network call.
1 parent 095ad33 commit 6b69b9e

12 files changed

Lines changed: 471 additions & 317 deletions

‎NArk.Abstractions/Wallets/IDescriptorSigningSource.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
using NBitcoin;
2+
using NBitcoin.Scripting;
3+
using NBitcoin.Secp256k1;
4+
using NBitcoin.Secp256k1.Musig;
5+
6+
namespace NArk.Abstractions.Wallets;
7+
8+
/// <summary>
9+
/// A descriptor-scoped source of signing operations. The source owns signing material (locally
10+
/// or by reference) and exposes operations rooted in it — it never returns the raw key, which
11+
/// is what lets a remote-signing implementation honour the same contract.
12+
/// <para>
13+
/// A wallet's signer is a composition of one or more signing sources (see
14+
/// <see cref="IArkadeWalletSigner"/> implementations that compose sources). Each source answers
15+
/// <see cref="CanProvideAsync"/> for the descriptors it covers; the composing signer dispatches
16+
/// each call to the first source that claims the descriptor.
17+
/// </para>
18+
/// <para>
19+
/// The MuSig2 nonce lifecycle from <see cref="IArkadeWalletSigner"/> applies per-source:
20+
/// <see cref="GenerateNoncesAsync"/> retains the secret half indexed by <c>sessionId</c>, and
21+
/// <see cref="SignMusigAsync"/> consumes it on use. Different sources maintain independent nonce
22+
/// stores — there is no cross-source sharing.
23+
/// </para>
24+
/// </summary>
25+
public interface IDescriptorSigningSource
26+
{
27+
/// <summary>
28+
/// Returns <c>true</c> iff this source can sign for <paramref name="descriptor"/>.
29+
/// Implementations should make this cheap (fingerprint / x-only match) — it is called on
30+
/// every dispatch and may run against the BIP-32 / BIP-39 derivation path of long descriptors.
31+
/// </summary>
32+
Task<bool> CanProvideAsync(OutputDescriptor descriptor, CancellationToken cancellationToken = default);
33+
34+
/// <summary>
35+
/// Gets the compressed public key for <paramref name="descriptor"/>, preserving parity.
36+
/// </summary>
37+
Task<ECPubKey> GetPubKeyAsync(OutputDescriptor descriptor, CancellationToken cancellationToken = default);
38+
39+
/// <summary>
40+
/// Produces a BIP-340 Schnorr signature over <paramref name="hash"/> using the descriptor's
41+
/// private key, returning the x-only pubkey alongside the signature.
42+
/// </summary>
43+
Task<(ECXOnlyPubKey, SecpSchnorrSignature)> SignAsync(
44+
OutputDescriptor descriptor,
45+
uint256 hash,
46+
CancellationToken cancellationToken = default);
47+
48+
/// <summary>
49+
/// Generates a fresh MuSig2 nonce pair for <paramref name="context"/>, retains the secret
50+
/// half indexed by <paramref name="sessionId"/>, and returns the public half. See
51+
/// <see cref="IArkadeWalletSigner.GenerateNonces"/> for the lifecycle contract.
52+
/// </summary>
53+
Task<MusigPubNonce> GenerateNoncesAsync(
54+
OutputDescriptor descriptor,
55+
MusigContext context,
56+
string sessionId,
57+
CancellationToken cancellationToken = default);
58+
59+
/// <summary>
60+
/// Produces a MuSig2 partial signature using the secret nonce generated for the same
61+
/// <paramref name="sessionId"/>. The secret nonce is consumed on this call. See
62+
/// <see cref="IArkadeWalletSigner.SignMusig"/> for the lifecycle contract.
63+
/// </summary>
64+
Task<MusigPartialSignature> SignMusigAsync(
65+
OutputDescriptor descriptor,
66+
MusigContext context,
67+
string sessionId,
68+
CancellationToken cancellationToken = default);
69+
}

‎NArk.Core/Wallet/CompositeArkadeWalletSigner.cs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
using NArk.Abstractions.Wallets;
2+
using NBitcoin;
3+
using NBitcoin.Scripting;
4+
using NBitcoin.Secp256k1;
5+
using NBitcoin.Secp256k1.Musig;
6+
7+
namespace NArk.Core.Wallet;
8+
9+
/// <summary>
10+
/// An <see cref="IArkadeWalletSigner"/> composed of one or more <see cref="IDescriptorSigningSource"/>s.
11+
/// For every signing call the composite resolves the first source whose
12+
/// <see cref="IDescriptorSigningSource.CanProvideAsync"/> returns <c>true</c> and dispatches
13+
/// the operation to it. Source order is significant — earlier sources take precedence — so
14+
/// callers should register the cheapest / most-local source first and any fallbacks
15+
/// (e.g. remote-signer transports) last.
16+
/// </summary>
17+
public class CompositeArkadeWalletSigner : IArkadeWalletSigner
18+
{
19+
private readonly IReadOnlyList<IDescriptorSigningSource> _sources;
20+
21+
public CompositeArkadeWalletSigner(IEnumerable<IDescriptorSigningSource> sources)
22+
{
23+
_sources = (sources ?? throw new ArgumentNullException(nameof(sources))).ToArray();
24+
if (_sources.Count == 0)
25+
throw new ArgumentException("At least one signing source is required.", nameof(sources));
26+
}
27+
28+
public CompositeArkadeWalletSigner(params IDescriptorSigningSource[] sources)
29+
: this((IEnumerable<IDescriptorSigningSource>)sources)
30+
{
31+
}
32+
33+
public async Task<ECPubKey> GetPubKey(OutputDescriptor descriptor, CancellationToken cancellationToken = default)
34+
=> await (await ResolveAsync(descriptor, cancellationToken)).GetPubKeyAsync(descriptor, cancellationToken);
35+
36+
public async Task<(ECXOnlyPubKey, SecpSchnorrSignature)> Sign(OutputDescriptor descriptor, uint256 hash, CancellationToken cancellationToken = default)
37+
=> await (await ResolveAsync(descriptor, cancellationToken)).SignAsync(descriptor, hash, cancellationToken);
38+
39+
public async Task<MusigPubNonce> GenerateNonces(OutputDescriptor descriptor, MusigContext context, string sessionId, CancellationToken cancellationToken = default)
40+
=> await (await ResolveAsync(descriptor, cancellationToken)).GenerateNoncesAsync(descriptor, context, sessionId, cancellationToken);
41+
42+
public async Task<MusigPartialSignature> SignMusig(OutputDescriptor descriptor, MusigContext context, string sessionId, CancellationToken cancellationToken = default)
43+
=> await (await ResolveAsync(descriptor, cancellationToken)).SignMusigAsync(descriptor, context, sessionId, cancellationToken);
44+
45+
private async Task<IDescriptorSigningSource> ResolveAsync(OutputDescriptor descriptor, CancellationToken cancellationToken)
46+
{
47+
foreach (var source in _sources)
48+
{
49+
if (await source.CanProvideAsync(descriptor, cancellationToken).ConfigureAwait(false))
50+
return source;
51+
}
52+
53+
throw new InvalidOperationException(
54+
$"No registered IDescriptorSigningSource claims descriptor '{descriptor}'. " +
55+
"Either the wallet is watch-only for this path, or a signing source is missing from the composition.");
56+
}
57+
}

‎NArk.Core/Wallet/DefaultWalletProvider.cs‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using NArk.Abstractions.Safety;
66
using NArk.Abstractions.Wallets;
77
using NArk.Core.Transport;
8+
using NArk.Core.Wallet.SigningSources;
89
using NBitcoin;
910

1011
namespace NArk.Core.Wallet;
@@ -30,9 +31,10 @@ public class DefaultWalletProvider(
3031
: IWalletProvider
3132
{
3233
// Signer instances must be reused across calls so the MuSig2 secret-nonce store on each
33-
// signer (populated by GenerateNonces, consumed by SignMusig — see IArkadeWalletSigner)
34-
// survives between the two calls. The cache key includes the wallet's secret so a wallet
35-
// re-imported with a different mnemonic gets a fresh signer.
34+
// signing source (populated by GenerateNonces, consumed by SignMusig — see
35+
// IArkadeWalletSigner) survives between the two calls. Keyed by wallet.Id only — if a
36+
// wallet is re-imported with different signing material the caller must explicitly
37+
// invalidate (restart the host, or call into a fresh provider instance).
3638
private readonly ConcurrentDictionary<string, IArkadeWalletSigner> _signerCache = new();
3739

3840
public async Task<IArkadeWalletSigner?> GetSignerAsync(string identifier, CancellationToken cancellationToken = default)
@@ -46,30 +48,40 @@ public class DefaultWalletProvider(
4648
logger?.LogDebug("GetSignerAsync: identifier={Identifier}, walletId={WalletId}, walletType={WalletType}, hasSecret={HasSecret}",
4749
identifier, wallet.Id, wallet.WalletType, !string.IsNullOrEmpty(wallet.Secret));
4850

49-
// Local signing material present → produce a local signer of the right shape.
51+
// Hot-path short-circuit: this method is called per-VTXO during batch participation,
52+
// so avoid constructing a fresh Bip39SigningSource (master-fingerprint derivation)
53+
// and round-tripping KnowsWalletAsync (potentially a network call for remote
54+
// transports) every time. Cache miss runs the full composition below.
55+
if (_signerCache.TryGetValue(wallet.Id, out var cached))
56+
return cached;
57+
58+
var sources = new List<IDescriptorSigningSource>();
59+
60+
// Local signing material present → add the matching local signing source.
5061
if (!string.IsNullOrEmpty(wallet.Secret))
5162
{
52-
var cacheKey = $"local:{wallet.Id}:{wallet.Secret.GetHashCode():x}";
53-
return _signerCache.GetOrAdd(cacheKey, _ => wallet.WalletType switch
63+
sources.Add(wallet.WalletType switch
5464
{
55-
WalletType.HD => new HierarchicalDeterministicWalletSigner(wallet),
56-
WalletType.SingleKey => NSecWalletSigner.FromNsec(wallet.Secret, logger),
65+
WalletType.HD => new Bip39SigningSource(wallet.Secret),
66+
WalletType.SingleKey => NsecSigningSource.FromNsec(wallet.Secret, logger),
5767
_ => throw new ArgumentOutOfRangeException(nameof(wallet.WalletType))
5868
});
5969
}
6070

61-
// No local secret → ask the remote-signer transport (if any) whether it can sign for
62-
// this wallet. The transport is the source of truth for "remote vs watch-only" —
63-
// no flag on ArkWalletInfo encodes it.
71+
// Remote-signer transport claims this wallet → add a remote source as a fallback
72+
// for descriptors no local source covers. Order is significant: local sources get
73+
// first refusal.
6474
if (remoteSignerTransport is not null
6575
&& await remoteSignerTransport.KnowsWalletAsync(wallet.Id, cancellationToken).ConfigureAwait(false))
6676
{
67-
return _signerCache.GetOrAdd($"remote:{wallet.Id}",
68-
_ => new RemoteArkadeWalletSigner(wallet.Id, remoteSignerTransport));
77+
sources.Add(new RemoteTransportSigningSource(remoteSignerTransport, wallet.Id));
6978
}
7079

71-
// No local key, no remote signer claims it → watch-only.
72-
return null;
80+
// Nothing can sign for this wallet → watch-only.
81+
if (sources.Count == 0)
82+
return null;
83+
84+
return _signerCache.GetOrAdd(wallet.Id, _ => new CompositeArkadeWalletSigner(sources));
7385
}
7486
catch (KeyNotFoundException)
7587
{

‎NArk.Core/Wallet/HierarchicalDeterministicWalletSigner.cs‎

Lines changed: 0 additions & 76 deletions
This file was deleted.

0 commit comments

Comments
 (0)