NUT-20/29: length-prefixed quote signature message - #38
Open
d4rp4t wants to merge 4 commits into
Open
Conversation
cashubtc/nuts@b969c4aa replaced the concatenated quote id and hex outputs with a domain-separated, length-prefixed commitment over raw bytes: "Cashu_MintQuoteSig_v1" || len32(quote) || quote || (len32(amount) || amount || len32(B_) || B_) per output Amounts are canonical minimal big-endian, so zero is empty, and B_ is the decoded point rather than its hex string. The old vector in the tests was signed over the old message and is replaced by the one from tests/20-test.md, asserting msg_to_sign and its hash directly so a mistake in the length prefixes is visible on its own. This is breaking against mints still on the old message: cdk-mintd 0.15.1 rejects these signatures, and NUT-20 advertises no version to negotiate on.
No released cdk-mintd understands the domain-separated message yet — it is on main but in no tag up to v0.17.3 — so signing only the current way makes NUT-20 minting fail against every mint deployed today, including the one CI starts. Mints sign the current message first and, when the mint answers 20008, sign the same outputs again the old way and retry once. Nothing is issued on a rejected signature, so the retry cannot double-spend. Verification accepts either, with allowLegacy to refuse. This mirrors cdk, which added sign_legacy for wallets and a legacy branch in verify_signature, and nutshell, which has verified both since 0.20.2. Removable once mints without the current format are gone; nutshell already marks its legacy path deprecated.
The signature error code sat on MintQuoteSigner, which is not where callers would look for it. Lifts it next to CashuProtocolError, along with the rest of the codes from the spec's error_codes.md.
NUT-29 does not define a message of its own: signatures[i] is a NUT-20 signature over quotes[i] and the batch's whole outputs array, since the outputs are one consolidated set rather than partitioned per quote. So the message fix carries over, and what was missing was assembling the array — one entry per quote, null for the unlocked ones, and the field left out when nothing is locked. Covered by the vector from tests/29-tests.md.
Contributor
📝 WalkthroughWalkthroughThe PR implements domain-separated NUT-20 mint quote signatures with legacy compatibility, adds batched NUT-29 signing, defines Cashu error codes, and retries mint requests with legacy signatures when required. ChangesMint signature flows
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MintHandler
participant MintQuoteSigner
participant Mint
MintHandler->>MintQuoteSigner: SignMintQuote
MintHandler->>Mint: Submit mint request
Mint-->>MintHandler: MintRequestSignatureInvalid
MintHandler->>MintQuoteSigner: SignMintQuoteLegacy
MintHandler->>Mint: Retry mint request
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
DotNut/Abstractions/Handlers/MintHandlerBolt11.cs (1)
59-77: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDuplicated retry-with-legacy-signature logic in both handlers; shared safety assumption unverified.
Both handlers implement the identical catch-and-retry pattern for
CashuErrorCodes.MintRequestSignatureInvalid, re-signing the same outputs withSignMintQuoteLegacy. The root cause is one piece of logic copy-pasted into two files, and both copies rely on the same unverified external contract: that the mint does not issue promises before rejecting a request for an invalid NUT-20 signature.
DotNut/Abstractions/Handlers/MintHandlerBolt11.cs#L59-L77: confirm this ordering guarantee against the NUT-04/NUT-20 spec (or the target mints), then extract the try/catch-and-retry block into a shared helper (for example, a static method taking the mint client, endpoint name, request, and signing key) that both handlers call.DotNut/Abstractions/Handlers/MintHandlerBolt12.cs#L60-L75: replace this near-identical block with a call to the same shared helper once extracted, and rely on the same verification performed for the Bolt11 handler.🤖 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/Handlers/MintHandlerBolt11.cs` around lines 59 - 77, The Bolt11 and Bolt12 handlers duplicate legacy-signature retry logic and rely on an unverified mint ordering guarantee. Verify the NUT-04/NUT-20 or target-mint guarantee that invalid NUT-20 signatures issue no promises, then extract the shared try/catch-and-retry behavior into a helper accepting the mint client, endpoint, request, and signing key; update MintHandlerBolt11.cs lines 59-77 and MintHandlerBolt12.cs lines 60-75 to call that helper.
🤖 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 `@DotNut/NUT20/MintQuoteSigner.cs`:
- Around line 113-136: Update VerifySignature to catch FormatException from
Convert.FromHexString when parsing quote.Signature, returning false for
malformed odd-length or non-hex input before invoking
SecpSchnorrSignature.TryCreate. Preserve the existing signature verification and
allowLegacy behavior for valid input.
---
Nitpick comments:
In `@DotNut/Abstractions/Handlers/MintHandlerBolt11.cs`:
- Around line 59-77: The Bolt11 and Bolt12 handlers duplicate legacy-signature
retry logic and rely on an unverified mint ordering guarantee. Verify the
NUT-04/NUT-20 or target-mint guarantee that invalid NUT-20 signatures issue no
promises, then extract the shared try/catch-and-retry behavior into a helper
accepting the mint client, endpoint, request, and signing key; update
MintHandlerBolt11.cs lines 59-77 and MintHandlerBolt12.cs lines 60-75 to call
that helper.
🪄 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 Plus
Run ID: 0587c09e-3777-4dc0-9bae-d5ae2dff629e
📒 Files selected for processing (7)
DotNut.Tests/Nut20Tests.csDotNut.Tests/Unit/Nut29Tests.csDotNut/Abstractions/Handlers/MintHandlerBolt11.csDotNut/Abstractions/Handlers/MintHandlerBolt12.csDotNut/Api/CashuErrorCodes.csDotNut/NUT20/MintQuoteSigner.csDotNut/NUT29/BatchedMintQuoteSigner.cs
Comment on lines
+113
to
136
| /// <param name="allowLegacy"> | ||
| /// Also accept a signature over the superseded message, the way cdk and nutshell do, so | ||
| /// that wallets which have not upgraded can still mint. | ||
| /// </param> | ||
| public static bool VerifySignature(this PostMintRequest quote, PubKey pk, bool allowLegacy = true) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(quote.Signature, nameof(quote.Signature)); | ||
| if (!SecpSchnorrSignature.TryCreate(Convert.FromHexString(quote.Signature), out var sig)) | ||
| { | ||
| return false; | ||
| } | ||
| return xonly.SigVerifyBIP340(sig, hash); | ||
|
|
||
| var xonly = pk.Key.ToXOnlyPubKey(); | ||
| if (xonly.SigVerifyBIP340(sig, SHA256.HashData(GetMessageToSign(quote.Quote, quote.Outputs)))) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return allowLegacy | ||
| && xonly.SigVerifyBIP340( | ||
| sig, | ||
| SHA256.HashData(GetLegacyMessageToSign(quote.Quote, quote.Outputs)) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find file =="
fd -a 'MintQuoteSigner.cs' . || true
echo "== file outline =="
ast-grep outline DotNut/NUT20/MintQuoteSigner.cs --view expanded || true
echo "== relevant lines =="
nl -ba DotNut/NUT20/MintQuoteSigner.cs | sed -n '1,180p'
echo "== project target/framework/package hints =="
find . -maxdepth 3 \( -name '*.csproj' -o -name 'Directory.Build.props' -o -name 'Directory.Packages.props' \) -print | sort | xargs -r sed -n '1,160p'Repository: Kukks/DotNut
Length of output: 1031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' DotNut/NUT20/MintQuoteSigner.cs | sed -n '1,180p'
echo "== signature-related API docs/usages =="
rg -n "VerifySignature|FromHexString|TryCreate|SigVerifyBIP340|FormatException" DotNut -S
echo "== project sdk/package context =="
find . -maxdepth 3 \( -name '*.csproj' -o -name 'Directory.Build.props' -o -name 'Directory.Packages.props' \) -print | sort | while read -r f; do
echo "--- $f"
sed -n '1,160p' "$f"
done
echo "== deterministic probe if SDK present =="
cat > /tmp/FromHexStringProbe.cs <<'CS'
using System;
class P {
static void Main() {
foreach (var s in new string[] { "97c6b9...", "97c6b9", "zz", null }) {
try {
Convert.FromHexString(s);
Console.WriteLine(s ?? "null: ok");
}
catch (Exception ex) {
Console.WriteLine(s ?? "null: " + ex.GetType().Name + ": " + ex.Message);
}
}
}
}
CS
dotnet --version 2>/dev/null && mkdir -p /tmp/probe && cp /tmp/FromHexStringProbe.cs /tmp/probe/Probe.csproj <<'EOF'
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
</PropertyGroup>
</Project>
EOF
cd /tmp/probe && dotnet run --no-restore 2>&1 || trueRepository: Kukks/DotNut
Length of output: 16599
🌐 Web query:
.NET Convert.FromHexString throws FormatException odd length documentation
💡 Result:
In.NET, the Convert.FromHexString method requires the input string (or span) to have a length that is a multiple of two because each byte in a hexadecimal-encoded string is represented by exactly two characters. If you provide an input with an odd length, the method explicitly throws a System.FormatException [1][2][3]. This behavior is documented in the official.NET API reference for all relevant versions (including.NET 8.0, 9.0, and 10.0), which states that a FormatException is thrown if the length of the source input is not zero or a multiple of two [1][2][3]. If you need to process a hexadecimal string that might be of an odd length, you must validate or pad the input before passing it to Convert.FromHexString. A common approach is to check if the length is odd and, if so, prepend a leading '0' (or handle it according to your specific data requirements) before performing the conversion.
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.convert.fromhexstring?view=net-10.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.convert.fromhexstring?view=net-8.0
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.convert.fromhexstring?view=net-9.0
Return false for malformed quote.Signature hex strings.
Convert.FromHexString(quote.Signature) can throw FormatException for odd-length or non-hex input, so VerifySignature does not return false for malformed signature strings. Catch FormatException around the parse step before calling SecpSchnorrSignature.TryCreate.
🤖 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/NUT20/MintQuoteSigner.cs` around lines 113 - 136, Update
VerifySignature to catch FormatException from Convert.FromHexString when parsing
quote.Signature, returning false for malformed odd-length or non-hex input
before invoking SecpSchnorrSignature.TryCreate. Preserve the existing signature
verification and allowLegacy behavior for valid input.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aligns the mint quote signature with cashubtc/nuts#b969c4aa, which replaced the concatenated quote id and hex outputs with a domain-separated, length-prefixed commitment over raw bytes:
Amounts are canonical minimal big-endian (
0→ empty,256→0x0100) andB_is the decoded point, not its hex string. The old test vector was signed over the old message, so it is replaced with the one fromtests/20-test.md; the test assertsmsg_to_signand its hash directly, so a mistake in the length prefixes shows up on its own rather than as a failed signature.Legacy fallback
No released cdk-mintd understands the new message. The change is on
main(56fb4d7b) but in no tag up to v0.17.3, so signing only the new way makes NUT-20 minting fail against every mint deployed today — including the one CI starts.So the handlers sign the current message first and, when the mint answers error
20008, sign the same outputs the old way and retry once. Nothing is issued on a rejected signature, so the retry cannot double-spend. Verification accepts either, withallowLegacy: falseto refuse.This mirrors what the reference implementations did: cdk added
sign_legacyfor wallets plus a legacy branch inverify_signature, and nutshell has verified both since 0.20.2. Removable once mints without the current format are gone.Verified against live mints:
latest-amd64)NUT-29
NUT-29 defines no message of its own —
signatures[i]is a NUT-20 signature overquotes[i]and the batch's wholeoutputsarray, since the outputs are one consolidated set rather than partitioned per quote. The models were already there but nothing filledSignatures, so this addsPostBatchedMintRequest.Sign(keysByQuoteId): one entry per quote,nullfor unlocked ones, field omitted when nothing is locked. Covered by the vector fromtests/29-tests.md.Also
Error codes moved off
MintQuoteSignerintoCashuErrorCodes, carrying the full table fromerror_codes.md— including12003and NUT-29's11016/11017, which upstream added recently.Unit tests: 126 passing.