Chore/bls12-381#36
Conversation
(maybe add a mandatory pairing validation b4 any interaction)
📝 WalkthroughWalkthroughThis PR introduces comprehensive BLS12-381 signature support (v3 keysets) to the DotNut Cashu library, alongside existing secp256k1 (v0/v1/v2). Changes span cryptographic primitives, polymorphic type representations, JSON serialization, derivation logic, integration points, and extensive test coverage. ChangesBLS v3 Keyset Implementation
Sequence Diagram(s)sequenceDiagram
participant Wallet as Wallet<br/>(Client)
participant BlsCashu as BlsCashu<br/>(Crypto)
participant Mint as Mint<br/>(Server)
Wallet->>BlsCashu: HashToCurveG1(secret) → Y
Wallet->>BlsCashu: BlindMessage(secret, r) → B_
Wallet->>Mint: POST /request_mint (B_)
Mint->>BlsCashu: CreateBlindSignature(B_, privkey) → C_
Mint->>Wallet: blind_signature: C_
Wallet->>BlsCashu: UnblindSignature(C_, r) → C
Wallet->>BlsCashu: VerifySignature(K2, C, secret) → true
Wallet->>Wallet: Store proof {C, secret}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DotNut/NUT01/Keyset.cs (1)
32-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard secp256k1-only assumption in v0 keyset ID derivation.
pair.Value.Key.ToBytes()now dereferences a nullable key. If a BLSPubKeyslips into a v0 keyset, this becomes aNullReferenceExceptioninstead of a clear contract error.💡 Proposed fix
var sortedBytes = sortedKeys - .Select(pair => pair.Value.Key.ToBytes()) + .Select(pair => pair.Value.Key?.ToBytes() + ?? throw new ArgumentException("Version 0x00 keyset IDs require secp256k1 public keys only.")) .SelectMany(b => b) .ToArray();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNut/NUT01/Keyset.cs` around lines 32 - 35, The current v0 keyset ID derivation dereferences nullable keys when building sortedBytes (using pair.Value.Key.ToBytes()), which can cause a NullReferenceException if a BLS PubKey is present; update the logic around sortedKeys/sortedBytes in Keyset.cs to explicitly validate each pair.Value.Key is non-null and of the expected secp256k1 kind (or throw a clear contract exception) before calling ToBytes(), and fail fast with a descriptive error when a non-secp256k1 or null key is encountered to preserve the secp256k1-only assumption.DotNut/Encoding/CashuTokenV4Encoder.cs (1)
109-109:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDecoding does not handle BLS G1 points, causing asymmetric encode/decode.
The encoding at line 44 correctly branches on
IsBlsG1to emit compressed BLS G1 bytes (48 bytes), but decoding unconditionally usesECPubKey.Create(...)which expects 33-byte secp256k1 compressed keys. BLS v3 tokens will fail to decode with an exception fromECPubKey.Create.The decoder needs to detect the key type (by length or by inspecting the keyset ID prefix) and construct the appropriate
PubKey.🐛 Proposed fix
- C = ECPubKey.Create(proof["c"].GetByteString()), + C = id.IsBlsKeyset() + ? new PubKey(Convert.ToHexString(proof["c"].GetByteString()).ToLowerInvariant()) + : ECPubKey.Create(proof["c"].GetByteString()),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNut/Encoding/CashuTokenV4Encoder.cs` at line 109, The decoder in CashuTokenV4Encoder currently always calls ECPubKey.Create(...) for proof["c"], which breaks for BLS G1 keys; change the decode path to detect key type (check IsBlsG1 flag on the token or inspect proof["c"].GetByteString().Length == 48 vs 33 or keyset-id prefix) and construct the matching PubKey implementation instead of unconditionally using ECPubKey.Create; when BLS/G1 is detected, call the BLS/G1 public-key constructor (the project’s BLS pubkey factory/constructor) to create the PubKey, otherwise keep using ECPubKey.Create so encode/decode are symmetric.
🧹 Nitpick comments (4)
DotNut.Tests/Unit/BlsTests.cs (2)
148-174: 💤 Low valueInconsistent indentation on line 159.
Line 159 has 4-space indentation instead of 8-space like the surrounding code.
🧹 Proposed fix
- var B_ = BlsCashu.BlindMessage(secretBytes, r); + var B_ = BlsCashu.BlindMessage(secretBytes, r);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNut.Tests/Unit/BlsTests.cs` around lines 148 - 174, Fix the inconsistent indentation inside the test method BatchVerifySignatures_OneForgedSignature_ReturnsFalse: align the line that declares var B_ = BlsCashu.BlindMessage(secretBytes, r); to use the same 8-space indentation as the surrounding block so it matches the other statements inside the for loop (and keep indentation consistent for related lines using variables r, B_, C_, C and items.Add).
111-146: 💤 Low valueInconsistent indentation on line 139.
Line 139 has 4-space indentation instead of 8-space like the surrounding code.
🧹 Proposed fix
- var B_ = BlsCashu.BlindMessage(secretBytes, r); + var B_ = BlsCashu.BlindMessage(secretBytes, r);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNut.Tests/Unit/BlsTests.cs` around lines 111 - 146, In the BatchVerifySignatures_MultipleValidItems test, one line inside the loop (the BlsCashu.BlindMessage call that assigns B_) is indented with 4 spaces instead of matching the surrounding 8-space block; fix by adjusting that line's indentation to align with the other statements in the for loop (inside the method BatchVerifySignatures_MultipleValidItems in BlsTests.cs) so formatting is consistent.DotNut/Abstractions/Utils.cs (1)
2-2: 💤 Low valueUnused import.
System.Textappears to be unused in this file. TheEncodingclass is not referenced anywhere in the visible code.🧹 Proposed fix
-using System.Text;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNut/Abstractions/Utils.cs` at line 2, Remove the unused using directive "using System.Text" from the top of Utils.cs; locate the using statement in the file (near the namespace/class Utils declaration) and delete it, ensuring no references to Encoding or System.Text remain in methods like any helper functions inside the Utils class.DotNut/Abstractions/SwapBuilder.cs (1)
152-159: ⚡ Quick winBLS proofs receive no client-side verification when
_verifyDleqis true.When
_verifyDleqis enabled, secp256k1 proofs get DLEQ verification, but BLS proofs skip the entire verification block viacontinue. If BLS signature verification should happen client-side (similar to how secp256k1 proofs are verified), consider adding a BLS-specific verification path usingBlsCashu.VerifySignature.If verification is intentionally deferred (e.g., performed during proof construction in
ConstructBlsProofFromPromise), a brief comment clarifying this design decision would help maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNut/Abstractions/SwapBuilder.cs` around lines 152 - 159, The current block skips client-side verification for BLS proofs (proof.DLEQ == null && proof.Id.IsBlsKeyset()) which causes BLS signatures to be unverified when _verifyDleq is true; update the logic in SwapBuilder to either (a) add a BLS-specific verification path that calls BlsCashu.VerifySignature for BLS keysets when proof.DLEQ is null and _verifyDleq is enabled, locating the check around proof.DLEQ and proof.Id.IsBlsKeyset(), or (b) if verification is intentionally done elsewhere (e.g., ConstructBlsProofFromPromise), replace the continue with a concise comment referencing ConstructBlsProofFromPromise to document why client-side verification is skipped; ensure the behavior is gated by _verifyDleq and that any thrown exceptions remain for unexpected missing proofs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitmodules:
- Around line 1-3: The .gitmodules entry adds a submodule named
"DotNut.BLS12-381" pointing to a personal GitHub repo; replace or validate this
by either swapping the submodule to an established, audited BLS12-381
implementation (e.g., Neo.Cryptography.BLS12_381 NuGet package or blst with C#
bindings) or documenting and approving the security review for DotNut.BLS12-381;
update the submodule reference in .gitmodules (submodule "DotNut.BLS12-381")
accordingly and ensure build/CI uses the chosen source and that dependency
provenance and maintenance rationale are recorded.
In `@DotNut.BLS12-381`:
- Line 1: The submodule entry DotNut.BLS12-381 points to a non-existent commit
9eaeb6730144dd2008663d859a0c008b9176432f; update the submodule reference in
.gitmodules (and the gitlink in the index) to a valid commit or branch/tag
present in https://github.com/d4rp4t/DotNut.BLS12-381.git, then run git
submodule sync && git submodule update --init --recursive (or set the submodule
to track a branch and update that ref); ensure the commit hash you set actually
exists in the remote and commit the .gitmodules/index change so CI can clone the
submodule successfully.
In `@DotNut/Crypto/BlsCashu.cs`:
- Around line 26-30: BlindMessage accepts a zero blinding scalar which yields
the point at infinity; change BlindMessage to convert r to a Scalar (use
Scalar.FromBytesBigEndian or equivalent), check that the resulting scalar is not
zero (Scalar.IsZero or compare to Scalar.Zero) and throw an ArgumentException
(or other fast-fail) if it is zero before computing HashToCurveG1(secret) *
scalar; reference BlindMessage, HashToCurveG1, and Scalar.FromBytesBigEndian
when applying the check.
In `@DotNut/JsonConverters/BlsKeysetJsonConverter.cs`:
- Around line 35-43: The code currently uses ulong.Parse and reads
reader.GetString() without verifying the token type; update the Read logic in
BlsKeysetJsonConverter (the branch handling reader.TokenType ==
JsonTokenType.String or PropertyName and the subsequent hex-reading branch) to
first validate the token type before calling reader.GetString(), then replace
ulong.Parse(s) with ulong.TryParse(s, out amount) and throw a JsonException if
TryParse returns false (so parsing failures produce JsonException rather than
FormatException/OverflowException); apply the same pattern to any other
numeric/hex parsing in the method to ensure consistent JsonException error
handling.
In `@DotNut/PubKey.cs`:
- Around line 26-34: The hex constructor for PubKey currently accepts
96/192-char BLS compressed byte arrays and stores them in
_blsG1Bytes/_blsG2Bytes without validating the decoded points or rejecting the
infinity point, which is inconsistent with PubKey(G1Affine)/PubKey(G2Affine);
update the hex constructor to decode the bytes, convert to G1/G2 affine points
and throw the same error used by PubKey(G1Affine)/PubKey(G2Affine) if the point
is infinity, and update GetBlsG1Point() and GetBlsG2Point() to likewise parse
the stored bytes into affine points and throw on infinity (mirroring the checks
in BlsG1PubKey and BlsG2PubKey) so all paths uniformly reject infinity points.
---
Outside diff comments:
In `@DotNut/Encoding/CashuTokenV4Encoder.cs`:
- Line 109: The decoder in CashuTokenV4Encoder currently always calls
ECPubKey.Create(...) for proof["c"], which breaks for BLS G1 keys; change the
decode path to detect key type (check IsBlsG1 flag on the token or inspect
proof["c"].GetByteString().Length == 48 vs 33 or keyset-id prefix) and construct
the matching PubKey implementation instead of unconditionally using
ECPubKey.Create; when BLS/G1 is detected, call the BLS/G1 public-key constructor
(the project’s BLS pubkey factory/constructor) to create the PubKey, otherwise
keep using ECPubKey.Create so encode/decode are symmetric.
In `@DotNut/NUT01/Keyset.cs`:
- Around line 32-35: The current v0 keyset ID derivation dereferences nullable
keys when building sortedBytes (using pair.Value.Key.ToBytes()), which can cause
a NullReferenceException if a BLS PubKey is present; update the logic around
sortedKeys/sortedBytes in Keyset.cs to explicitly validate each pair.Value.Key
is non-null and of the expected secp256k1 kind (or throw a clear contract
exception) before calling ToBytes(), and fail fast with a descriptive error when
a non-secp256k1 or null key is encountered to preserve the secp256k1-only
assumption.
---
Nitpick comments:
In `@DotNut.Tests/Unit/BlsTests.cs`:
- Around line 148-174: Fix the inconsistent indentation inside the test method
BatchVerifySignatures_OneForgedSignature_ReturnsFalse: align the line that
declares var B_ = BlsCashu.BlindMessage(secretBytes, r); to use the same 8-space
indentation as the surrounding block so it matches the other statements inside
the for loop (and keep indentation consistent for related lines using variables
r, B_, C_, C and items.Add).
- Around line 111-146: In the BatchVerifySignatures_MultipleValidItems test, one
line inside the loop (the BlsCashu.BlindMessage call that assigns B_) is
indented with 4 spaces instead of matching the surrounding 8-space block; fix by
adjusting that line's indentation to align with the other statements in the for
loop (inside the method BatchVerifySignatures_MultipleValidItems in BlsTests.cs)
so formatting is consistent.
In `@DotNut/Abstractions/SwapBuilder.cs`:
- Around line 152-159: The current block skips client-side verification for BLS
proofs (proof.DLEQ == null && proof.Id.IsBlsKeyset()) which causes BLS
signatures to be unverified when _verifyDleq is true; update the logic in
SwapBuilder to either (a) add a BLS-specific verification path that calls
BlsCashu.VerifySignature for BLS keysets when proof.DLEQ is null and _verifyDleq
is enabled, locating the check around proof.DLEQ and proof.Id.IsBlsKeyset(), or
(b) if verification is intentionally done elsewhere (e.g.,
ConstructBlsProofFromPromise), replace the continue with a concise comment
referencing ConstructBlsProofFromPromise to document why client-side
verification is skipped; ensure the behavior is gated by _verifyDleq and that
any thrown exceptions remain for unexpected missing proofs.
In `@DotNut/Abstractions/Utils.cs`:
- Line 2: Remove the unused using directive "using System.Text" from the top of
Utils.cs; locate the using statement in the file (near the namespace/class Utils
declaration) and delete it, ensuring no references to Encoding or System.Text
remain in methods like any helper functions inside the Utils class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 082af3f6-3ec8-4c47-880a-73156fcec681
📒 Files selected for processing (22)
.gitmodulesDotNut.BLS12-381DotNut.Tests/Unit/BlsTests.csDotNut.slnDotNut/Abstractions/SwapBuilder.csDotNut/Abstractions/Utils.csDotNut/ApiModels/GetKeysResponse.csDotNut/BlsG1PubKey.csDotNut/BlsG2PubKey.csDotNut/Crypto/BlsCashu.csDotNut/DotNut.csprojDotNut/Encoding/CashuTokenV4Encoder.csDotNut/JsonConverters/AnyKeysetJsonConverter.csDotNut/JsonConverters/BlsKeysetJsonConverter.csDotNut/JsonConverters/KeysetJsonConverter.csDotNut/JsonConverters/Nut10SecretJsonConverter.csDotNut/JsonConverters/PubKeyJsonConverter.csDotNut/NUT01/BlsKeyset.csDotNut/NUT01/Keyset.csDotNut/NUT02/KeysetId.csDotNut/NUT13/Nut13.csDotNut/PubKey.cs
| [submodule "DotNut.BLS12-381"] | ||
| path = DotNut.BLS12-381 | ||
| url = https://github.com/d4rp4t/DotNut.BLS12-381.git |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the submodule repository is accessible
echo "=== Checking submodule repository accessibility ==="
curl -s -o /dev/null -w "%{http_code}" https://github.com/d4rp4t/DotNut.BLS12-381 | grep -q "200" && echo "Repository is accessible" || echo "Repository may not be accessible"
# Check repository metadata
echo -e "\n=== Repository metadata ==="
gh repo view d4rp4t/DotNut.BLS12-381 --json name,description,createdAt,pushedAt,stargazerCount,forkCount,isArchived,isFork,parent || echo "Failed to fetch repository metadata"Repository: Kukks/DotNut
Length of output: 357
🌐 Web query:
BLS12-381 .NET library audited cryptographic
💡 Result:
There isn’t evidence (from authoritative/official sources) that a specific.NET library implementing BLS12-381 has been independently “audited cryptographically” (i.e., has a published third-party audit report) as a.NET library. What I did find: 1) Neo.Cryptography.BLS12_381 (.NET) - The NuGet package exists (example: Neo.Cryptography.BLS12_381 3.9.0) [1]. - The repository’s GitHub “Security” page shows no published security advisories and indicates no SECURITY.md policy file is detected [2]. This is not a cryptographic audit confirmation. 2) BLS12-381 audit evidence exists for non-.NET implementations, and sometimes they have C#/bindings - The supranational blst library (C/assembly; provides C# bindings via SWIG) states: “An initial audit of this library was conducted by NCC Group in January 2021” and links to where the audit is “here” [3]. This is evidence of third-party audit for the underlying library, but it is not an audit claim specifically for any.NET wrapper quality. 3) gnark-crypto - gnark-crypto’s README states: “gnark-crypto is not fully audited … [and] makes no security guarantees such as constant time … or side-channel attack resistance” [4]. So even if audit reports exist for certain curves (including bls12-381), it does not amount to a fully-audited security claim for the whole package. Conclusion - For “BLS12-381.NET library audited cryptographic”: the closest directly sourced third-party audit evidence I found is for the underlying blst implementation (NCC Group audit in Jan 2021) and that it offers C# bindings [3]. For.NET libraries specifically (e.g., Neo.Cryptography.BLS12_381), I found no published third-party cryptographic audit status in the sources retrieved. If you share the exact NuGet package name (and version), I can re-check whether it references a specific audit (for the exact code version) or whether there are any published third-party audit reports tied to that.NET library.
Citations:
- 1: https://www.nuget.org/packages/Neo.Cryptography.BLS12_381/
- 2: https://github.com/neo-project/Neo.Cryptography.BLS12_381/security
- 3: https://github.com/supranational/blst
- 4: https://github.com/consensys/gnark-crypto
Verify the cryptographic library source and consider established alternatives.
The submodule points to a personal repository (d4rp4t/DotNut.BLS12-381) for BLS12-381 cryptographic operations. While the repository is accessible and recently active (last updated 2026-05-20), using a novel, community-unreviewed implementation for cryptographic operations raises significant concerns:
- Supply chain security: This is a novel implementation (not a fork of an established library) with zero community adoption (0 stars, 0 forks). Cryptographic libraries require rigorous security review and established trust.
- Availability risk: Personal repositories can be deleted, made private, or renamed, breaking builds for all users.
- Maintenance: Long-term maintenance and security updates may not be guaranteed.
Verify whether established .NET BLS12-381 libraries meet your requirements before committing to this implementation. Known alternatives include:
- Neo.Cryptography.BLS12_381 — Available on NuGet with version history
- Supranational blst — C library with C# bindings; underwent third-party audit by NCC Group (January 2021)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitmodules around lines 1 - 3, The .gitmodules entry adds a submodule named
"DotNut.BLS12-381" pointing to a personal GitHub repo; replace or validate this
by either swapping the submodule to an established, audited BLS12-381
implementation (e.g., Neo.Cryptography.BLS12_381 NuGet package or blst with C#
bindings) or documenting and approving the security review for DotNut.BLS12-381;
update the submodule reference in .gitmodules (submodule "DotNut.BLS12-381")
accordingly and ensure build/CI uses the chosen source and that dependency
provenance and maintenance rationale are recorded.
| @@ -0,0 +1 @@ | |||
| Subproject commit 9eaeb6730144dd2008663d859a0c008b9176432f | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify submodule configuration and commit accessibility
# Check .gitmodules for repository URL
echo "=== Submodule Configuration ==="
cat .gitmodules | grep -A 2 "DotNut.BLS12-381"
# Verify the submodule commit exists and is accessible
cd DotNut.BLS12-381 2>/dev/null || {
echo "ERROR: Submodule not initialized"
exit 1
}
echo -e "\n=== Commit Information ==="
git log -1 --format="%H%n%an <%ae>%n%ad%n%s" 9eaeb6730144dd2008663d859a0c008b9176432f
echo -e "\n=== Remote Repository ==="
git remote get-url origin
# Check recent commits/tags to see if this is latest
echo -e "\n=== Recent Tags ==="
git tag --sort=-version:refname | head -5
echo -e "\n=== Commits Since Referenced Commit ==="
git log --oneline 9eaeb6730144dd2008663d859a0c008b9176432f..HEAD 2>/dev/null || echo "This is the latest commit in the current branch"Repository: Kukks/DotNut
Length of output: 589
The submodule references a non-existent commit in the target repository.
The commit 9eaeb6730144dd2008663d859a0c008b9176432f does not exist in https://github.com/d4rp4t/DotNut.BLS12-381.git. Verification failed with: fatal: remote error: upload-pack: not our ref.
This prevents cloning and initializing the submodule. Confirm the commit hash is correct or use a valid commit that exists in the target repository.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNut.BLS12-381` at line 1, The submodule entry DotNut.BLS12-381 points to a
non-existent commit 9eaeb6730144dd2008663d859a0c008b9176432f; update the
submodule reference in .gitmodules (and the gitlink in the index) to a valid
commit or branch/tag present in https://github.com/d4rp4t/DotNut.BLS12-381.git,
then run git submodule sync && git submodule update --init --recursive (or set
the submodule to track a branch and update that ref); ensure the commit hash you
set actually exists in the remote and commit the .gitmodules/index change so CI
can clone the submodule successfully.
| public static G1Affine BlindMessage(byte[] secret, byte[] r) | ||
| { | ||
| var Y = HashToCurveG1(secret); | ||
| return (Y.ToProjective() * Scalar.FromBytesBigEndian(r)).ToAffine(); | ||
| } |
There was a problem hiding this comment.
Reject zero blinding factor in BlindMessage.
BlindMessage currently accepts r = 0 mod Fr, which yields the point at infinity and creates invalid blinded messages instead of failing fast.
💡 Proposed fix
public static G1Affine BlindMessage(byte[] secret, byte[] r)
{
var Y = HashToCurveG1(secret);
- return (Y.ToProjective() * Scalar.FromBytesBigEndian(r)).ToAffine();
+ var rScalar = Scalar.FromBytesBigEndian(r);
+ if (Scalar.IsZero(rScalar))
+ throw new ArgumentException("Blinding factor reduces to zero mod Fr");
+ return (Y.ToProjective() * rScalar).ToAffine();
}📝 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.
| public static G1Affine BlindMessage(byte[] secret, byte[] r) | |
| { | |
| var Y = HashToCurveG1(secret); | |
| return (Y.ToProjective() * Scalar.FromBytesBigEndian(r)).ToAffine(); | |
| } | |
| public static G1Affine BlindMessage(byte[] secret, byte[] r) | |
| { | |
| var Y = HashToCurveG1(secret); | |
| var rScalar = Scalar.FromBytesBigEndian(r); | |
| if (Scalar.IsZero(rScalar)) | |
| throw new ArgumentException("Blinding factor reduces to zero mod Fr"); | |
| return (Y.ToProjective() * rScalar).ToAffine(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNut/Crypto/BlsCashu.cs` around lines 26 - 30, BlindMessage accepts a zero
blinding scalar which yields the point at infinity; change BlindMessage to
convert r to a Scalar (use Scalar.FromBytesBigEndian or equivalent), check that
the resulting scalar is not zero (Scalar.IsZero or compare to Scalar.Zero) and
throw an ArgumentException (or other fast-fail) if it is zero before computing
HashToCurveG1(secret) * scalar; reference BlindMessage, HashToCurveG1, and
Scalar.FromBytesBigEndian when applying the check.
| else if (reader.TokenType is JsonTokenType.String or JsonTokenType.PropertyName) | ||
| { | ||
| var s = reader.GetString(); | ||
| if (string.IsNullOrEmpty(s)) | ||
| { | ||
| throw new JsonException("Expected string key"); | ||
| } | ||
| amount = ulong.Parse(s); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In .NET 8, what exceptions can ulong.Parse(string)throw, and isulong.TryParsepreferred inSystem.Text.Jsonconverters to preserveJsonException-based error handling?
💡 Result:
In.NET 8, calling ulong.Parse(string) can throw the following exceptions: - ArgumentNullException: Thrown if the input string is null [1]. - FormatException: Thrown if the input string is not in the correct numeric format [1]. - OverflowException: Thrown if the value represented by the string is less than UInt64.MinValue or greater than UInt64.MaxValue [1]. Regarding System.Text.Json custom converters, using ulong.TryParse is often preferred, but not specifically to preserve JsonException behavior; rather, it is a defensive programming best practice to handle invalid data gracefully [2]. System.Text.Json itself relies on JsonException to communicate deserialization failures [3][4]. If you are writing a custom converter, you should throw a JsonException when data is malformed or invalid [5]. Using ulong.TryParse allows you to detect parsing failures without relying on the overhead of catching exceptions [2]. If TryParse fails, you can then manually throw a descriptive JsonException, which ensures that your custom converter remains consistent with the framework's standard error reporting mechanism [3].
Citations:
- 1: https://msdn.microsoft.com/library/9czfdcyy(v=vs.95)
- 2: https://www.dotnetperls.com/long
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonexception?view=net-7.0
- 4: https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonexception?view=net-6.0
- 5: https://www.benday.com/blog/fixing-the-dreaded-jsonconverter-jsonexception-converter-read-too-much-or-not-enough
🏁 Script executed:
find . -name "BlsKeysetJsonConverter.cs" -type fRepository: Kukks/DotNut
Length of output: 106
🏁 Script executed:
cat -n ./DotNut/JsonConverters/BlsKeysetJsonConverter.cs | head -60Repository: Kukks/DotNut
Length of output: 2287
Use TryParse for amount parsing and validate token type before reading hex value.
On line 42, ulong.Parse can throw FormatException or OverflowException instead of JsonException, breaking error consistency. Use TryParse and throw JsonException on failure.
On line 49-50, the code calls reader.GetString() without validating the token type first. Add a token type check before reading the hex value to prevent unexpected exceptions.
🔧 Proposed fix
- amount = ulong.Parse(s);
+ if (!ulong.TryParse(s, out amount))
+ {
+ throw new JsonException($"Invalid amount key: '{s}'");
+ } reader.Read();
+ if (reader.TokenType != JsonTokenType.String)
+ {
+ throw new JsonException("Expected string value for public key");
+ }
var hex = reader.GetString();📝 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.
| else if (reader.TokenType is JsonTokenType.String or JsonTokenType.PropertyName) | |
| { | |
| var s = reader.GetString(); | |
| if (string.IsNullOrEmpty(s)) | |
| { | |
| throw new JsonException("Expected string key"); | |
| } | |
| amount = ulong.Parse(s); | |
| } | |
| else if (reader.TokenType is JsonTokenType.String or JsonTokenType.PropertyName) | |
| { | |
| var s = reader.GetString(); | |
| if (string.IsNullOrEmpty(s)) | |
| { | |
| throw new JsonException("Expected string key"); | |
| } | |
| if (!ulong.TryParse(s, out amount)) | |
| { | |
| throw new JsonException($"Invalid amount key: '{s}'"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNut/JsonConverters/BlsKeysetJsonConverter.cs` around lines 35 - 43, The
code currently uses ulong.Parse and reads reader.GetString() without verifying
the token type; update the Read logic in BlsKeysetJsonConverter (the branch
handling reader.TokenType == JsonTokenType.String or PropertyName and the
subsequent hex-reading branch) to first validate the token type before calling
reader.GetString(), then replace ulong.Parse(s) with ulong.TryParse(s, out
amount) and throw a JsonException if TryParse returns false (so parsing failures
produce JsonException rather than FormatException/OverflowException); apply the
same pattern to any other numeric/hex parsing in the method to ensure consistent
JsonException error handling.
| if (hex.Length == 96) // BLS G1 compressed: 48 bytes | ||
| { | ||
| throw new ArgumentException("Only compressed public keys are allowed"); | ||
| _blsG1Bytes = Convert.FromHexString(hex); | ||
| return; | ||
| } | ||
| if (hex.Length == 192) // BLS G2 compressed: 96 bytes | ||
| { | ||
| _blsG2Bytes = Convert.FromHexString(hex); | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PubKey.cs" -type fRepository: Kukks/DotNut
Length of output: 75
🏁 Script executed:
cat -n ./DotNut/PubKey.cs | head -60Repository: Kukks/DotNut
Length of output: 2322
🏁 Script executed:
cat -n ./DotNut/PubKey.cs | sed -n '60,100p'Repository: Kukks/DotNut
Length of output: 2026
🏁 Script executed:
find . -name "*.cs" -type f | xargs grep -l "class G1Affine\|struct G1Affine" | head -5Repository: Kukks/DotNut
Length of output: 38
🏁 Script executed:
rg "G1Affine.*TryFromCompressed" -A 2 -B 2Repository: Kukks/DotNut
Length of output: 1091
🏁 Script executed:
rg "G2Affine.*TryFromCompressed" -A 2 -B 2Repository: Kukks/DotNut
Length of output: 1105
🏁 Script executed:
rg "_blsG1Bytes|_blsG2Bytes" -B 2 -A 2Repository: Kukks/DotNut
Length of output: 4135
🏁 Script executed:
rg "new PubKey\(" | head -20Repository: Kukks/DotNut
Length of output: 1884
Add BLS infinity point validation to the hex constructor and getter methods.
The hex constructor accepts 96/192-char payloads and stores them as raw bytes without validating that they represent valid curve points or checking for infinity points. This creates an inconsistency with the internal constructors PubKey(G1Affine) and PubKey(G2Affine), which explicitly reject infinity points. Additionally, GetBlsG1Point() and GetBlsG2Point() should validate against infinity, consistent with the pattern used in BlsG1PubKey and BlsG2PubKey.
💡 Proposed fix
if (hex.Length == 96) // BLS G1 compressed: 48 bytes
{
- _blsG1Bytes = Convert.FromHexString(hex);
+ var g1 = Convert.FromHexString(hex);
+ if (!G1Affine.TryFromCompressed(g1, out var p) || p.IsInfinity)
+ throw new ArgumentException("Invalid BLS G1 compressed public key");
+ _blsG1Bytes = g1;
return;
}
if (hex.Length == 192) // BLS G2 compressed: 96 bytes
{
- _blsG2Bytes = Convert.FromHexString(hex);
+ var g2 = Convert.FromHexString(hex);
+ if (!G2Affine.TryFromCompressed(g2, out var p) || p.IsInfinity)
+ throw new ArgumentException("Invalid BLS G2 compressed public key");
+ _blsG2Bytes = g2;
return;
}Also add infinity checks to the getter methods to match BlsG1PubKey/BlsG2PubKey:
if (!G1Affine.TryFromCompressed(_blsG1Bytes, out var p))
throw new InvalidOperationException("Stored BLS G1 bytes are invalid");
+if (p.IsInfinity)
+ throw new InvalidOperationException("BLS G1 point at infinity is not valid");
return p;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNut/PubKey.cs` around lines 26 - 34, The hex constructor for PubKey
currently accepts 96/192-char BLS compressed byte arrays and stores them in
_blsG1Bytes/_blsG2Bytes without validating the decoded points or rejecting the
infinity point, which is inconsistent with PubKey(G1Affine)/PubKey(G2Affine);
update the hex constructor to decode the bytes, convert to G1/G2 affine points
and throw the same error used by PubKey(G1Affine)/PubKey(G2Affine) if the point
is infinity, and update GetBlsG1Point() and GetBlsG2Point() to likewise parse
the stored bytes into affine points and throw on infinity (mirroring the checks
in BlsG1PubKey and BlsG2PubKey) so all paths uniformly reject infinity points.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Tests