Skip to content

Pay to blinded key (p2bk)#21

Merged
Kukks merged 7 commits intoKukks:masterfrom
d4rp4t:feat/p2bk
Nov 21, 2025
Merged

Pay to blinded key (p2bk)#21
Kukks merged 7 commits intoKukks:masterfrom
d4rp4t:feat/p2bk

Conversation

@d4rp4t
Copy link
Collaborator

@d4rp4t d4rp4t commented Oct 17, 2025

cashubtc/nuts#291

Summary by CodeRabbit

  • New Features

    • NUT26 support for advanced cryptographic flows
    • Pay-to-Blinded-Key (P2BK) and enhanced Pay-to-Public-Key (P2PK) blind‑witness signing and blinded-builder APIs
    • Token encoding extended to include an optional ephemeral proof key field
    • Keyset identifiers can be exported as raw bytes
  • Tests

    • New comprehensive Nut26 unit test suite added (duplicate insertions present)

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

Adds Nut26 P2PK blinded-proof support: new cryptographic primitives and public APIs (ComputeZx, ComputeRi, CheckRiOverflow, Concat), P2PK builder/witness blind flows, CBOR encoding/decoding for P2PkE, KeysetId byte accessor, and unit tests (duplicate test block and duplicated P2BK_PREFIX present).

Changes

Cohort / File(s) Summary
Cryptographic core
\DotNut/Cashu.cs`, `DotNut/NUT13/BIP32.cs``
Added P2BK_PREFIX and exposed modulus N. New public APIs: ComputeZx(ECPrivKey,ECPubKey), ComputeRi(...), reintroduced Concat(...), plus overloads/updates for ToUncompressedBytes/ToHex and use of Cashu.N in BIP32.
P2PK builders & witness generation
\DotNut/P2PkBuilder.cs`, `DotNut/P2PKProofSecret.cs`, `DotNut/P2PKProofSecret.cs`*`
P2PkBuilder adds BuildBlinded(keysetId, out ECPubKey) and BuildBlinded(keysetId, ECPrivKey) plus _blindPubkeys(...). P2PKProofSecret adds multiple GenerateBlindWitness(...) overloads implementing blind-witness flows (some HTLC variants are stubs/NotImplemented).
Encoding / Serialization
\DotNut/Encoding/CashuTokenV4Encoder.cs``
CBOR encode/decode extended to include optional proof P2PkE as "pe" (hex) when present and to reconstruct P2PkE on decode.
Data models
\DotNut/Proof.cs`, `DotNut/KeysetId.cs``
Proof gains nullable P2PkE (PubKey?) with JSON attributes. KeysetId gains public byte[] GetBytes() returning hex-decoded bytes.
HTLC API surface
\DotNut/HTLCBuilder.cs`, `DotNut/HTLCProofSecret.cs``
New BuildBlinded overloads on HTLCBuilder and multiple GenerateBlindWitness overloads on HTLCProofSecret; most HTLC blind-witness methods currently throw NotImplementedException or InvalidOperationException (obsolete overrides).
Tests
\DotNut.Tests/UnitTest1.cs`*`
Added Nut26-related tests, using directives, and P2BK_PREFIX constant; the test block and P2BK_PREFIX are duplicated within the file.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Builder as P2PkBuilder
    participant Cashu as Cashu (crypto)
    participant Witness as P2PKProofSecret
    participant Encoder as TokenEncoder
    participant Tests as UnitTests

    Note over Builder: BuildBlinded -> generate p2pk_e and call Build
    Builder->>Cashu: ComputeZx(priv_e, pub_P)
    Cashu-->>Builder: Zx
    Builder->>Builder: _blindPubkeys(rs) -> apply blinding to pubkeys/refunds
    Tests->>Witness: GenerateBlindWitness(..., keys, keysetId, p2pkE)

    loop per candidate index i
        Witness->>Cashu: ComputeRi(Zx, keysetId, i)
        Cashu-->>Witness: Ri (private)
        Witness->>Cashu: (optional) CheckRiOverflow(Zx, keysetId, i)
        Cashu-->>Witness: valid? / overflow
        alt valid & key allowed
            Witness->>Witness: Tweak key, sign, verify, aggregate signature
        else invalid / overflow
            Witness-->>Witness: skip candidate
        end
    end

    Witness->>Encoder: include p2pk_e in CBOR proof (optional)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas needing careful review:
    • Cryptographic correctness in DotNut/Cashu.cs (hash inputs, byte ordering, fallback when hash results are zero/>= N).
    • Tweak/negation, signature logic, verification, and threshold enforcement in DotNut/P2PKProofSecret.cs.
    • Blinding application and indexing in DotNut/P2PkBuilder.cs.
    • CBOR encode/decode symmetry and hex handling for P2PkE in DotNut/Encoding/CashuTokenV4Encoder.cs.
    • Duplicate test block and duplicate P2BK_PREFIX in DotNut.Tests/UnitTest1.cs should be deduplicated.

Possibly related PRs

  • Keyset ID v2 #15 — modifies KeysetId behavior; strongly related because this PR adds KeysetId.GetBytes() and both change the same type.

Suggested reviewers

  • Kukks

Poem

🐰
I hop on keys and blind a clue,
Zx whispers secrets, Ri hops through.
Proofs assemble, signatures cling,
Nut26 dances — carrots in spring!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Pay to blinded key (p2bk)' directly describes the main feature being implemented across the changeset. The PR adds comprehensive support for P2BK functionality including new cryptographic operations, builder methods, witness generation, and encoding/decoding support.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@d4rp4t
Copy link
Collaborator Author

d4rp4t commented Nov 8, 2025

cashubtc/nuts#300

@d4rp4t d4rp4t marked this pull request as ready for review November 8, 2025 00:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
DotNut/Encoding/CashuTokenV4Encoder.cs (1)

54-57: Avoid string roundtrip when encoding P2PkE

Line 55 currently converts proof.P2PkE.Key.ToString() back into bytes. ECPubKey.ToString() isn’t guaranteed to remain in canonical hex (a change would surface as a FormatException here), and we already expose the raw bytes via ToBytes(). Emitting them directly mirrors how the "c" field is handled.

                 if (proof.P2PkE?.Key is not null)
                 {
-                    proofItem.Add("pe", Convert.FromHexString(proof.P2PkE.Key.ToString()));
+                    proofItem.Add("pe", proof.P2PkE.Key.ToBytes());
                 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f35ea1 and 4596016.

📒 Files selected for processing (8)
  • DotNut.Tests/UnitTest1.cs (2 hunks)
  • DotNut/Cashu.cs (3 hunks)
  • DotNut/Encoding/CashuTokenV4Encoder.cs (2 hunks)
  • DotNut/KeysetId.cs (2 hunks)
  • DotNut/NUT13/BIP32.cs (1 hunks)
  • DotNut/P2PKProofSecret.cs (3 hunks)
  • DotNut/P2PkBuilder.cs (3 hunks)
  • DotNut/Proof.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T17:11:38.738Z
Learnt from: d4rp4t
Repo: Kukks/DotNut PR: 16
File: DotNut/PubKey.cs:41-51
Timestamp: 2025-09-17T17:11:38.738Z
Learning: NBitcoin's ECPubKey (from NBitcoin.Secp256k1) implements value-based equality comparison through operator ==, not reference comparison. The operator compares the underlying cryptographic key material, making it suitable for structural equality implementations.

Applied to files:

  • DotNut/NUT13/BIP32.cs
  • DotNut/P2PKProofSecret.cs
  • DotNut/Cashu.cs
🧬 Code graph analysis (7)
DotNut/KeysetId.cs (3)
DotNut/ISecret.cs (1)
  • GetBytes (10-10)
DotNut/Nut10Secret.cs (1)
  • GetBytes (28-33)
DotNut/StringSecret.cs (1)
  • GetBytes (13-16)
DotNut/NUT13/BIP32.cs (1)
DotNut/Cashu.cs (1)
  • Cashu (8-219)
DotNut/P2PKProofSecret.cs (4)
DotNut/P2PKWitness.cs (1)
  • P2PKWitness (5-8)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/Cashu.cs (16)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (158-170)
  • ToBytes (194-199)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • Cashu (8-219)
  • ComputeZx (134-143)
  • CheckRiOverflow (145-156)
DotNut/KeysetId.cs (3)
  • GetBytes (79-82)
  • KeysetId (53-61)
  • KeysetId (63-66)
DotNut/P2PkBuilder.cs (2)
DotNut/Cashu.cs (16)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • Concat (173-187)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (158-170)
  • Cashu (8-219)
  • ComputeZx (134-143)
  • CheckRiOverflow (145-156)
DotNut/KeysetId.cs (3)
  • KeysetId (53-61)
  • KeysetId (63-66)
  • GetBytes (79-82)
DotNut.Tests/UnitTest1.cs (8)
DotNut/PrivKey.cs (3)
  • PrivKey (12-15)
  • PrivKey (17-20)
  • ToString (22-25)
DotNut/PubKey.cs (3)
  • PubKey (12-19)
  • PubKey (21-24)
  • ToString (26-29)
DotNut/KeysetId.cs (4)
  • ToString (68-71)
  • KeysetId (53-61)
  • KeysetId (63-66)
  • GetBytes (79-82)
DotNut/Cashu.cs (7)
  • Cashu (8-219)
  • ComputeZx (134-143)
  • CheckRiOverflow (145-156)
  • ToBytes (194-199)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (158-170)
DotNut/Nut10Secret.cs (3)
  • GetBytes (28-33)
  • Nut10Secret (13-17)
  • Nut10Secret (19-22)
DotNut/P2PkBuilder.cs (5)
  • P2PkBuilder (6-156)
  • P2PkBuilder (55-98)
  • P2PKProofSecret (18-53)
  • P2PKProofSecret (102-107)
  • P2PKProofSecret (110-133)
DotNut/P2PKProofSecret.cs (1)
  • P2PKProofSecret (9-196)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/Encoding/CashuTokenV4Encoder.cs (2)
DotNut/KeysetId.cs (1)
  • ToString (68-71)
DotNut/Cashu.cs (9)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
DotNut/Proof.cs (2)
DotNut/PubKey.cs (2)
  • PubKey (12-19)
  • PubKey (21-24)
DotNut/JsonConverters/PubKeyJsonConverter.cs (1)
  • PubKey (8-23)
🔇 Additional comments (4)
DotNut/NUT13/BIP32.cs (1)

15-30: Consistent curve order reuse looks good

Routing the modulus through Cashu.N keeps every caller on the same value source and lines up with the new P2BK helpers. Nice tightening.

DotNut/KeysetId.cs (1)

79-82: Hex decoding helper fits the new flow

GetBytes() gives the P2PK builders a straightforward way to reuse the keyset identifier in hashes without duplicating parsing logic. Looks solid.

DotNut/Proof.cs (1)

27-30: Optional P2PkE field lands cleanly

The JSON metadata mirrors the existing optional fields, so downstream encoders can opt in without upsetting legacy payloads. Nicely scoped addition.

DotNut.Tests/UnitTest1.cs (1)

658-812: Great coverage for the Nut26 happy path

These deterministic vectors exercise the new P2BK primitives end-to-end, which will make refactors around ComputeRi/ComputeZx much safer. Thanks for locking this down.

d4rp4t and others added 2 commits November 8, 2025 02:33
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
DotNut/P2PkBuilder.cs (1)

118-118: Consider removing the redundant local variable.

The assignment var e = p2pke; is unnecessary—you can use p2pke directly throughout the method, reducing cognitive overhead.

Apply this diff to simplify:

         var keysetIdBytes = keysetId.GetBytes();

-        var e = p2pke;
-        
         for (int i = 0; i < pubkeys.Length; i++)
         {
-            var Zx = Cashu.ComputeZx(e, pubkeys[i]);
+            var Zx = Cashu.ComputeZx(p2pke, pubkeys[i]);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4596016 and f252908.

📒 Files selected for processing (1)
  • DotNut/P2PkBuilder.cs (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
DotNut/P2PkBuilder.cs (2)
DotNut/Cashu.cs (16)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • Concat (173-187)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (158-170)
  • Cashu (8-219)
  • ComputeZx (134-143)
  • CheckRiOverflow (145-156)
DotNut/KeysetId.cs (3)
  • KeysetId (53-61)
  • KeysetId (63-66)
  • GetBytes (79-82)
🔇 Additional comments (1)
DotNut/P2PkBuilder.cs (1)

135-156: LGTM! Null-safe length check correctly implemented.

The blinding logic correctly partitions the rs array across Pubkeys and RefundPubkeys, and the null-safe length validation at lines 137-138 properly handles the case when RefundPubkeys is null, addressing the prior review feedback.

Comment on lines +120 to +130
for (int i = 0; i < pubkeys.Length; i++)
{
var Zx = Cashu.ComputeZx(e, pubkeys[i]);
if (i == 0)
{
extraByte = Cashu.CheckRiOverflow(Zx, keysetIdBytes, i);
}

var Ri = Cashu.ComputeRi(Zx, keysetIdBytes, i, extraByte);
rs.Add(Ri);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that extraByte applies uniformly to all indices.

The extraByte flag is computed only once at i == 0 (line 125), then reused for all subsequent indices in ComputeRi (line 128). However, CheckRiOverflow accepts an index parameter, and each ComputeRi call hashes with index i, suggesting overflow conditions might vary per key. Confirm whether the spec requires checking overflow only once or independently for each blinding factor.


🏁 Script executed:

#!/bin/bash
# Search for P2BK/NUT-26 specification references or documentation
# that clarifies whether extraByte overflow check is per-index or per-set

rg -nC5 'CheckRiOverflow|extraByte|P2BK_PREFIX' --type=cs

Length of output: 7849


Fix extraByte overflow check to validate each index independently.

The extraByte flag must be computed for each index, not just once at i == 0. The CheckRiOverflow method includes the index i in its hash computation (line 147 in Cashu.cs), and ComputeRi also includes i in its hash (lines 165, 168), meaning each index has its own potential overflow condition. The test cases (UnitTest1.cs lines 693–768) and the reference implementation in P2PKProofSecret.cs (lines 122–123) both check overflow independently for each i.

Move the overflow check inside the loop for each iteration:

for (int i = 0; i < pubkeys.Length; i++)
{
    var Zx = Cashu.ComputeZx(e, pubkeys[i]);
    var extraByte = Cashu.CheckRiOverflow(Zx, keysetIdBytes, i);
    
    var Ri = Cashu.ComputeRi(Zx, keysetIdBytes, i, extraByte);
    rs.Add(Ri);
}
🤖 Prompt for AI Agents
In DotNut/P2PkBuilder.cs around lines 120 to 130, extraByte is only computed
once when i == 0 but must be computed per index because CheckRiOverflow and
ComputeRi both depend on i; change the loop to call Cashu.CheckRiOverflow(Zx,
keysetIdBytes, i) inside each iteration, store that result in a local extraByte
variable, and pass that local extraByte to Cashu.ComputeRi for the same i so
each Ri uses its own overflow flag; remove any earlier single-scope extraByte
usage so the flag is scoped per-iteration.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
DotNut/P2PKProofSecret.cs (1)

111-154: Prevent infinite loop when a signer cannot match any slot.

If a candidate key never matches an allowed tweaked pubkey in the inner loop, it remains in availableKeysLeft. The outer while loop will repeatedly process the same key indefinitely instead of advancing or throwing "Not enough valid keys to sign".

This issue was previously identified. Track whether a signature was produced for the current key and remove it from availableKeysLeft after the inner loop regardless of whether a match was found, so the loop can progress.

🧹 Nitpick comments (1)
DotNut/P2PKProofSecret.cs (1)

1-1: Remove unused import.

The System.Security.Cryptography.X509Certificates namespace is not used anywhere in this file.

Apply this diff to remove the unused import:

-using System.Security.Cryptography.X509Certificates;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f252908 and f9cfc01.

📒 Files selected for processing (1)
  • DotNut/P2PKProofSecret.cs (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T17:11:38.738Z
Learnt from: d4rp4t
Repo: Kukks/DotNut PR: 16
File: DotNut/PubKey.cs:41-51
Timestamp: 2025-09-17T17:11:38.738Z
Learning: NBitcoin's ECPubKey (from NBitcoin.Secp256k1) implements value-based equality comparison through operator ==, not reference comparison. The operator compares the underlying cryptographic key material, making it suitable for structural equality implementations.

Applied to files:

  • DotNut/P2PKProofSecret.cs
🧬 Code graph analysis (1)
DotNut/P2PKProofSecret.cs (5)
DotNut/P2PKWitness.cs (1)
  • P2PKWitness (5-8)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/Cashu.cs (20)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (158-170)
  • ToBytes (194-199)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • Cashu (8-219)
  • ComputeZx (134-143)
  • CheckRiOverflow (145-156)
  • ToHex (189-192)
  • ToHex (207-210)
  • ToHex (211-214)
  • ToHex (215-218)
DotNut/KeysetId.cs (3)
  • GetBytes (79-82)
  • KeysetId (53-61)
  • KeysetId (63-66)
DotNut/BlindedMessage.cs (1)
  • BlindedMessage (5-11)

Comment on lines +96 to +99
if (Key != "P2PK")
{
throw new InvalidOperationException("Only P2PK is supported");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix constant self-comparison that disables the guard.

The guard compares the constant Key (defined as "P2PK" at line 11) to the literal "P2PK", which always evaluates to false and makes the check ineffective. This likely should verify a runtime property from the secret data or base class.

Verify what property should be checked here. If the intent is to validate the secret kind/type, consider:

-        if (Key != "P2PK")
+        if (this.Data?.Kind != "P2PK")

Or if verifying the builder's configuration:

-        if (Key != "P2PK")
+        if (Builder./* appropriate property */ != "P2PK")

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
DotNut/P2PKProofSecret.cs (1)

96-99: Fix the ineffective guard that was previously flagged.

This guard compares the constant Key (defined as "P2PK" at line 11) to the literal "P2PK", which always evaluates to false, making the check completely ineffective. This critical issue was flagged in a previous review but remains unaddressed.

As previously suggested, verify what property should be checked. If validating the secret kind/type:

-        if (Key != "P2PK")
+        if (this.Data?.Kind != "P2PK")
         {
             throw new InvalidOperationException("Only P2PK is supported");
         }

Or consider removing the check entirely if it's redundant given the class type.

🧹 Nitpick comments (3)
DotNut/P2PKProofSecret.cs (3)

78-92: Consider adding parameter validation.

The overloaded methods delegate directly without validating that keys, keysetId, or P2PkE are non-null. While the main implementation will eventually fail with a NullReferenceException, explicit validation with descriptive error messages would improve debuggability.

Example validation at the start of the main implementation:

 public P2PKWitness GenerateBlindWitness(ECPrivKey hash, ECPrivKey[] keys, KeysetId keysetId, ECPubKey P2PkE)
 {
+    if (keys == null || keys.Length == 0)
+        throw new ArgumentException("At least one key is required", nameof(keys));
+    if (keysetId == null)
+        throw new ArgumentNullException(nameof(keysetId));
+    if (P2PkE == null)
+        throw new ArgumentNullException(nameof(P2PkE));
+
     if (Key != "P2PK")

137-137: Consider removing redundant signature verification for performance.

Lines 137 and 147 verify the signature immediately after signing. While this is defensive programming that catches unexpected cryptographic library issues, it adds overhead in a code path where we control the private key and signing should always succeed.

If you trust the underlying NBitcoin.Secp256k1 library, you could remove these verification calls:

                 {
                     usedSlots.Add(i);
                     var sig = tweakedPrivkey.SignBIP340(msg);
-                    tweakedPrivkey.CreateXOnlyPubKey().SigVerifyBIP340(sig, msg);
                     result.Signatures = result.Signatures.Append(sig.ToHex()).ToArray();

However, keeping them provides an additional safety guarantee for security-critical cryptographic operations.

Also applies to: 147-147


1-1: Remove unused System.Security.Cryptography.X509Certificates import.

Line 1 imports X509Certificates but this namespace is not used anywhere in the file. Remove this import to keep the code clean.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9cfc01 and d5bd5f2.

📒 Files selected for processing (1)
  • DotNut/P2PKProofSecret.cs (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T17:11:38.738Z
Learnt from: d4rp4t
Repo: Kukks/DotNut PR: 16
File: DotNut/PubKey.cs:41-51
Timestamp: 2025-09-17T17:11:38.738Z
Learning: NBitcoin's ECPubKey (from NBitcoin.Secp256k1) implements value-based equality comparison through operator ==, not reference comparison. The operator compares the underlying cryptographic key material, making it suitable for structural equality implementations.

Applied to files:

  • DotNut/P2PKProofSecret.cs
🧬 Code graph analysis (1)
DotNut/P2PKProofSecret.cs (5)
DotNut/P2PKWitness.cs (1)
  • P2PKWitness (5-8)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/Cashu.cs (20)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (158-170)
  • ToBytes (194-199)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • Cashu (8-219)
  • ComputeZx (134-143)
  • CheckRiOverflow (145-156)
  • ToHex (189-192)
  • ToHex (207-210)
  • ToHex (211-214)
  • ToHex (215-218)
DotNut/KeysetId.cs (3)
  • GetBytes (79-82)
  • KeysetId (53-61)
  • KeysetId (63-66)
DotNut/ISecret.cs (2)
  • GetBytes (10-10)
  • ECPubKey (11-11)
🔇 Additional comments (2)
DotNut/P2PKProofSecret.cs (2)

111-155: Loop logic correctly addresses previous infinite-loop concerns.

The implementation properly handles key removal (line 114 computes remainingKeys, line 154 updates availableKeysLeft) and null handling (line 107 uses ?? 0), resolving the critical issues flagged in prior reviews. The signature generation flow with tweaked keys and negated variants aligns with the blinded proof specification.


130-130: No issues found — code correctly implements negative tweaking.

The manual scalar negation via .sec.Negate().Add(ri.sec) is the correct and recommended approach. NBitcoin's Secp256k1 doesn't expose a dedicated "negative tweak" helper; scalar negation (the seckey negate operation) is the intended pattern. The asymmetry with line 127's TweakAdd is justified since that API is for positive tweaks only.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
DotNut/HTLCBuilder.cs (1)

49-57: Track implementation of blinded HTLC construction.

Both BuildBlinded methods currently throw NotImplementedException. These stubs should be tracked to ensure they're implemented before the feature is considered complete.

Do you want me to open an issue to track the implementation of these blinded HTLC methods?

DotNut.Tests/UnitTest1.cs (1)

1-1: Remove unused using directive.

The System.Runtime.Intrinsics.Arm namespace is not used anywhere in this file.

Apply this diff:

-using System.Runtime.Intrinsics.Arm;
 using System.Security.Cryptography;
DotNut/HTLCProofSecret.cs (1)

60-86: Track implementation of blinded HTLC witness generation.

All five GenerateBlindWitness methods throw NotImplementedException. These should be implemented to complete the P2BK feature for HTLC secrets.

Do you want me to open an issue to track the implementation of these blinded HTLC witness methods?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5bd5f2 and bc0cc66.

📒 Files selected for processing (6)
  • DotNut.Tests/UnitTest1.cs (2 hunks)
  • DotNut/Cashu.cs (3 hunks)
  • DotNut/HTLCBuilder.cs (1 hunks)
  • DotNut/HTLCProofSecret.cs (4 hunks)
  • DotNut/P2PKProofSecret.cs (2 hunks)
  • DotNut/P2PkBuilder.cs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • DotNut/P2PkBuilder.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-08T01:49:27.288Z
Learnt from: d4rp4t
Repo: Kukks/DotNut PR: 21
File: DotNut/P2PkBuilder.cs:100-107
Timestamp: 2025-11-08T01:49:27.288Z
Learning: In the DotNut codebase, the PrivKey class (DotNut/PrivKey.cs) defines an implicit operator that converts PrivKey to ECPrivKey by returning privKey.Key. This allows PrivKey instances to be passed directly to methods expecting ECPrivKey parameters without explicit conversion.

Applied to files:

  • DotNut.Tests/UnitTest1.cs
  • DotNut/Cashu.cs
📚 Learning: 2025-09-17T17:11:38.738Z
Learnt from: d4rp4t
Repo: Kukks/DotNut PR: 16
File: DotNut/PubKey.cs:41-51
Timestamp: 2025-09-17T17:11:38.738Z
Learning: NBitcoin's ECPubKey (from NBitcoin.Secp256k1) implements value-based equality comparison through operator ==, not reference comparison. The operator compares the underlying cryptographic key material, making it suitable for structural equality implementations.

Applied to files:

  • DotNut/Cashu.cs
🧬 Code graph analysis (5)
DotNut.Tests/UnitTest1.cs (8)
DotNut/PrivKey.cs (3)
  • PrivKey (12-15)
  • PrivKey (17-20)
  • ToString (22-25)
DotNut/PubKey.cs (3)
  • PubKey (12-19)
  • PubKey (21-24)
  • ToString (26-29)
DotNut/KeysetId.cs (4)
  • ToString (68-71)
  • KeysetId (53-61)
  • KeysetId (63-66)
  • GetBytes (79-82)
DotNut/Cashu.cs (6)
  • Cashu (8-205)
  • ComputeZx (134-143)
  • ToBytes (180-185)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (145-156)
DotNut/Nut10Secret.cs (3)
  • GetBytes (28-33)
  • Nut10Secret (13-17)
  • Nut10Secret (19-22)
DotNut/P2PkBuilder.cs (5)
  • P2PkBuilder (6-158)
  • P2PkBuilder (55-98)
  • P2PKProofSecret (18-53)
  • P2PKProofSecret (108-113)
  • P2PKProofSecret (116-134)
DotNut/P2PKProofSecret.cs (4)
  • P2PKProofSecret (8-202)
  • VerifyWitness (167-171)
  • VerifyWitness (173-176)
  • VerifyWitness (178-182)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/HTLCBuilder.cs (3)
DotNut/KeysetId.cs (2)
  • KeysetId (53-61)
  • KeysetId (63-66)
DotNut/Keyset.cs (1)
  • KeysetId (13-63)
DotNut/Cashu.cs (12)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (145-156)
DotNut/Cashu.cs (4)
DotNut/Encoding/ConvertUtils.cs (2)
  • ECPrivKey (12-15)
  • ECPubKey (7-10)
DotNut/ISecret.cs (1)
  • ECPubKey (11-11)
DotNut/Nut10Secret.cs (1)
  • ECPubKey (35-38)
DotNut/StringSecret.cs (1)
  • ECPubKey (18-21)
DotNut/P2PKProofSecret.cs (6)
DotNut/HTLCProofSecret.cs (2)
  • P2PKWitness (164-167)
  • ECPubKey (14-25)
DotNut/P2PKWitness.cs (1)
  • P2PKWitness (5-8)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/Cashu.cs (19)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (145-156)
  • ToBytes (180-185)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
  • Cashu (8-205)
  • ComputeZx (134-143)
  • ToHex (175-178)
  • ToHex (193-196)
  • ToHex (197-200)
  • ToHex (201-204)
DotNut/KeysetId.cs (3)
  • GetBytes (79-82)
  • KeysetId (53-61)
  • KeysetId (63-66)
DotNut/BlindedMessage.cs (1)
  • BlindedMessage (5-11)
DotNut/HTLCProofSecret.cs (6)
DotNut/HTLCWitness.cs (1)
  • HTLCWitness (5-8)
DotNut/Proof.cs (1)
  • Proof (7-31)
DotNut/Cashu.cs (12)
  • ECPrivKey (56-59)
  • ECPrivKey (84-95)
  • ECPrivKey (145-156)
  • ECPubKey (16-20)
  • ECPubKey (22-26)
  • ECPubKey (27-42)
  • ECPubKey (51-54)
  • ECPubKey (61-64)
  • ECPubKey (66-69)
  • ECPubKey (71-75)
  • ECPubKey (77-81)
  • ECPubKey (128-132)
DotNut/P2PKProofSecret.cs (10)
  • ECPubKey (14-25)
  • P2PKWitness (28-31)
  • P2PKWitness (33-36)
  • P2PKWitness (38-42)
  • P2PKWitness (44-74)
  • P2PKWitness (82-86)
  • P2PKWitness (88-91)
  • P2PKWitness (93-96)
  • P2PKWitness (98-102)
  • P2PKWitness (104-164)
DotNut/BlindedMessage.cs (1)
  • BlindedMessage (5-11)
DotNut/P2PKWitness.cs (1)
  • P2PKWitness (5-8)
🔇 Additional comments (8)
DotNut.Tests/UnitTest1.cs (1)

657-766: LGTM! Comprehensive Nut26 test coverage.

The test validates key derivation, Zx computation, Ri computation, blinded public keys, and tweaked keys for both standard and negated paths. The test data matches the expected values from the specification.

DotNut/Cashu.cs (4)

12-15: LGTM! Cryptographic constants properly defined.

The P2BK_PREFIX domain separator and secp256k1 curve order N are correctly defined for the Pay to Blinded Key implementation.


134-143: LGTM! Correct x-coordinate extraction.

The method properly computes the shared secret point and extracts its x-coordinate using the xOnly pubkey format, which is standard for Schnorr signatures.


145-156: LGTM! Proper overflow handling for key derivation.

The method correctly:

  1. Hashes the inputs to derive a candidate private key
  2. Checks if the result is zero or >= N (invalid for secp256k1)
  3. Re-hashes with an extra byte (0xff) if needed to get a valid value

This ensures the derived key is always in the valid range [1, N-1].


159-173: LGTM! Efficient byte array concatenation.

The helper properly handles null/empty arrays and uses Buffer.BlockCopy for efficient byte copying.

DotNut/P2PKProofSecret.cs (2)

82-102: LGTM! Convenient overloads for blind witness generation.

These overloads provide a clean API surface, delegating to the core implementation while handling different input types (Proof, BlindedMessage, byte[], ECPrivKey).


104-164: LGTM! Correct blind witness generation logic.

The implementation properly:

  1. Computes Zx from the ephemeral key and P2PkE
  2. Derives per-slot blinding factors (Ri) deterministically
  3. Tweaks each signer key (both positive and negative cases)
  4. Checks tweaked keys against allowed pubkeys
  5. Signs and verifies signatures for matches
  6. Tracks used slots to avoid reuse
  7. Progresses through available keys (line 159) to prevent infinite loops

Based on learnings.

DotNut/HTLCProofSecret.cs (1)

131-161: LGTM! Proper obsolescence markers for base class overrides.

The obsolete overrides correctly prevent calling base class methods without the required preimage parameter, directing users to the appropriate HTLC-specific overloads.

Assert.Equal(1640995200, response3.Expiry);
Assert.Null(response3.PaymentPreimage);
}
private static readonly byte[] P2BK_PREFIX = "Cashu_P2BK_v1"u8.ToArray();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove duplicate P2BK_PREFIX constant.

This duplicates the P2BK_PREFIX defined in DotNut/Cashu.cs (line 12). Tests should reference the production constant to ensure they stay in sync.

Remove this field and update tests to use Cashu.P2BK_PREFIX instead:

-    private static readonly byte[] P2BK_PREFIX = "Cashu_P2BK_v1"u8.ToArray();
-
     [Fact]
     public void Nut26Tests()

Then update test code that references P2BK_PREFIX to use the fully qualified name or add a using static directive.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static readonly byte[] P2BK_PREFIX = "Cashu_P2BK_v1"u8.ToArray();
[Fact]
public void Nut26Tests()
🤖 Prompt for AI Agents
In DotNut.Tests/UnitTest1.cs around line 655, remove the duplicated private
static readonly byte[] P2BK_PREFIX field and update all test references to use
the production constant instead (Cashu.P2BK_PREFIX); either replace usages with
the fully qualified Cashu.P2BK_PREFIX or add a using static directive for
DotNut.Cashu at the top of the test file so tests reference the single source of
truth.

@Kukks Kukks merged commit 46f627e into Kukks:master Nov 21, 2025
1 check passed
@coderabbitai coderabbitai bot mentioned this pull request Nov 28, 2025
@d4rp4t d4rp4t deleted the feat/p2bk branch January 18, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants