feat: batch minting models and api#31
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds NUT‑29 batch mint support: new batch mint/check API surface and DTOs, consolidates several API model types into MintInfo, exposes a MeltHandlerBolt11 accessor, and applies formatting/whitespace changes across tests and a Nostr handler. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Client/Service
participant ClientLib as CashuHttpClient
participant Server as Cashu API Server
participant MintSvc as Mint Service
Caller->>ClientLib: BatchCheckMintQuoteState(method, request)
ClientLib->>Server: POST /v1/mint/quote/{method}/check (request JSON)
Server->>MintSvc: validate quotes / quote states
MintSvc-->>Server: quote states response
Server-->>ClientLib: 200 + JSON
ClientLib-->>Caller: deserialized response
Caller->>ClientLib: BatchMint(method, request)
ClientLib->>Server: POST /v1/mint/{method}/batch (request JSON)
Server->>MintSvc: perform batch mint (outputs, signatures)
MintSvc-->>Server: blind signatures
Server-->>ClientLib: 200 + signatures JSON
ClientLib-->>Caller: PostBatchedMintResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
cleanup MintInfo
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.cs (1)
21-23: Extract the NIP-17 transport predicate into a shared helper.The same filter is duplicated in
CanHandleandSendPayment; centralizing it reduces drift risk when transport-tag rules change.♻️ Proposed refactor
public class NostrNip17PaymentRequestInterfaceHandler : PaymentRequestInterfaceHandler { + private static bool IsNip17NostrTransport(PaymentRequestTransport transport) + { + return transport.Type == "nostr" + && transport.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true; + } + public static void Register() { PaymentRequestTransportInitiator.Handlers.Add( new NostrNip17PaymentRequestInterfaceHandler() ); @@ public bool CanHandle(PaymentRequest request) { - return request.Transports.Any(t => - t.Type == "nostr" - && t.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true - ); + return request.Transports.Any(IsNip17NostrTransport); } @@ - var nostrTransport = request.Transports.FirstOrDefault(t => - t.Type == "nostr" - && t.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true - ); + var nostrTransport = request.Transports.FirstOrDefault(IsNip17NostrTransport);Also applies to: 33-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.cs` around lines 21 - 23, Extract the duplicated NIP-17 transport predicate into a single shared helper to avoid drift: create a private static method (e.g., IsNip17Transport or MatchesNip17Transport) on the NostrNip17PaymentRequestInterfaceHandler class and replace the inline predicate in both CanHandle and SendPayment with calls to that helper; the helper should encapsulate the check t.Type == "nostr" && t.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true so both methods use the exact same logic.DotNut.sln.DotSettings.user (1)
18-23: Keep Rider user-session state out of version control.These entries are personal IDE state (
IsActive, session ancestry, generated session IDs). Committing them just adds machine-specific churn and merge noise without affecting the product. Please drop this file from the PR, or ignore*.DotSettings.user.Also applies to: 41-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNut.sln.DotSettings.user` around lines 18 - 23, The committed DotNut.sln.DotSettings.user contains personal Rider session state (entries under UnitTestSessionStore/Sessions like the SessionState/ TestAncestor blocks) and should be removed from the PR: remove the file from the commit (git rm --cached DotNut.sln.DotSettings.user or simply drop it from the change), add a rule to ignore these IDE user files by adding "*.DotSettings.user" to .gitignore, and commit the .gitignore update so future personal Rider settings (including UnitTestSessionStore entries) are not tracked.DotNut/Api/ICashuApi.cs (1)
61-65: Make the batch quote-check response shape explicit.This endpoint returns a JSON array of quote objects in request order, so
Task<TResponse>leaves a fixed part of the contract implicit.Task<TQuoteResponse[]>orIReadOnlyList<TQuoteResponse>would make the public API harder to misuse. (github.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNut/Api/ICashuApi.cs` around lines 61 - 65, The BatchCheckMintQuoteState method in ICashuApi currently returns a generic Task<TResponse> which hides the fact the endpoint returns an ordered JSON array of quote objects; change the signature of BatchCheckMintQuoteState to return a concrete collection type such as Task<TQuoteResponse[]> or Task<IReadOnlyList<TQuoteResponse>> (replace the generic TResponse), update the method declaration in the ICashuApi interface, and then update any callers or implementations to deserialize and handle an array/list of TQuoteResponse in request order to ensure the API contract is explicit and harder to misuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DotNut/Abstractions/Handlers/MeltHandlerBolt11.cs`:
- Line 15: GetBlankOutputs currently returns the live blankOutputs list which
allows callers to mutate the internal collection and corrupt downstream Melt()
processing; change GetBlankOutputs to return a defensive copy or an
immutable/read-only view (e.g., return new List<OutputData>(blankOutputs) or
IReadOnlyList<OutputData>) so callers cannot add/remove/reorder the internal
list, and update any call sites to accept the new return type if you switch to
IReadOnlyList; keep the internal blankOutputs field private and only mutate it
inside Melt() and other internal methods.
In `@DotNut/Abstractions/MintInfo.cs`:
- Around line 223-247: The CheckNut29 logic incorrectly treats omitted NUT-29
fields as "unsupported"; update CheckNut29 (and the analogous function around
lines 382-390) to: mark Supported = true whenever a valid nut29 payload
deserializes (regardless of Methods being null/empty), preserve the semantics
that a null Methods means "all NUT-04 methods" (i.e., keep Methods null rather
than forcing Supported = false), and make MaxBatchSize optional (change its type
to int? or otherwise detect presence) so you only set MaxBatchSize when nut29
provides it; copy nut29.Methods and nut29.MaxBatchSize when present but do not
treat missing fields as disabling batch support. Ensure JsonException handling
remains.
In `@DotNut/ApiModels/BatchMint/PostBatchedMintRequest.cs`:
- Around line 10-12: The Amounts property currently allows nullable array
entries (ulong?[]?) which permits invalid payloads like [100, null]; change the
property type to an optional array of non-null ulongs (ulong[]?) so
quote_amounts remains optional but, when present, contains only non-null values;
update the Amounts declaration in PostBatchedMintRequest (keeping the existing
attributes JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault) and
JsonPropertyName("quote_amounts")) to use ulong[]? instead of ulong?[]?.
---
Nitpick comments:
In `@DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.cs`:
- Around line 21-23: Extract the duplicated NIP-17 transport predicate into a
single shared helper to avoid drift: create a private static method (e.g.,
IsNip17Transport or MatchesNip17Transport) on the
NostrNip17PaymentRequestInterfaceHandler class and replace the inline predicate
in both CanHandle and SendPayment with calls to that helper; the helper should
encapsulate the check t.Type == "nostr" && t.Tags?.Any(tag => tag.Key == "n" &&
tag.Value.Any(v => v == "17")) == true so both methods use the exact same logic.
In `@DotNut.sln.DotSettings.user`:
- Around line 18-23: The committed DotNut.sln.DotSettings.user contains personal
Rider session state (entries under UnitTestSessionStore/Sessions like the
SessionState/ TestAncestor blocks) and should be removed from the PR: remove the
file from the commit (git rm --cached DotNut.sln.DotSettings.user or simply drop
it from the change), add a rule to ignore these IDE user files by adding
"*.DotSettings.user" to .gitignore, and commit the .gitignore update so future
personal Rider settings (including UnitTestSessionStore entries) are not
tracked.
In `@DotNut/Api/ICashuApi.cs`:
- Around line 61-65: The BatchCheckMintQuoteState method in ICashuApi currently
returns a generic Task<TResponse> which hides the fact the endpoint returns an
ordered JSON array of quote objects; change the signature of
BatchCheckMintQuoteState to return a concrete collection type such as
Task<TQuoteResponse[]> or Task<IReadOnlyList<TQuoteResponse>> (replace the
generic TResponse), update the method declaration in the ICashuApi interface,
and then update any callers or implementations to deserialize and handle an
array/list of TQuoteResponse in request order to ensure the API contract is
explicit and harder to misuse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4a3df9d-708c-44ef-a089-9f7cfc749ddf
📒 Files selected for processing (15)
DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.csDotNut.Tests/Integration.csDotNut.Tests/UnitTest1.csDotNut.sln.DotSettings.userDotNut/Abstractions/Handlers/MeltHandlerBolt11.csDotNut/Abstractions/MintInfo.csDotNut/Api/CashuHttpClient.csDotNut/Api/ICashuApi.csDotNut/ApiModels/BatchMint/PostBatchedMintQuoteStateRequest.csDotNut/ApiModels/BatchMint/PostBatchedMintRequest.csDotNut/ApiModels/BatchMint/PostBatchedMintResponse.csDotNut/ApiModels/Info/MPPInfo.csDotNut/ApiModels/Info/SwapInfo.csDotNut/ApiModels/Info/WebSocketSupport.csDotNut/NUT14/HTLCProofSecret.cs
💤 Files with no reviewable changes (3)
- DotNut/ApiModels/Info/WebSocketSupport.cs
- DotNut/ApiModels/Info/MPPInfo.cs
- DotNut/ApiModels/Info/SwapInfo.cs
| { | ||
| public PostMeltQuoteBolt11Response GetQuote() => quote; | ||
|
|
||
| public List<OutputData> GetBlankOutputs() => blankOutputs; |
There was a problem hiding this comment.
Don't expose the live blankOutputs list.
This hands callers the same collection that Melt() later reuses for NUT-10 processing, request construction, and change reconstruction. A caller can add, remove, or reorder outputs between those steps and corrupt the melt flow.
Possible fix
- public List<OutputData> GetBlankOutputs() => blankOutputs;
+ public IReadOnlyList<OutputData> GetBlankOutputs() => blankOutputs.AsReadOnly();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@DotNut/Abstractions/Handlers/MeltHandlerBolt11.cs` at line 15,
GetBlankOutputs currently returns the live blankOutputs list which allows
callers to mutate the internal collection and corrupt downstream Melt()
processing; change GetBlankOutputs to return a defensive copy or an
immutable/read-only view (e.g., return new List<OutputData>(blankOutputs) or
IReadOnlyList<OutputData>) so callers cannot add/remove/reorder the internal
list, and update any call sites to accept the new return type if you switch to
IReadOnlyList; keep the internal blankOutputs field private and only mutate it
inside Melt() and other internal methods.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
DotNut/Abstractions/MintInfo.cs (1)
223-247:⚠️ Potential issue | 🟠 MajorDon't collapse the NUT-29 "all methods" case into an empty array.
Support should be based on successfully deserializing the NUT-29 payload. Line 232 still gates it on
Methods, while Lines 391-393 initializeMethodsto[]. Because Line 385 definesAllSupportedasMethods is null, valid payloads like{}or{"max_batch_size": 64}lose the "all NUT-04 methods" signal and surface asSupported = true, AllSupported = false.In System.Text.Json, if a property is declared as `public string[]? Methods { get; set; } = [];`, does omitting `"methods"` during deserialization keep `Methods` as `[]` instead of `null`?💡 Proposed fix
- if (nut29?.Methods is not null) + if (nut29 is not null) { return new BatchMintInfo { Supported = true, MaxBatchSize = nut29.MaxBatchSize, Methods = nut29.Methods, }; } @@ - public string[]? Methods { get; set; } = []; + public string[]? Methods { get; set; }Also applies to: 382-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DotNut/Abstractions/MintInfo.cs` around lines 223 - 247, The CheckNut29 method incorrectly treats a successfully deserialized NUT-29 payload with missing "methods" as unsupported because it checks nut29?.Methods is not null; change the logic to consider the payload supported when JsonSerializer.Deserialize<BatchMintInfo> returns a non-null nut29 (i.e., test nut29 != null), and assign Methods = nut29.Methods (preserving a null value if the JSON omitted the property) so that AllSupported (which depends on Methods being null) remains correct; also remove any default initializer that sets Methods = [] in the BatchMintInfo type (and fix the same pattern where BatchMintInfo is constructed/initialized elsewhere) so deserialization can produce null for omitted "methods".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@DotNut/Abstractions/MintInfo.cs`:
- Around line 223-247: The CheckNut29 method incorrectly treats a successfully
deserialized NUT-29 payload with missing "methods" as unsupported because it
checks nut29?.Methods is not null; change the logic to consider the payload
supported when JsonSerializer.Deserialize<BatchMintInfo> returns a non-null
nut29 (i.e., test nut29 != null), and assign Methods = nut29.Methods (preserving
a null value if the JSON omitted the property) so that AllSupported (which
depends on Methods being null) remains correct; also remove any default
initializer that sets Methods = [] in the BatchMintInfo type (and fix the same
pattern where BatchMintInfo is constructed/initialized elsewhere) so
deserialization can produce null for omitted "methods".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae3c11e9-4dd5-4bd3-8f3e-8af2abe3f3a2
📒 Files selected for processing (2)
DotNut/Abstractions/MintInfo.csDotNut/ApiModels/BatchMint/PostBatchedMintRequest.cs
cashubtc/nuts#333
Summary by CodeRabbit
New Features
Improvements
Tests
Chores