Skip to content

Conversation

@d4rp4t
Copy link
Collaborator

@d4rp4t d4rp4t commented Jan 19, 2026

Fixes #26
Spec change: cashubtc/nuts#300 (review)

Summary by CodeRabbit

Release Notes

  • Refactor

    • Streamlined core cryptographic operation signatures by removing redundant KeysetId parameters, reducing method complexity and developer integration overhead.
  • Tests

    • Comprehensive test suite updates validating functionality with simplified API signatures and internal logic improvements.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

Removes KeysetId parameter from P2BK (Pay-to-Public-Key) cryptographic computation APIs in alignment with NUT-28 specification updates. Updates method signatures for ComputeRi, BuildBlinded, and TrySignBlindPath to eliminate keyset-specific context from blinding factor calculations, with corresponding test updates and renamed test identifiers.

Changes

Cohort / File(s) Summary
Crypto Computation API
DotNut/Cashu.cs
Removed keysetId parameter from ComputeRi(byte[] Zx, int i) signature; updated hash computation logic to exclude keysetId from input material
P2PK Builder API
DotNut/P2PKBuilder.cs
Updated two BuildBlinded overload signatures to remove KeysetId keysetId parameter; replaced ComputeRi(Zx, keysetIdBytes, i) with ComputeRi(Zx, i)
Blind Signature Flow
DotNut/P2PKProofSecret.cs
Removed KeysetId from TrySignBlindPath calls; updated blind-witness path signing to use simplified ComputeRi(Zx, i) computation
Test Updates
DotNut.Tests/UnitTest1.cs
Renamed test method from Nut26_Flow to Nut28_P2BK_Flow and test class identifier; updated all BuildBlinded and ComputeRi call sites to match new signatures; added null-safety via ToString()?.ToLowerInvariant() on assertions

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #21: Directly modifies the same APIs and implementation points (ComputeRi, P2PKBuilder BuildBlinded overloads, P2PKProofSecret blind-witness flow).
  • PR #24: Overlaps on P2PKBuilder and P2PKProofSecret APIs (BuildBlinded/TrySignBlindPath) and ComputeRi/keysetId parameter usage.

Suggested reviewers

  • Kukks

Poem

🐰 A KeysetId hopped away,
BuildBlinded bloomed a simpler way,
ComputeRi sings with fewer notes,
NUT-28 in our cryptographic coats! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Chore/update p2bk' is vague and generic; it uses non-descriptive phrasing that does not clearly convey the main change (removing keyset id from P2BK blinding factor calculation). Replace with a more specific title such as 'Remove keyset id from P2BK blinding factor calculation' or 'Update P2BK derivation per NUT-28 spec change'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The pull request successfully implements the core objective from issue #26: removing the keyset id parameter from ComputeRi and BuildBlinded methods used in P2BK derivation, aligning with the draft NUT-28 spec change.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the P2BK spec change. Test renaming from Nut26 to Nut28 and API updates to ComputeRi/BuildBlinded are all within scope of removing keyset id from blinding factor calculations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@d4rp4t d4rp4t merged commit d9ab57b into Kukks:master Jan 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P2BK changes

1 participant