Skip to content

perf(mobile): derive keys natively and hash NIP-98 bodies faster - #8965

Merged
brow merged 1 commit into
mainfrom
perf/mobile-native-keys-nip98-hash
Nov 5, 2026
Merged

brow merged 1 commit into
mainfrom
perf/mobile-native-keys-nip98-hash

Conversation

@brow

@brow brow commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Two pure-Dart crypto paths that run on the UI isolate during normal use:

  • NIP-98 body hashes. Every relay HTTP query (RelaySessionNotifier._postForEvents) hashes its request body for the NIP-98 auth header with pointycastle's SHA256Digest. This switches that hash, the media upload hash and the NIP-OA preimage hash to package:crypto's sha256, which is already a dependency. The output is identical.
  • Public key derivation. AccountIdentityStore.publicKeyForNsec runs on every relay auth assertion and derived the key with the nostr package's pure-Dart curve math. It now uses the native derivePublicKeyHex (libsecp256k1) with the same validation. The default-agent writer now signs with the native signNostrEvent instead of nostr.Event.from.

Left as they were: nostr.Keys construction for signing keys (nostr 2.0.0 always derives in Dart, with no way to pass a known public key), and nostr.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. publicKeyForNsec took 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/main 3d26ed7, 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 through nostr.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.Keys derivation: 12 to 17 ms per resume on both builds.

Testing

  • New known-vector tests for the NIP-98 header hash: an empty body, abc, and the one-million-a FIPS vector. They also check the event's kind, method, signature and pubkey.
  • New publicKeyForNsec tests: the generator-point vector, matching the pure-Dart derivation, and rejecting invalid keys (scalar 0 and n, garbage, an npub).
  • These are correctness tests only. A timing test would be flaky, so none would fail if the old pure-Dart code came back.
  • just mobile-check mobile-test passes (pre-push hook).
  • Automated device run in place of manual testing: the B build above on the 12 mini, launched and resumed twice into a signed-in Block community, loading the channel list, channel notices and read state over authenticated relay queries (NIP-98 headers and relay auth assertions on every request). Not exercised on the device: media upload and the default-agent writer.

🤖 Generated with Claude Code

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]>
@brow

brow commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T03:13:19.975227Z d08f598 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: d08f598a55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@brow
brow marked this pull request as ready for review November 4, 2026 03:10
@brow
brow requested a review from a team as a code owner November 4, 2026 03:10
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: 069cb9a0fe7a67e21a57e4a8780c5208f3a0eb01...d08f598a55ee265278c38f3ee8846602eafc3f9a
  • Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.


Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability regressions found in the authorized PR range. Key validation remains intact, and the changed signing and hashing paths preserve their existing inputs and scope.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Review used read-only source and test inspection. Builds and tests were not executed, as instructed.

Generated by Codex Security Review |
Requested by: @brow |
Workflow run

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 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.
  • publicKeyForNsec still validates the scalar before the native derivation and still compares lowercase x-only keys against stored records. saveDefaultAgent passes 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.

@brow
brow merged commit 9c5032b into main Nov 5, 2026
101 of 102 checks passed
@brow
brow deleted the perf/mobile-native-keys-nip98-hash branch November 5, 2026 00:23
brow added a commit that referenced this pull request Nov 5, 2026
## 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]>
brow added a commit that referenced this pull request Nov 5, 2026
…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]>

This branch was previously deployed

1 inactive deployment
codex-review — d08f598a Deployed Nov 4, 2026 by brow via Run Codex Security Review #12416
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants