Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds Bech32-based payment request encoding/decoding, Nut10 locking-condition support, a unified Tag type replacing transport tag POCOs, updates Nostr NIP-17 detection, and expands unit tests for many payment-request scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PR as PaymentRequestBech32Encoder
participant B32 as Bech32Encoder
participant PRModel as PaymentRequest
Client->>PR: Encode(paymentRequest)
PR->>PR: Serialize TLV fields (id, amount, unit, transports, nut10, tags)
PR->>PR: ConvertBits(8→5)
PR->>B32: EncodeRaw(data, encodingType)
B32->>B32: CreateChecksum / Polymod(HRP + data)
B32-->>PR: bech32 string ("creqb1...")
PR-->>Client: bech32 string
sequenceDiagram
participant Client
participant B32 as Bech32Encoder
participant PR as PaymentRequestBech32Encoder
participant PRModel as PaymentRequest
Client->>PR: Decode("creqb...")
PR->>B32: DecodeDataRaw(string)
B32->>B32: VerifyChecksum / Polymod validate
B32-->>PR: decoded bytes (5-bit words)
PR->>PR: ConvertBits(5→8)
PR->>PR: Parse TLV → fields (transports, nut10, tags)
PR->>PRModel: populate PaymentRequest
PR-->>Client: PaymentRequest object
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 5
🤖 Fix all issues with AI agents
In `@DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.cs`:
- Around line 16-24: CanHandle and SendPayment currently assume
PaymentRequestTransport.Tags is non-null which can throw NREs; update the
predicates to be null-safe (e.g., check t.Tags != null or use null-conditional
checks when calling Any) when scanning PaymentRequest.Transports, and in
SendPayment replace First(...) with a safe lookup (FirstOrDefault or TryFind)
for the nostr n=17 transport, check the result for null before accessing Target
or Tags, and return/fail gracefully if no matching transport is found; reference
PaymentRequest.Transports, PaymentRequestTransport.Tags, CanHandle, SendPayment,
and the nprofileStr assignment when making these changes.
In `@DotNut/NBitcoin/Bech32/Bech32Encoder.cs`:
- Around line 1-4: The file header in Bech32Encoder.cs uses the unprofessional
term "stolen" — replace that wording with a neutral attribution such as "copied
from" or "adapted from" and mention the original source URL
(https://github.com/MetacoSA/NBitcoin/blob/master/NBitcoin/DataEncoders/Bech32Encoder.cs);
keep the note about removing older dotnet code and leave license attribution
unchanged (LICENSE remains the source of legal attribution).
In `@DotNut/PaymentRequestBech32Encoder.cs`:
- Around line 274-280: DecodeTLV leaves PaymentRequest.Transports null when no
transports are found, but the property is declared non-nullable; ensure
Transports is always non-null by either initializing the property on the
PaymentRequest class (e.g., set Transports =
Array.Empty<PaymentRequestTransport>() in the PaymentRequest declaration) or by
assigning pr.Transports = Array.Empty<PaymentRequestTransport>() in the
DecodeTLV method before returning (similar to how mints/mints array are
handled), so callers never encounter a null reference.
- Around line 83-89: The code currently lowercases entire mint URLs in
PaymentRequestBech32Encoder.cs when writing TLV Mint entries; instead, parse
each mint string into a URI (the mint variable), lowercase only the scheme and
host, and recompose the URL preserving original path, query and fragment case
before passing it to WriteTlvUtf8(writer, TlvTag.Mint, ...). Use Uri/UriBuilder
(or equivalent) to set Scheme and Host to ToLowerInvariant() while leaving
PathAndQuery intact, and handle invalid URIs by falling back to the original
mint string or validating earlier.
- Around line 405-411: The prefix check in DecodeNostr uses a case-sensitive
StartsWith("nprofile", StringComparison.Ordinal) which will reject valid Bech32
variants; change the comparison to case-insensitive (use
StringComparison.OrdinalIgnoreCase) so nprofile (and other valid-cased Bech32
inputs) are accepted, and ensure DecodeNprofile/DecodeNpub are invoked with the
original string (no accidental lowercasing) to preserve downstream decoding.
🧹 Nitpick comments (4)
DotNut.Tests/UnitTest1.cs (1)
975-1004: Clarify the non‑canonical Bech32 roundtrip in the test.The test currently skips asserting the original encoded string with an “idk why” comment. If the encoder is expected to be canonical, consider fixing ordering/normalization so the equality check can be restored, or update the expected value to the canonical output.
DotNut/PaymentRequestEncoder.cs (2)
37-41: Consider extracting duplicated tag serialization logic.The same tag serialization pattern (iterating over
tag.Keyandtag.Value) is duplicated for transport tags (lines 35-41) and Nut10 tags (lines 59-65). Consider extracting a helper method.♻️ Suggested helper method
+ private static CBORObject TagToCBOR(Tag tag) + { + var tagItem = CBORObject.NewArray(); + tagItem.Add(tag.Key); + foreach (var tagValue in tag.Value) + { + tagItem.Add(tagValue); + } + return tagItem; + }Then use it in both places:
tags.Add(TagToCBOR(tag));Also applies to: 57-66
118-123: Consider filtering empty arrays to avoid cryptic exceptions.The
Tagconstructor throwsArgumentExceptionfor empty arrays. If malformed CBOR contains empty arrays, the error message may be unclear. Consider adding.Where(tag => tag.Count > 0)for more graceful handling.♻️ Suggested improvement
transport.Tags = transportValue.Values - .Where(tag => tag.Type == CBORType.Array) + .Where(tag => tag.Type == CBORType.Array && tag.Count > 0) .Select(tag => new Tag(tag.Values.Select(cborObject => cborObject.AsString()).ToArray()) ) .ToArray();Also applies to: 145-150
DotNut/PaymentRequestBech32Encoder.cs (1)
237-243: Add explicit bounds checking for malformed TLV data.The TLV parsing reads length and slices data without verifying sufficient bytes remain. Malformed input could cause cryptic exceptions. Consider adding bounds validation.
♻️ Suggested improvement
while (offset < data.Length) { + if (offset + 3 > data.Length) + throw new FormatException("TLV data truncated: missing tag or length"); + var tag = data[offset]; var length = BinaryPrimitives.ReadUInt16BigEndian(data.Slice(offset + 1, 2)); offset += 3; + + if (offset + length > data.Length) + throw new FormatException($"TLV data truncated: expected {length} bytes for tag 0x{tag:X2}"); + var value = data.Slice(offset, length); offset += length;Apply similar changes to
DecodeTransportandDecodeNut10.
Comment on lines
+1
to
+4
| // This file was stolen from NBitcoin and bundled into a single file for Bech32 encoding. | ||
| // Also code for older dotnet versions (without spans) was removed. | ||
| // Original source: https://github.com/MetacoSA/NBitcoin/blob/master/NBitcoin/DataEncoders/Bech32Encoder.cs | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Explore the repository structure to understand license/attribution handling
find . -type f -name "LICENSE*" -o -name "NOTICE*" -o -name "COPYRIGHT*" 2>/dev/null | head -20Repository: Kukks/DotNut
Length of output: 91
🏁 Script executed:
# Read the actual file in question
cat -n "DotNut/NBitcoin/Bech32/Bech32Encoder.cs" | head -20Repository: Kukks/DotNut
Length of output: 807
🏁 Script executed:
# Check if there are other ported files with similar headers
find . -type f -name "*.cs" | xargs grep -l "stolen\|ported from" 2>/dev/null | head -10Repository: Kukks/DotNut
Length of output: 97
🏁 Script executed:
# Read the main LICENSE file
cat "./LICENSE"Repository: Kukks/DotNut
Length of output: 1129
🏁 Script executed:
# Read the NBitcoin subdirectory LICENSE
cat "./DotNut/NBitcoin/LICENSE"Repository: Kukks/DotNut
Length of output: 1131
🏁 Script executed:
# Check if there's a NOTICE file
find . -name "NOTICE*" -o -name "ATTRIBUTION*" 2>/dev/nullRepository: Kukks/DotNut
Length of output: 38
🏁 Script executed:
# Check if other ported files in the NBitcoin subdirectory have proper header comments
find ./DotNut/NBitcoin -type f -name "*.cs" | head -10 | while read f; do echo "=== $f ==="; head -5 "$f"; doneRepository: Kukks/DotNut
Length of output: 1595
Replace unprofessional "stolen" wording in the file header.
The file header uses "stolen" which is unprofessional wording. Update it to a neutral attribution. Note: License attribution is already properly handled via ./DotNut/NBitcoin/LICENSE.
Suggested header fix
-// This file was stolen from NBitcoin and bundled into a single file for Bech32 encoding.
+// Ported from NBitcoin; code for older dotnet versions (without spans) was removed.📝 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
| // This file was stolen from NBitcoin and bundled into a single file for Bech32 encoding. | |
| // Also code for older dotnet versions (without spans) was removed. | |
| // Original source: https://github.com/MetacoSA/NBitcoin/blob/master/NBitcoin/DataEncoders/Bech32Encoder.cs | |
| // Ported from NBitcoin; code for older dotnet versions (without spans) was removed. | |
| // Also code for older dotnet versions (without spans) was removed. | |
| // Original source: https://github.com/MetacoSA/NBitcoin/blob/master/NBitcoin/DataEncoders/Bech32Encoder.cs | |
🤖 Prompt for AI Agents
In `@DotNut/NBitcoin/Bech32/Bech32Encoder.cs` around lines 1 - 4, The file header
in Bech32Encoder.cs uses the unprofessional term "stolen" — replace that wording
with a neutral attribution such as "copied from" or "adapted from" and mention
the original source URL
(https://github.com/MetacoSA/NBitcoin/blob/master/NBitcoin/DataEncoders/Bech32Encoder.cs);
keep the note about removing older dotnet code and leave license attribution
unchanged (LICENSE remains the source of legal attribution).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.cs`:
- Around line 27-34: The lookup for the NIP17 transport can NRE when
request.Transports is null; update the null-safety so the search uses a
null-tolerant call (e.g., use request.Transports? or a null-coalescing empty
sequence) when calling FirstOrDefault to avoid dereferencing a null collection,
then keep the existing nostrTransport null check and throw
InvalidOperationException("No NIP17 nostr transport found.") if no match;
reference the variables request.Transports and nostrTransport to locate and fix
the code in NostrNip17PaymentRequestInterfaceHandler.
In `@DotNut/PaymentRequestBech32Encoder.cs`:
- Around line 230-280: The DecodeTLV method reads tag/length/value without
bounds checks and can throw ArgumentOutOfRangeException on malformed input; fix
DecodeTLV by validating there are at least 3 bytes remaining before reading tag
and 2-byte length, ensure the declared length does not exceed remaining data
before slicing value, and specifically validate fixed-size fields (e.g., for tag
0x02 require value.Length == 8 before calling
BinaryPrimitives.ReadUInt64BigEndian) — on invalid or truncated input either
throw a clear FormatException or return a graceful error, and update references
in DecodeTLV (mints, transports, DecodeTransport, DecodeNut10 handling) to rely
on these checks.
- Around line 282-343: DecodeTransport currently can return a
PaymentRequestTransport with missing Type or Target when TLV data lacks those
fields; update DecodeTransport to validate after parsing that transport.Type and
targetBytes (or transport.Target) are present for the declared transport kind
and throw a FormatException when required fields are missing. Specifically,
after the parsing loop in DecodeTransport, check if transport.Type is null/empty
and throw new FormatException("Missing transport type"), and for each branch
require targetBytes (or transport.Target) be present—throw new
FormatException("Missing transport target for nostr/post") before proceeding to
build relays/tags or call EncodeNprofile, ensuring no null Type/Target can be
returned.
- Around line 26-49: The Decode method currently calls
Encoder.DecodeDataRaw(creqb, out _) and ignores the returned encoding type,
allowing non-BECH32M inputs; update Decode in PaymentRequestBech32Encoder to
capture the encoding type from Encoder.DecodeDataRaw (e.g., var words =
Encoder.DecodeDataRaw(creqb, out var encoding)) and validate that encoding ==
Bech32EncodingType.BECH32M, throwing an ArgumentException if not, mirroring the
checks used in DecodeNpub/DecodeNprofile; keep the rest of the flow (ConvertBits
and DecodeTLV) unchanged.
- Around line 83-89: The mint URLs are written verbatim in
PaymentRequestBech32Encoder (when iterating paymentRequest.Mints), but NUT-00
requires stripping trailing slashes; before calling WriteTlvUtf8(writer,
TlvTag.Mint, mint) normalize each mint with mint.TrimEnd('/') (and optionally
preserve null/empty checks) so the value written is the trimmed URL; update the
loop where paymentRequest.Mints is enumerated to use the trimmed mint string.
Comment on lines
+27
to
+34
| var nostrTransport = request.Transports.FirstOrDefault(t => | ||
| t.Type == "nostr" && | ||
| t.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true); | ||
| if (nostrTransport is null) | ||
| { | ||
| throw new InvalidOperationException("No NIP17 nostr transport found."); | ||
| } | ||
| var nprofileStr = nostrTransport.Target; |
Contributor
There was a problem hiding this comment.
Avoid NRE if Transports is null.
Line 27 will throw before the explicit error if request.Transports is null (e.g., CBOR payloads without transports). Consider a null-safe lookup to preserve the intended error path.
🛠️ Proposed fix
- var nostrTransport = request.Transports.FirstOrDefault(t =>
- t.Type == "nostr" &&
- t.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true);
- if (nostrTransport is null)
- {
- throw new InvalidOperationException("No NIP17 nostr transport found.");
- }
+ var nostrTransport = request.Transports?.FirstOrDefault(t =>
+ t.Type == "nostr" &&
+ t.Tags?.Any(tag => tag.Key == "n" && tag.Value.Any(v => v == "17")) == true)
+ ?? throw new InvalidOperationException("No NIP17 nostr transport found.");🤖 Prompt for AI Agents
In `@DotNut.Nostr/NostrNip17PaymentRequestInterfaceHandler.cs` around lines 27 -
34, The lookup for the NIP17 transport can NRE when request.Transports is null;
update the null-safety so the search uses a null-tolerant call (e.g., use
request.Transports? or a null-coalescing empty sequence) when calling
FirstOrDefault to avoid dereferencing a null collection, then keep the existing
nostrTransport null check and throw InvalidOperationException("No NIP17 nostr
transport found.") if no match; reference the variables request.Transports and
nostrTransport to locate and fix the code in
NostrNip17PaymentRequestInterfaceHandler.
Comment on lines
+26
to
+49
| public static string Encode(PaymentRequest paymentRequest) | ||
| { | ||
| var writer = new ArrayBufferWriter<byte>(256); | ||
| EncodeTLV(writer, paymentRequest); | ||
|
|
||
| var tlvBytes = writer.WrittenSpan; | ||
| Span<byte> words = tlvBytes.Length * 2 > 1024 | ||
| ? new byte[tlvBytes.Length * 2] | ||
| : stackalloc byte[tlvBytes.Length * 2]; | ||
|
|
||
| var wordsLen = ConvertBits(tlvBytes, words, 8, 5, true); | ||
| return Encoder.EncodeRaw(words[..wordsLen].ToArray(), Bech32EncodingType.BECH32M).ToUpperInvariant(); | ||
| } | ||
|
|
||
| public static PaymentRequest Decode(string creqb) | ||
| { | ||
| if (!creqb.StartsWith("creqb1", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| throw new ArgumentException("Invalid payment request type!"); | ||
| } | ||
|
|
||
| var words = Encoder.DecodeDataRaw(creqb, out _); | ||
| var tlv = ConvertBits(words, 5, 8, false); | ||
| return DecodeTLV(tlv); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cashu NUT-26 payment request creqb BECH32M encoding specification
💡 Result:
Summary — Cashu NUT-26 (Payment Request Bech32m Encoding):
- Overall format: "creqb" + "1" + bech32m(TLV(PaymentRequest)). HRP = "creqb" (use Bech32m). Implementations SHOULD output uppercase; decoders MUST accept both cases. [1]
- When parsing creq parameters: support legacy NUT-18 CBOR+base64 (prefix creqA), else if valid Bech32m with HRP creqb parse as NUT-26; otherwise error. [1]
- TLV wire format: repeated TLV entries where each entry = Type (1 byte) | Length (2 bytes, big-endian) | Value (length bytes). [1]
- Top-level tags (all optional; unknown tags MUST be ignored):
- 0x01 id — string (i)
- 0x02 amount — u64 (a)
- 0x03 unit — u8 or UTF‑8 string (0x00 = "sat") (u)
- 0x04 single_use — u8 (0=false,1=true) (s)
- 0x05 mint — string (repeatable) (m)
- 0x06 description — string (d)
- 0x07 transport — sub‑TLV (repeatable; maps to t)
- 0x08 nut10 — sub‑TLV for NUT‑10 spending conditions (nut10)
Transport sub‑TLV defines sub‑tags including: kind (0=nostr,1=http_post), target (raw 32‑byte X‑only pubkey for nostr or UTF‑8 URL for http_post), and tag_tuple entries for extras. Nostr targets are stored as raw 32‑byte X‑only keys; NIP‑19 nprofile/npub handling is specified for JSON↔TLV conversion. [1]
Reference: NUT-26: Payment Request Bech32m Encoding (Cashu NUTs). [1]
🏁 Script executed:
# Search for the Encoder class and its DecodeDataRaw method signature
rg "class Encoder|DecodeDataRaw" --type cs -A 2
# Also check for any tests related to PaymentRequestBech32Encoder
fd ".*test.*" -e cs | head -20Repository: Kukks/DotNut
Length of output: 1303
🏁 Script executed:
# Look for Bech32EncodingType enum to understand available types
rg "enum Bech32EncodingType|BECH32M|BECH32" --type cs | head -30Repository: Kukks/DotNut
Length of output: 2182
Validate BECH32M on decode.
The Cashu NUT-26 specification requires "creqb" to use Bech32m (BECH32M) encoding. Line 47 discards the encoding type returned by DecodeDataRaw, allowing BECH32 inputs to be incorrectly accepted. The Encode method correctly enforces BECH32M (line 36), but Decode must validate it too. Similar methods in this file (DecodeNpub, DecodeNprofile) already enforce their respective encoding types—apply the same validation here.
🛠️ Proposed fix
- var words = Encoder.DecodeDataRaw(creqb, out _);
+ var words = Encoder.DecodeDataRaw(creqb, out var encodingType);
+ if (encodingType != Bech32EncodingType.BECH32M)
+ throw new FormatException("Invalid payment request: expected BECH32M encoding");📝 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
| public static string Encode(PaymentRequest paymentRequest) | |
| { | |
| var writer = new ArrayBufferWriter<byte>(256); | |
| EncodeTLV(writer, paymentRequest); | |
| var tlvBytes = writer.WrittenSpan; | |
| Span<byte> words = tlvBytes.Length * 2 > 1024 | |
| ? new byte[tlvBytes.Length * 2] | |
| : stackalloc byte[tlvBytes.Length * 2]; | |
| var wordsLen = ConvertBits(tlvBytes, words, 8, 5, true); | |
| return Encoder.EncodeRaw(words[..wordsLen].ToArray(), Bech32EncodingType.BECH32M).ToUpperInvariant(); | |
| } | |
| public static PaymentRequest Decode(string creqb) | |
| { | |
| if (!creqb.StartsWith("creqb1", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| throw new ArgumentException("Invalid payment request type!"); | |
| } | |
| var words = Encoder.DecodeDataRaw(creqb, out _); | |
| var tlv = ConvertBits(words, 5, 8, false); | |
| return DecodeTLV(tlv); | |
| public static string Encode(PaymentRequest paymentRequest) | |
| { | |
| var writer = new ArrayBufferWriter<byte>(256); | |
| EncodeTLV(writer, paymentRequest); | |
| var tlvBytes = writer.WrittenSpan; | |
| Span<byte> words = tlvBytes.Length * 2 > 1024 | |
| ? new byte[tlvBytes.Length * 2] | |
| : stackalloc byte[tlvBytes.Length * 2]; | |
| var wordsLen = ConvertBits(tlvBytes, words, 8, 5, true); | |
| return Encoder.EncodeRaw(words[..wordsLen].ToArray(), Bech32EncodingType.BECH32M).ToUpperInvariant(); | |
| } | |
| public static PaymentRequest Decode(string creqb) | |
| { | |
| if (!creqb.StartsWith("creqb1", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| throw new ArgumentException("Invalid payment request type!"); | |
| } | |
| var words = Encoder.DecodeDataRaw(creqb, out var encodingType); | |
| if (encodingType != Bech32EncodingType.BECH32M) | |
| throw new FormatException("Invalid payment request: expected BECH32M encoding"); | |
| var tlv = ConvertBits(words, 5, 8, false); | |
| return DecodeTLV(tlv); | |
| } |
🤖 Prompt for AI Agents
In `@DotNut/PaymentRequestBech32Encoder.cs` around lines 26 - 49, The Decode
method currently calls Encoder.DecodeDataRaw(creqb, out _) and ignores the
returned encoding type, allowing non-BECH32M inputs; update Decode in
PaymentRequestBech32Encoder to capture the encoding type from
Encoder.DecodeDataRaw (e.g., var words = Encoder.DecodeDataRaw(creqb, out var
encoding)) and validate that encoding == Bech32EncodingType.BECH32M, throwing an
ArgumentException if not, mirroring the checks used in
DecodeNpub/DecodeNprofile; keep the rest of the flow (ConvertBits and DecodeTLV)
unchanged.
Comment on lines
+83
to
+89
| if (paymentRequest.Mints is { } mints) | ||
| { | ||
| foreach (var mint in mints) | ||
| { | ||
| WriteTlvUtf8(writer, TlvTag.Mint, mint); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cashu NUT-00 NUT-26 specification mint URL normalization trailing slash lowercase scheme host
💡 Result:
Short answer:
- NUT-00 mandates normalizing mint URLs by stripping any trailing slash. [1]
- NUT-00 does not require lowercasing the scheme/host. There is no NUT-26 in the official NUT list (no spec found). For general URI rules (http/https), scheme and host are case‑insensitive and normally normalized to lowercase per RFC/HTTP guidance (RFC3986 / HTTP Semantics). [2][3]
Sources:
- NUT-00 (Cashu): token/mint URL normalization — strip trailing slashes. [https://cashubtc.github.io/nuts/00/]. [1]
- HTTP/URI normalization guidance: scheme and host are case‑insensitive / normally provided in lowercase (RFC3986 / HTTP Semantics). [2][3]
🏁 Script executed:
# First, let me locate and examine the PaymentRequestBech32Encoder.cs file
find . -name "PaymentRequestBech32Encoder.cs" -type fRepository: Kukks/DotNut
Length of output: 95
🏁 Script executed:
# Read the PaymentRequestBech32Encoder.cs file to see the implementation
cat -n ./DotNut/PaymentRequestBech32Encoder.cs | head -120Repository: Kukks/DotNut
Length of output: 4649
🏁 Script executed:
# Search for any mint normalization logic in the codebase
rg -i "mint.*normaliz|normaliz.*mint|trailing.*slash|striptrailing" --type csRepository: Kukks/DotNut
Length of output: 135
🏁 Script executed:
# Also search for WriteTlvUtf8 to understand what it does
rg -A 5 -B 5 "WriteTlvUtf8" ./DotNut/PaymentRequestBech32Encoder.csRepository: Kukks/DotNut
Length of output: 2444
Apply NUT-00-compliant mint URL normalization.
Line 87 writes mint URLs verbatim. NUT-00 specification mandates stripping trailing slashes from mint URLs to prevent mismatches. The Unit field (line 74) is normalized with ToLowerInvariant(), but mint URLs receive no normalization. Add trailing slash stripping (e.g., mint.TrimEnd('/') ) before writing.
🤖 Prompt for AI Agents
In `@DotNut/PaymentRequestBech32Encoder.cs` around lines 83 - 89, The mint URLs
are written verbatim in PaymentRequestBech32Encoder (when iterating
paymentRequest.Mints), but NUT-00 requires stripping trailing slashes; before
calling WriteTlvUtf8(writer, TlvTag.Mint, mint) normalize each mint with
mint.TrimEnd('/') (and optionally preserve null/empty checks) so the value
written is the trimmed URL; update the loop where paymentRequest.Mints is
enumerated to use the trimmed mint string.
Comment on lines
+230
to
+280
| private static PaymentRequest DecodeTLV(ReadOnlySpan<byte> data) | ||
| { | ||
| var pr = new PaymentRequest(); | ||
| var offset = 0; | ||
| var mints = new List<string>(); | ||
| var transports = new List<PaymentRequestTransport>(); | ||
|
|
||
| while (offset < data.Length) | ||
| { | ||
| var tag = data[offset]; | ||
| var length = BinaryPrimitives.ReadUInt16BigEndian(data.Slice(offset + 1, 2)); | ||
| offset += 3; | ||
| var value = data.Slice(offset, length); | ||
| offset += length; | ||
|
|
||
| switch (tag) | ||
| { | ||
| case 0x01: | ||
| pr.PaymentId = Encoding.UTF8.GetString(value); | ||
| break; | ||
| case 0x02: | ||
| pr.Amount = BinaryPrimitives.ReadUInt64BigEndian(value); | ||
| break; | ||
| case 0x03: | ||
| pr.Unit = value.Length == 1 && value[0] == 0x00 ? "sat" : Encoding.UTF8.GetString(value); | ||
| break; | ||
| case 0x04: | ||
| pr.OneTimeUse = value.Length == 1 && value[0] == 0x01; | ||
| break; | ||
| case 0x05: | ||
| mints.Add(Encoding.UTF8.GetString(value)); | ||
| break; | ||
| case 0x06: | ||
| pr.Memo = Encoding.UTF8.GetString(value); | ||
| break; | ||
| case 0x07: | ||
| transports.Add(DecodeTransport(value)); | ||
| break; | ||
| case 0x08: | ||
| pr.Nut10 = DecodeNut10(value); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (mints.Count > 0) | ||
| pr.Mints = mints.ToArray(); | ||
|
|
||
| pr.Transports = transports.ToArray(); | ||
|
|
||
| return pr; | ||
| } |
Contributor
There was a problem hiding this comment.
Add bounds checks for malformed TLV input.
Line 237+ assumes a full header and value are present; malformed input will throw ArgumentOutOfRangeException. Consider explicit header/value bounds checks, and validate fixed-length fields (e.g., Amount = 8 bytes).
🛠️ Proposed fix
- while (offset < data.Length)
+ while (offset < data.Length)
{
+ if (offset + 3 > data.Length)
+ throw new FormatException("TLV header truncated");
var tag = data[offset];
var length = BinaryPrimitives.ReadUInt16BigEndian(data.Slice(offset + 1, 2));
offset += 3;
+ if (offset + length > data.Length)
+ throw new FormatException("TLV value truncated");
var value = data.Slice(offset, length);
offset += length;
switch (tag)
{
case 0x01:
pr.PaymentId = Encoding.UTF8.GetString(value);
break;
case 0x02:
+ if (value.Length != 8)
+ throw new FormatException("Invalid amount length");
pr.Amount = BinaryPrimitives.ReadUInt64BigEndian(value);
break;🤖 Prompt for AI Agents
In `@DotNut/PaymentRequestBech32Encoder.cs` around lines 230 - 280, The DecodeTLV
method reads tag/length/value without bounds checks and can throw
ArgumentOutOfRangeException on malformed input; fix DecodeTLV by validating
there are at least 3 bytes remaining before reading tag and 2-byte length,
ensure the declared length does not exceed remaining data before slicing value,
and specifically validate fixed-size fields (e.g., for tag 0x02 require
value.Length == 8 before calling BinaryPrimitives.ReadUInt64BigEndian) — on
invalid or truncated input either throw a clear FormatException or return a
graceful error, and update references in DecodeTLV (mints, transports,
DecodeTransport, DecodeNut10 handling) to rely on these checks.
Comment on lines
+282
to
+343
| private static PaymentRequestTransport DecodeTransport(ReadOnlySpan<byte> data) | ||
| { | ||
| var transport = new PaymentRequestTransport(); | ||
| var offset = 0; | ||
| byte[]? targetBytes = null; | ||
| var allTuples = new List<string[]>(); | ||
|
|
||
| while (offset < data.Length) | ||
| { | ||
| var tag = data[offset]; | ||
| var length = BinaryPrimitives.ReadUInt16BigEndian(data.Slice(offset + 1, 2)); | ||
| offset += 3; | ||
| var value = data.Slice(offset, length); | ||
| offset += length; | ||
|
|
||
| switch (tag) | ||
| { | ||
| case 0x01: | ||
| transport.Type = value[0] switch | ||
| { | ||
| 0x00 => "nostr", | ||
| 0x01 => "post", | ||
| _ => throw new FormatException("Unknown transport kind") | ||
| }; | ||
| break; | ||
| case 0x02: | ||
| targetBytes = value.ToArray(); | ||
| break; | ||
| case 0x03: | ||
| allTuples.Add(DecodeTagTuple(value)); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (transport.Type == "nostr" && targetBytes != null) | ||
| { | ||
| var relays = new List<string>(); | ||
| var tags = new List<string[]>(); | ||
|
|
||
| foreach (var tuple in allTuples) | ||
| { | ||
| if (tuple is ["r", _, ..]) | ||
| relays.Add(tuple[1]); | ||
| else | ||
| tags.Add(tuple); | ||
| } | ||
|
|
||
| transport.Target = EncodeNprofile(targetBytes, relays.ToArray()); | ||
|
|
||
| if (tags.Count > 0) | ||
| transport.Tags = tags.Select(t => new Tag(t)).ToArray(); | ||
| } | ||
| else if (transport.Type == "post" && targetBytes != null) | ||
| { | ||
| transport.Target = Encoding.UTF8.GetString(targetBytes); | ||
|
|
||
| if (allTuples.Count > 0) | ||
| transport.Tags = allTuples.Select(t => new Tag(t)).ToArray(); | ||
| } | ||
|
|
||
| return transport; | ||
| } |
Contributor
There was a problem hiding this comment.
Reject transports missing required fields.
If TLV data lacks type or target, this returns a PaymentRequestTransport with null Type/Target, which can cause downstream NREs. Fail fast with a FormatException.
🛠️ Proposed fix
- if (transport.Type == "nostr" && targetBytes != null)
+ if (string.IsNullOrEmpty(transport.Type))
+ throw new FormatException("Transport missing type");
+ if (targetBytes is null)
+ throw new FormatException("Transport missing target");
+
+ if (transport.Type == "nostr")
{
var relays = new List<string>();
var tags = new List<string[]>();
foreach (var tuple in allTuples)
{
if (tuple is ["r", _, ..])
relays.Add(tuple[1]);
else
tags.Add(tuple);
}
transport.Target = EncodeNprofile(targetBytes, relays.ToArray());
if (tags.Count > 0)
transport.Tags = tags.Select(t => new Tag(t)).ToArray();
}
- else if (transport.Type == "post" && targetBytes != null)
+ else if (transport.Type == "post")
{
transport.Target = Encoding.UTF8.GetString(targetBytes);
if (allTuples.Count > 0)
transport.Tags = allTuples.Select(t => new Tag(t)).ToArray();
}🤖 Prompt for AI Agents
In `@DotNut/PaymentRequestBech32Encoder.cs` around lines 282 - 343,
DecodeTransport currently can return a PaymentRequestTransport with missing Type
or Target when TLV data lacks those fields; update DecodeTransport to validate
after parsing that transport.Type and targetBytes (or transport.Target) are
present for the declared transport kind and throw a FormatException when
required fields are missing. Specifically, after the parsing loop in
DecodeTransport, check if transport.Type is null/empty and throw new
FormatException("Missing transport type"), and for each branch require
targetBytes (or transport.Target) be present—throw new FormatException("Missing
transport target for nostr/post") before proceeding to build relays/tags or call
EncodeNprofile, ensuring no null Type/Target can be returned.
Kukks
reviewed
Feb 28, 2026
|
|
||
| public class PaymentRequestBech32Encoder | ||
| { | ||
| private static readonly Bech32Encoder Encoder = new("creqb") { StrictLength = false }; |
Owner
There was a problem hiding this comment.
May want to double check the SquashBytes setting on the encoder too
Kukks
reviewed
Feb 28, 2026
|
|
||
| #region Nostr Helpers | ||
|
|
||
| public static (byte[] Pubkey, string[] Relays) DecodeNostr(string n) |
Owner
There was a problem hiding this comment.
so much code that can be reused from the NNostr repo :'(
Kukks
approved these changes
Feb 28, 2026
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
2 participants
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.
cashubtc/nuts#294
https://github.com/cashubtc/nuts/blob/main/26.md
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.