Skip to content

feat(core): Updates to nut18 and nut26 payment request - #1059

Open
KvngMikey wants to merge 12 commits into
cashubtc:mainfrom
KvngMikey:payment_request_update
Open

feat(core): Updates to nut18 and nut26 payment request#1059
KvngMikey wants to merge 12 commits into
cashubtc:mainfrom
KvngMikey:payment_request_update

Conversation

@KvngMikey

Copy link
Copy Markdown
Member

implements cashubtc/nuts#381 to keep Nutshell up to date !

Copilot AI review requested due to automatic review settings June 21, 2026 09:50
@github-project-automation github-project-automation Bot moved this to Backlog in nutshell Jun 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.41379% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.81%. Comparing base (a935eb9) to head (49d09a5).
⚠️ Report is 25 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
cashu/core/nuts/nut26.py 94.28% 2 Missing ⚠️
cashu/core/mint_info.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1059      +/-   ##
==========================================
+ Coverage   74.54%   74.81%   +0.27%     
==========================================
  Files         115      115              
  Lines       13195    13310     +115     
==========================================
+ Hits         9836     9958     +122     
+ Misses       3359     3352       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@KvngMikey KvngMikey changed the title feat(core): new properties added to nut18 and nut26 payment request feat(core): Updates to nut18 and nut26 payment request Jun 21, 2026
@a1denvalu3

Copy link
Copy Markdown
Collaborator

@KvngMikey If I am not mistaken this only adds support for decoding such payment requests with the additional fields but does not add any logic to handle them or to create a payment request with them.

@KvngMikey

Copy link
Copy Markdown
Member Author

@KvngMikey If I am not mistaken this only adds support for decoding such payment requests with the additional fields but does not add any logic to handle them or to create a payment request with them.

@a1denvalu3 you're right that this PR only adds encoding/decoding for the new ms / fr / sm fields and doesn't add enforcement logic.

Happy to either extend this PR with that handling + a request-creation path, or land this as the serialization layer and do the behavior in a focused follow-up. Which would you prefer?

@a1denvalu3

a1denvalu3 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Happy to either extend this PR with that handling + a request-creation path

Yes please

@robwoodgate

Copy link
Copy Markdown
Contributor

The nut has been updated: ms (mint_strict) is now flipped to mp (mint_preferred) to avoid the omitted=true antipattern.

a1denvalu3
a1denvalu3 previously approved these changes Jul 1, 2026
@a1denvalu3

Copy link
Copy Markdown
Collaborator

I think there are new changes to the spec @KvngMikey

Drop `fr` and replace `sm: List[str]` with `sm: List[SupportedMethod]`
(`mn`, optional `mf`) per cashubtc/nuts#381. NUT-26 tag 0x0a is now a
repeatable supported_method sub-TLV (0x01=method, 0x02=fee u64 BE);
0x0b removed. Also fixes a mis-scoped guard error message (tag 0x01 is
id, not transport). Byte-verified against frozen spec vectors.
Add MintInfo.payment_request_method_fee (mirrors supports_mpp): reject
when the mint melts none of a request's `sm` methods, else return the
lowest matching mf, unit-filtered. Wire it into `pay`: fee applies only
when paying from a mint outside a strict/preferred list, or when no
mint list is set. Fix `request` command's --method/--fee-reserve to
match the new SupportedMethod shape (fr no longer exists).
… path

the creq pay path already selects proofs via include_fees=True, which enforces sum(proofs) - input_fee(proofs) >= amount. Add a dust-proof regression test (20x 1-sat proofs at 250 ppk) proving the payer tops up rather than underpays. No CDK oracle exists for this clause.
--method now accepts name or name:fee (e.g. bolt11:50), rejecting non-integer or negative fees. Help text states the fee only applies when the payer mint is outside a preferred mint list. Full suite green (839 passed); no fr references remain outside unrelated tor binaries.
@KvngMikey
KvngMikey force-pushed the payment_request_update branch from 11612fe to 91195fd Compare July 10, 2026 17:03
@robwoodgate

Copy link
Copy Markdown
Contributor

Noted by my agent while checking compliance to current spec:

  • Pay flow currently never reads pr.u: add "reject if pr.sm set and pr.u absent" plus "reject if pr.u != wallet.unit"
    before calling payment_request_method_fee(pr.sm, wallet.unit). With that guard the existing wallet-unit resolution becomes correct as-is.
    • Optionally a Pydantic validator on the model (u required if a or sm); the CLI request command already always sets u, so creation side is fine in practice.

a and mf are denominated in the request unit u: reject when sm is set
without u, and when u differs from the wallet unit, before the method
fee lookup.
@a1denvalu3

a1denvalu3 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator
  • [P1] Reject negative method fees during validation — cashu/core/
    base.py:1681-1683
    A NUT-18 request can supply a negative mf, which passes model validation and is selected as the
    lowest supported fee. When the fee applies, pay adds this negative value to amount_to_pay,
    causing the wallet to send less than the requested amount. Constrain mf to the NUT-26 u64 range
    so both encodings share safe semantics.

  • [P2] Require a positive payment-request amount — cashu/wallet/cli/
    cli.py:529-530
    The new request command accepts zero and negative amounts, so commands such as cashu request -1
    emit requests that the corresponding pay path cannot process correctly (0 is rejected and
    negative values reach send). Use a positive integer range at argument parsing time.

  • [P2] Validate the NUT-26 mint-preferred flag length — cashu/core/nuts/
    nut26.py:342-343
    For a malformed NUT-26 request whose 0x09 field is empty, indexing val[0] raises IndexError;
    multi-byte values are also silently accepted despite the field being specified as u8. Validate
    that the field is exactly one byte, as is already done for the adjacent single-use flag.

A negative mf passed model validation and was picked as the lowest fee,
making the payer send less than the requested amount; constrain mf to the
NUT-26 u64 range so both encodings agree. Require a positive request
amount at parse time, and reject 0x09 values that are not exactly one
byte (empty raised IndexError, multi-byte was silently accepted).
@robwoodgate

robwoodgate commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

I had my agent give this an initial look-over. The findings:

Code review

Reviewed against NUT-18 and NUT-26 at current nuts main (e0a16f8), which includes the spec consolidation this implements (cashubtc/nuts#381).

Both frozen vectors check out byte-for-byte, and the mp strict/preferred logic, the fee applicability rule, the lowest-mf-among-supported-methods rule, and the sum(proofs) - input_fee >= a + mf invariant all match the spec. Matching mn against NUT-05 method rather than the new method_name is right too. Three issues:

1. Mint URL compared by exact string equality, so a trailing-slash mismatch silently overpays

Before this PR a mismatch produced a loud "not accepted" error. Now, with mp=true, the same mismatch classifies the payer's own mint as outside the list, and the payer silently pays mf more than they owe. The codebase already normalises for this comparison elsewhere (npc.py#L160-L162 uses rstrip("/")).

# The mint list is strict unless `mp` is explicitly true (preferred)
mint_outside_list = pr.m is not None and wallet.url not in pr.m
if mint_outside_list and not pr.mp:
print(

2. NUT-05 disabled is ignored when deciding whether the mint supports a requested method

The point of sm is that the receiver can melt out of the sender's mint ("the payer MUST send ecash from a mint that supports melting the request unit"). If that mint has melting switched off, that is false, but the payer still proceeds. Nutshell's own mint emits the flag next to methods (features.py#L112-L117), so it is available here.

return None
nut_05 = self.nuts.get(MELT_NUT)
if not nut_05 or not nut_05.get("methods"):
return None
melt_methods = [MeltMethodSetting.model_validate(e) for e in nut_05["methods"]]

3. Fee message is wrong when the request carries no mint list

fee_applies is true when pr.m is None, but the message then claims the payer is outside a preferred list that does not exist.

# list, or when no mint list is set at all
fee_applies = pr.m is None or mint_outside_list
if fee_applies and method_fee:
amount_to_pay += method_fee
print(
f"Adding method fee of {wallet.unit.str(method_fee)} "
"(paying from a mint outside the preferred list)."
)

Minor: SupportedMethod.mn is not constrained to non-empty, so --method ":5" encodes an empty method name; and the payment_request_method_fee docstring omits the sm empty/None -> 0 case, which is what separates "no requirement" from None = "unsupported".

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…e reason.

Also say the request has no mint list when the fee applies for that reason, and require a non-empty method name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

5 participants