Repository navigation
perf(mobile): derive keys natively and hash NIP-98 bodies faster - #8965
Conversation
Public key derivation in AccountIdentityStore (publicKeyForNsec, which runs on every relay auth assertion, and the record read/create paths) now uses libsecp256k1 instead of pure-Dart nostr.Keys (~7 ms per call on an iPhone 12 mini). Default agent writes sign natively too. NIP-98 payload hashing, media upload hashing and NIP-OA preimage hashing use package:crypto instead of pointycastle SHA256Digest. AOT benchmark on Mac (pointycastle vs crypto): 1 KB 39.9 us vs 6.6 us (6.1x), 50 KB 1518 us vs 293 us (5.2x), 1 MB 29.6 ms vs 6.1 ms (4.9x), identical output. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Tom Brow <[email protected]>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Approving at d08f598. No blocking findings.
- Hash output is still lowercase SHA-256 hex, so NIP-98 payload tags and NIP-OA preimages match the old pointycastle path.
publicKeyForNsecstill validates the scalar before the native derivation and still compares lowercase x-only keys against stored records.saveDefaultAgentpasses the pubkey derived from the same key it signs with.
Validation gap: CI shards and Android build pass. Native-failure injection (missing FFI asset) was not exercised, and there was no device run.
## Summary Read-state blobs are NIP-44 encrypted, and the app decrypts them with pure-Dart ChaCha20 and HMAC-SHA256 on the UI isolate. Most of those decrypts repeat earlier work: - Every live subscription (each launch, resume and reconnect) replays all of the account's read-state slots, and `_handleIncomingEvent` decrypts each one again. The measured account has 27 slots of about 55 KB ciphertext each. - Before every publish, `_publishOnce` fetches the read-state snapshot and decrypts it, and decodes our own slots a second time to check they're readable. - The relay echoes each publish back, and our own blob is decrypted again. `ReadStateCrypto` now keeps a bounded memo from ciphertext to plaintext, one per conversation key. `decrypt` checks it first, and `encrypt` records the ciphertext it produced. NIP-44 payloads carry a random nonce and a MAC, so under one key the same ciphertext always decrypts to the same plaintext. Failed decrypts aren't cached and still throw every time. The memo is least recently used: a hit moves the entry to the newest end, so our own superseded blobs are evicted before live ones. It holds at most 8,000,000 code units across keys and values (about 8 MB, about 80 full slots). The first version used oldest-first eviction with a 2M bound, which saved nothing on the device: the 27 slots total about 2.6M, and a replay in slot order evicted each entry just before its next use. ## Measurements iPhone 12 mini, Profile/AOT, Tom's Block community. Each run resumes the app twice from the home screen and samples UI-isolate CPU for 15 s from each resume. | Build | `ReadStateCrypto.decrypt` per resume | | --- | --- | | `main` 14037e3 | 73 ms, 78 ms | | `main` + this PR + [#8965](#8965) | 9 ms, 4 ms | On the older base 3d26ed7 the first version of this PR measured 74/70 ms before and 73/71 ms after (no gain). An instrumented build showed 27 decrypts per resume with 0 memo hits, all on one `ReadStateCrypto` instance. With the LRU fix, the same base measured 7/3 ms. ## Testing - New tests: the same ciphertext twice runs the cipher once. A ciphertext from `encrypt` runs it zero times. Failed decrypts throw every time. The bound caps retained size. - Regression tests for the device finding: replaying 27 full-size slots twice decrypts 27 times, not 54, and a hit keeps a live entry ahead of superseded blobs under a small bound. Both fail on the first version's oldest-first memo. - Manager-level test: after the first sync, three publishes plus the echo and snapshot fetches add no decrypts (3 total, instead of 6 with the memo bypassed). - `just mobile-check mobile-test` passes (pre-push hook), after merging current `main`. - Automated device run in place of manual testing: the build above on the 12 mini, launched and resumed twice into a signed-in Block community, loading the channel list and syncing read state (snapshot fetch and live replay of all 27 slots). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <[email protected]> Co-authored-by: Tom Brow <[email protected]> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <[email protected]>
…ge (#9085) ## Summary [#8965 native key derivation](#8965) removed `AccountIdentityStore._keys`. `keysForNsec`, added by [#9029 nsec backup](#9029), which merged first, still called it. The two PRs didn't conflict textually, so `main` now fails `flutter analyze` (`_keys` isn't defined at `account_identity_store.dart:356`) and mobile CI is red. This builds the keys through `keysForHex(_nsecHex(nsec))`, the same path `withKeys` uses since #8965. It applies the same bech32 decoding and secp256k1 scalar validation that `_keys` did. ## Testing - `just mobile-check`: no issues found. - `flutter test` (full mobile suite): 4460 passed, 49 skipped. No UI change, so no app run. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tom Brow <[email protected]> Co-authored-by: Tom Brow <[email protected]> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <[email protected]>
Summary
Two pure-Dart crypto paths that run on the UI isolate during normal use:
RelaySessionNotifier._postForEvents) hashes its request body for the NIP-98 auth header with pointycastle'sSHA256Digest. This switches that hash, the media upload hash and the NIP-OA preimage hash topackage:crypto'ssha256, which is already a dependency. The output is identical.AccountIdentityStore.publicKeyForNsecruns on every relay auth assertion and derived the key with thenostrpackage's pure-Dart curve math. It now uses the nativederivePublicKeyHex(libsecp256k1) with the same validation. The default-agent writer now signs with the nativesignNostrEventinstead ofnostr.Event.from.Left as they were:
nostr.Keysconstruction for signing keys (nostr 2.0.0 always derives in Dart, with no way to pass a known public key), andnostr.Keys.generate()on the rare key-creation paths.Measurements
Device (iPhone 12 mini, Profile build, #4things captures): NIP-98 hashing took 17 to 24 ms of UI CPU per 10 to 20 s capture, mostly from channel notices queries.
publicKeyForNsectook about 7 ms per relay auth assertion.Synthetic (Mac, AOT, same output): SHA-256 of a 1 KB body takes 39.9 µs with pointycastle and 6.6 µs with
package:crypto. 50 KB: 1,518 µs vs 293 µs. 1 MB: 29.6 ms vs 6.1 ms.Device A/B after the change (iPhone 12 mini, Profile/AOT, Tom's Block community, two app resumes per build from the home screen, 15 s windows; A is
origin/main3d26ed7, B is A plus this PR and #8966): NIP-98 signing per resume went from 42 / 50 ms to 9 / 13 ms of UI CPU, and all SHA-256 work per resume from 74 / 73 ms to 33 / 33 ms. Dart public-key derivation throughnostr.Keys(left as is, see above) still takes 15 to 23 ms per resume.Same A/B on current
main(14037e3): pointycastle SHA-256 per resume went from 39 / 45 ms to 7 / 0 ms, and NIP-98 header building from 4 / 5 ms to 2 / 0 ms.nostr.Keysderivation: 12 to 17 ms per resume on both builds.Testing
abc, and the one-million-aFIPS vector. They also check the event's kind, method, signature and pubkey.publicKeyForNsectests: the generator-point vector, matching the pure-Dart derivation, and rejecting invalid keys (scalar 0 and n, garbage, an npub).just mobile-check mobile-testpasses (pre-push hook).🤖 Generated with Claude Code