Skip to content

nip55: expose nip55_max_batch_size() so the batch cap has a single source - #974

Merged
kwsantiago merged 1 commit into
mainfrom
nip55-batch-size-ffi-327
Aug 23, 2026
Merged

nip55: expose nip55_max_batch_size() so the batch cap has a single source#974
kwsantiago merged 1 commit into
mainfrom
nip55-batch-size-ffi-327

Conversation

@wksantiago

@wksantiago wksantiago commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

The NIP-55 batch cap (MAX_BATCH_SIZE = 20) is duplicated in the Rust handle_batch_request and the keep-android Kotlin accumulation loop, and must be kept in sync by hand.

keep-android does not call the bulk handle_batch_request — it runs a Kotlin loop so each sign_event can interleave a preApproveNostrEvent before signing — so the cap lives on both sides of the RMP boundary.

This makes Rust the single source of truth for the cap value: MAX_BATCH_SIZE becomes pub(crate) and is exposed via nip55_max_batch_size(), mirroring the existing backup_min_passphrase_length(). keep-android reads it instead of hardcoding 20 (companion keep-android PR).

Refs privkeyio/keep-android#327.

Summary by CodeRabbit

  • New Features
    • Added access to the NIP-55 maximum batch size for mobile integrations.
    • The maximum supported batch size is 20.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ffe21b8-fecd-460b-ac7d-cd505bcc59fe

📥 Commits

Reviewing files that changed from the base of the PR and between 3a6aa55 and bd8ca8d.

📒 Files selected for processing (2)
  • keep-mobile/src/lib.rs
  • keep-mobile/src/nip55.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The mobile crate now exposes the NIP-55 maximum batch size through a new UniFFI-exported function. The underlying constant is crate-visible and remains set to 20.

Changes

NIP-55 batch size exposure

Layer / File(s) Summary
Expose the NIP-55 batch size
keep-mobile/src/nip55.rs, keep-mobile/src/lib.rs
MAX_BATCH_SIZE is crate-visible. nip55_max_batch_size() returns the constant as a u32 through the UniFFI surface.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to bd8ca

This localized change centralizes the batch-size value without any actionable merge-blocking risk remaining; it is merge-ready after normal checks and review.

Suggested reviewers: kwsantiago

Poem

A bunny checks the batch with care,
Twenty hops are waiting there.
Rust makes the number plainly known,
Kotlin and Swift can call it home.
“Hop exported!” the rabbit cheers.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states that the PR exposes nip55_max_batch_size() and establishes a single source for the batch cap.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nip55-batch-size-ffi-327

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.

@wksantiago wksantiago self-assigned this Aug 23, 2026
@wksantiago
wksantiago requested a review from kwsantiago August 23, 2026 16:39
@kwsantiago

Copy link
Copy Markdown
Contributor

Verified locally: cargo fmt --check, clippy --all-targets -D warnings, and the full keep-mobile lib suite (323 passed) are clean. CI green, including the android job, which is the one that matters here since it runs the bindgen keep-android will consume.

The framing in the description is the part worth agreeing with explicitly. The duplication exists because keep-android deliberately does not call handle_batch_request — it needs to interleave preApproveNostrEvent per event — so the cap genuinely has to be readable from the native side rather than merely enforced in Rust. Exposing the value rather than relocating the loop keeps enforcement where it belongs and removes the hand-sync, which is the right split.

Checked the things that would make this wrong rather than just reading it. MAX_BATCH_SIZE is still enforced at nip55.rs:448, so this only widens visibility and does not soften the check. Visibility goes to pub(crate), not pub, so the constant stays internal to the crate and only the accessor crosses the FFI. The accessor mirrors backup_min_passphrase_length() immediately above it, so there is one idiom for "expose a limit to native" rather than two.

No test asserting the exported value equals the enforced one, and I do not think one is warranted: nip55_max_batch_size() returns nip55::MAX_BATCH_SIZE directly, so any such test would restate the definition. The drift risk this PR closes now lives entirely on the keep-android side, in whether the Kotlin loop actually reads the FFI instead of keeping its literal 20. That is where a test earns its place, and I will check for it on the companion PR.

One sequencing note for whoever merges: keep-android #513 needs a keep.version bump to a commit containing this, so this has to land first. A pin predating it would leave nip55MaxBatchSize missing from the generated bindings.

No blockers.

@kwsantiago
kwsantiago merged commit 750d8f5 into main Aug 23, 2026
12 checks passed
@kwsantiago
kwsantiago deleted the nip55-batch-size-ffi-327 branch August 23, 2026 17:03
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.

2 participants