feat(experiment) quantum secure nostr tests#545
Merged
Conversation
📝 WalkthroughWalkthroughAdds post‑quantum Dilithium signing and verification via Rust FFI and Dart bindings: FFI types and functions, Dart signer/verifier implementations, Rust FFI exports and implementation changes, tests, and a sample-app UI to exercise generation/verification. Changes
Sequence DiagramssequenceDiagram
participant App as Dart App
participant Signer as QsRustEventSigner
participant FFI as Dart FFI (rust_lib)
participant Rust as Rust Crypto
participant Mem as Native Memory
App->>Signer: generateKeypair(level)
Signer->>Mem: allocate outPk/outSk buffers
Signer->>FFI: qsGenerateKeypair(level, outPk, outSk)
FFI->>Rust: qs_generate_keypair
Rust->>Mem: fill QsBuffer data/len
FFI->>Signer: return buffers
Signer->>Signer: read bytes -> Uint8List, build QsKeypair
Signer->>FFI: qsFreeBuffer(outPk)
Signer->>FFI: qsFreeBuffer(outSk)
Signer->>App: return QsKeypair
sequenceDiagram
participant App as Dart App
participant Signer as QsRustEventSigner
participant FFI as Dart FFI (rust_lib)
participant Rust as Rust Crypto
participant Mem as Native Memory
App->>Signer: sign(Nip01Event)
Signer->>Mem: allocate sk,msg buffers
Signer->>FFI: qsSign(level, skPtr, skLen, msgPtr, msgLen, outSig)
FFI->>Rust: qs_sign
Rust->>Mem: write signature into QsBuffer
FFI->>Signer: return signature buffer
Signer->>Signer: convert sig -> hex, attach to event
Signer->>FFI: qsFreeBuffer(outSig)
Signer->>App: return signed Nip01Event
sequenceDiagram
participant App as Dart App
participant Verifier as QsRustEventVerifier
participant FFI as Dart FFI (rust_lib)
participant Rust as Rust Crypto
participant Mem as Native Memory
App->>Verifier: verify(Nip01Event)
Verifier->>Verifier: decode hex pk,id,sig -> bytes
Verifier->>Mem: allocate pk,msg,sig buffers
Verifier->>FFI: qsVerify(level, pkPtr, pkLen, msgPtr, msgLen, sigPtr, sigLen)
FFI->>Rust: qs_verify
Rust->>FFI: return int (1 valid / 0 invalid)
FFI->>Verifier: result
Verifier->>Mem: free allocated buffers (finally)
Verifier->>App: return true/false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1-leo
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #545 +/- ##
==========================================
- Coverage 72.92% 72.17% -0.75%
==========================================
Files 199 201 +2
Lines 10039 10139 +100
==========================================
- Hits 7321 7318 -3
- Misses 2718 2821 +103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ndk/lib/data_layer/repositories/signers/qs_rust_event_signer.dart`:
- Around line 33-35: The stream controller _pendingController never emits the
initial empty pendingRequests value, so subscribers never receive the immediate
"no pending requests" state; fix this by emitting the current pendingRequests
into the controller right after the StreamController is created (e.g., in the
class constructor or immediately after the field initialization) using
_pendingController.add(pendingRequests); apply the same pattern to the other
pending-request StreamController referenced in the later block (lines ~183-195)
so both pendingRequestsStream getters emit the initial [] state to new
subscribers.
- Around line 93-136: The sign() method currently signs the caller-supplied
event.id but returns a Nip01Event using _keypair.publicKeyHex, causing a
mismatch if the input event.pubKey differs; to fix, reconstruct the event with
the signer's public key before computing msgBytes and signing (e.g. build a
newEvent with pubKey set to _keypair.publicKeyHex and the same
createdAt/kind/tags/content), derive the id/bytes from that newEvent, call
qsSign on those bytes, and then return the newEvent populated with the produced
sigHex so the signed ID matches the returned Nip01Event.
In
`@packages/ndk/lib/data_layer/repositories/verifiers/qs_rust_event_verifier.dart`:
- Around line 22-62: The verifier currently trusts event.id; update
verify(Nip01Event event) to recompute the canonical event ID from event's
visible fields (content, tags, createdAt, kind, pubKey, etc.) using the same
canonical serialization and hashing used elsewhere in the codebase, compare the
recomputed digest (as hex) to event.id and return false if they differ, and only
then proceed to convert the recomputed hex id to bytes (via _hexToBytes) and
call rust_lib.qsVerify with that message bytes; ensure the comparison uses the
exact encoding/normalization rules the rest of the system expects so qsVerify
receives the authentic message bytes for signature verification.
In `@packages/ndk/rust/src/lib.rs`:
- Around line 169-214: The manual serializer building serialized_event must be
replaced with serde_json to guarantee exact NIP-01 JSON escaping; locate the
code that constructs serialized_event (the loop-based escaping of tags and
content in lib.rs) and replace it by creating a JSON array value like
serde_json::json!([0, pubkey, created_at, kind, tags, content]).to_string() (or
otherwise build the same heterogeneous array as a serde_json::Value) and use
that string for the event ID hashing, removing the hand-rolled escaping logic.
In `@packages/ndk/test/scenarios/qs_sign_verify_test.dart`:
- Around line 11-14: The test group labelled "qs_sign + qs_verify" is skipped
via skip: true so the new signer/verifier flow isn't executed; either remove the
skip flag from the group declaration (i.e., delete skip: true) to run the tests
under normal CI, or relocate the scenario out of test/ into a manual/ or
examples/ folder, or wrap the group in a conditional guard that checks an opt-in
environment variable (e.g., Platform.environment['RUN_QS_SCENARIOS']) before
declaring the group; update the group invocation named "qs_sign + qs_verify"
accordingly so the scenarios become runnable when intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e89d7950-a9eb-42f2-a83c-489ee4d108ee
⛔ Files ignored due to path filters (1)
packages/ndk/rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
packages/ndk/lib/data_layer/repositories/signers/qs_rust_event_signer.dartpackages/ndk/lib/data_layer/repositories/verifiers/qs_rust_event_verifier.dartpackages/ndk/lib/src/rust_lib.dartpackages/ndk/rust/Cargo.tomlpackages/ndk/rust/src/lib.rspackages/ndk/test/scenarios/qs_sign_verify_test.dart
Comment on lines
+33
to
+35
| final StreamController<List<PendingSignerRequest>> _pendingController = | ||
| StreamController<List<PendingSignerRequest>>.broadcast(); | ||
|
|
There was a problem hiding this comment.
Emit the initial empty pending-request state.
pendingRequests returns [], but pendingRequestsStream never emits it. Consumers that subscribe to the stream for local signers will stay in a waiting state instead of immediately rendering “no pending requests.”
Also applies to: 183-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ndk/lib/data_layer/repositories/signers/qs_rust_event_signer.dart`
around lines 33 - 35, The stream controller _pendingController never emits the
initial empty pendingRequests value, so subscribers never receive the immediate
"no pending requests" state; fix this by emitting the current pendingRequests
into the controller right after the StreamController is created (e.g., in the
class constructor or immediately after the field initialization) using
_pendingController.add(pendingRequests); apply the same pattern to the other
pending-request StreamController referenced in the later block (lines ~183-195)
so both pendingRequestsStream getters emit the initial [] state to new
subscribers.
Comment on lines
+93
to
+136
| Future<Nip01Event> sign(Nip01Event event) async { | ||
| final msgBytes = _hexToBytes(event.id); | ||
| if (msgBytes == null) { | ||
| throw ArgumentError('Invalid event id hex: ${event.id}'); | ||
| } | ||
|
|
||
| final skPtr = calloc<Uint8>(_keypair.keypairBytes.length); | ||
| final msgPtr = calloc<Uint8>(msgBytes.length); | ||
| final outSig = calloc<rust_lib.QsBuffer>(); | ||
|
|
||
| try { | ||
| skPtr | ||
| .asTypedList(_keypair.keypairBytes.length) | ||
| .setAll(0, _keypair.keypairBytes); | ||
| msgPtr.asTypedList(msgBytes.length).setAll(0, msgBytes); | ||
|
|
||
| final result = rust_lib.qsSign( | ||
| level, | ||
| skPtr, | ||
| _keypair.keypairBytes.length, | ||
| msgPtr, | ||
| msgBytes.length, | ||
| outSig, | ||
| ); | ||
|
|
||
| if (result != 1) { | ||
| throw StateError('Failed to sign event with Dilithium'); | ||
| } | ||
|
|
||
| final sigLen = outSig.ref.len; | ||
| final sigBytes = Uint8List.fromList(outSig.ref.data.asTypedList(sigLen)); | ||
| final sigHex = _bytesToHex(sigBytes); | ||
|
|
||
| rust_lib.qsFreeBuffer(outSig.ref); | ||
|
|
||
| return Nip01Event( | ||
| id: event.id, | ||
| pubKey: _keypair.publicKeyHex, | ||
| createdAt: event.createdAt, | ||
| kind: event.kind, | ||
| tags: event.tags, | ||
| content: event.content, | ||
| sig: sigHex, | ||
| ); |
There was a problem hiding this comment.
Rebuild the event with the signer’s pubkey before signing.
sign() signs the caller-supplied event.id, then returns a new Nip01Event with _keypair.publicKeyHex. If the input was created with a blank or different pubKey—which the new scenario test currently does—the returned event is self-inconsistent: its visible fields hash to a different ID than the one you signed.
🔧 Proposed fix
`@override`
Future<Nip01Event> sign(Nip01Event event) async {
- final msgBytes = _hexToBytes(event.id);
+ final eventToSign = Nip01Event(
+ pubKey: _keypair.publicKeyHex,
+ createdAt: event.createdAt,
+ kind: event.kind,
+ tags: event.tags,
+ content: event.content,
+ );
+
+ final msgBytes = _hexToBytes(eventToSign.id);
if (msgBytes == null) {
- throw ArgumentError('Invalid event id hex: ${event.id}');
+ throw ArgumentError('Invalid event id hex: ${eventToSign.id}');
}
@@
return Nip01Event(
- id: event.id,
- pubKey: _keypair.publicKeyHex,
- createdAt: event.createdAt,
- kind: event.kind,
- tags: event.tags,
- content: event.content,
+ id: eventToSign.id,
+ pubKey: eventToSign.pubKey,
+ createdAt: eventToSign.createdAt,
+ kind: eventToSign.kind,
+ tags: eventToSign.tags,
+ content: eventToSign.content,
sig: sigHex,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Future<Nip01Event> sign(Nip01Event event) async { | |
| final msgBytes = _hexToBytes(event.id); | |
| if (msgBytes == null) { | |
| throw ArgumentError('Invalid event id hex: ${event.id}'); | |
| } | |
| final skPtr = calloc<Uint8>(_keypair.keypairBytes.length); | |
| final msgPtr = calloc<Uint8>(msgBytes.length); | |
| final outSig = calloc<rust_lib.QsBuffer>(); | |
| try { | |
| skPtr | |
| .asTypedList(_keypair.keypairBytes.length) | |
| .setAll(0, _keypair.keypairBytes); | |
| msgPtr.asTypedList(msgBytes.length).setAll(0, msgBytes); | |
| final result = rust_lib.qsSign( | |
| level, | |
| skPtr, | |
| _keypair.keypairBytes.length, | |
| msgPtr, | |
| msgBytes.length, | |
| outSig, | |
| ); | |
| if (result != 1) { | |
| throw StateError('Failed to sign event with Dilithium'); | |
| } | |
| final sigLen = outSig.ref.len; | |
| final sigBytes = Uint8List.fromList(outSig.ref.data.asTypedList(sigLen)); | |
| final sigHex = _bytesToHex(sigBytes); | |
| rust_lib.qsFreeBuffer(outSig.ref); | |
| return Nip01Event( | |
| id: event.id, | |
| pubKey: _keypair.publicKeyHex, | |
| createdAt: event.createdAt, | |
| kind: event.kind, | |
| tags: event.tags, | |
| content: event.content, | |
| sig: sigHex, | |
| ); | |
| Future<Nip01Event> sign(Nip01Event event) async { | |
| final eventToSign = Nip01Event( | |
| pubKey: _keypair.publicKeyHex, | |
| createdAt: event.createdAt, | |
| kind: event.kind, | |
| tags: event.tags, | |
| content: event.content, | |
| ); | |
| final msgBytes = _hexToBytes(eventToSign.id); | |
| if (msgBytes == null) { | |
| throw ArgumentError('Invalid event id hex: ${eventToSign.id}'); | |
| } | |
| final skPtr = calloc<Uint8>(_keypair.keypairBytes.length); | |
| final msgPtr = calloc<Uint8>(msgBytes.length); | |
| final outSig = calloc<rust_lib.QsBuffer>(); | |
| try { | |
| skPtr | |
| .asTypedList(_keypair.keypairBytes.length) | |
| .setAll(0, _keypair.keypairBytes); | |
| msgPtr.asTypedList(msgBytes.length).setAll(0, msgBytes); | |
| final result = rust_lib.qsSign( | |
| level, | |
| skPtr, | |
| _keypair.keypairBytes.length, | |
| msgPtr, | |
| msgBytes.length, | |
| outSig, | |
| ); | |
| if (result != 1) { | |
| throw StateError('Failed to sign event with Dilithium'); | |
| } | |
| final sigLen = outSig.ref.len; | |
| final sigBytes = Uint8List.fromList(outSig.ref.data.asTypedList(sigLen)); | |
| final sigHex = _bytesToHex(sigBytes); | |
| rust_lib.qsFreeBuffer(outSig.ref); | |
| return Nip01Event( | |
| id: eventToSign.id, | |
| pubKey: eventToSign.pubKey, | |
| createdAt: eventToSign.createdAt, | |
| kind: eventToSign.kind, | |
| tags: eventToSign.tags, | |
| content: eventToSign.content, | |
| sig: sigHex, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ndk/lib/data_layer/repositories/signers/qs_rust_event_signer.dart`
around lines 93 - 136, The sign() method currently signs the caller-supplied
event.id but returns a Nip01Event using _keypair.publicKeyHex, causing a
mismatch if the input event.pubKey differs; to fix, reconstruct the event with
the signer's public key before computing msgBytes and signing (e.g. build a
newEvent with pubKey set to _keypair.publicKeyHex and the same
createdAt/kind/tags/content), derive the id/bytes from that newEvent, call
qsSign on those bytes, and then return the newEvent populated with the produced
sigHex so the signed ID matches the returned Nip01Event.
Comment on lines
+22
to
+62
| Future<bool> verify(Nip01Event event) async { | ||
| if (event.sig == null) { | ||
| return false; | ||
| } | ||
|
|
||
| // Decode hex-encoded public key and signature to bytes | ||
| final pkBytes = _hexToBytes(event.pubKey); | ||
| final sigBytes = _hexToBytes(event.sig!); | ||
|
|
||
| if (pkBytes == null || sigBytes == null) { | ||
| return false; | ||
| } | ||
|
|
||
| // The message to verify is the event id (the hash of the serialized event) | ||
| final msgBytes = _hexToBytes(event.id); | ||
| if (msgBytes == null) { | ||
| return false; | ||
| } | ||
|
|
||
| // Allocate native memory | ||
| final pkPtr = calloc<Uint8>(pkBytes.length); | ||
| final msgPtr = calloc<Uint8>(msgBytes.length); | ||
| final sigPtr = calloc<Uint8>(sigBytes.length); | ||
|
|
||
| try { | ||
| // Copy bytes to native memory | ||
| pkPtr.asTypedList(pkBytes.length).setAll(0, pkBytes); | ||
| msgPtr.asTypedList(msgBytes.length).setAll(0, msgBytes); | ||
| sigPtr.asTypedList(sigBytes.length).setAll(0, sigBytes); | ||
|
|
||
| final result = rust_lib.qsVerify( | ||
| level, | ||
| pkPtr, | ||
| pkBytes.length, | ||
| msgPtr, | ||
| msgBytes.length, | ||
| sigPtr, | ||
| sigBytes.length, | ||
| ); | ||
|
|
||
| return result == 1; |
There was a problem hiding this comment.
Recompute the canonical event ID before verifying.
This verifier treats event.id as trusted input and only checks the Dilithium signature over that digest. A caller can change content, tags, createdAt, or kind after signing and still get true, because the native call never sees those fields. EventVerifier needs to reject events whose visible payload no longer hashes to event.id.
🔧 Proposed fix
+import '../../../domain_layer/entities/nip_01_utils.dart';
+
`@override`
Future<bool> verify(Nip01Event event) async {
if (event.sig == null) {
return false;
}
+
+ final expectedId = Nip01Utils.calculateId(event);
+ if (expectedId != event.id) {
+ return false;
+ }
// Decode hex-encoded public key and signature to bytes
final pkBytes = _hexToBytes(event.pubKey);
final sigBytes = _hexToBytes(event.sig!);
@@
- final msgBytes = _hexToBytes(event.id);
+ final msgBytes = _hexToBytes(expectedId);
if (msgBytes == null) {
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Future<bool> verify(Nip01Event event) async { | |
| if (event.sig == null) { | |
| return false; | |
| } | |
| // Decode hex-encoded public key and signature to bytes | |
| final pkBytes = _hexToBytes(event.pubKey); | |
| final sigBytes = _hexToBytes(event.sig!); | |
| if (pkBytes == null || sigBytes == null) { | |
| return false; | |
| } | |
| // The message to verify is the event id (the hash of the serialized event) | |
| final msgBytes = _hexToBytes(event.id); | |
| if (msgBytes == null) { | |
| return false; | |
| } | |
| // Allocate native memory | |
| final pkPtr = calloc<Uint8>(pkBytes.length); | |
| final msgPtr = calloc<Uint8>(msgBytes.length); | |
| final sigPtr = calloc<Uint8>(sigBytes.length); | |
| try { | |
| // Copy bytes to native memory | |
| pkPtr.asTypedList(pkBytes.length).setAll(0, pkBytes); | |
| msgPtr.asTypedList(msgBytes.length).setAll(0, msgBytes); | |
| sigPtr.asTypedList(sigBytes.length).setAll(0, sigBytes); | |
| final result = rust_lib.qsVerify( | |
| level, | |
| pkPtr, | |
| pkBytes.length, | |
| msgPtr, | |
| msgBytes.length, | |
| sigPtr, | |
| sigBytes.length, | |
| ); | |
| return result == 1; | |
| import '../../../domain_layer/entities/nip_01_utils.dart'; | |
| `@override` | |
| Future<bool> verify(Nip01Event event) async { | |
| if (event.sig == null) { | |
| return false; | |
| } | |
| final expectedId = Nip01Utils.calculateId(event); | |
| if (expectedId != event.id) { | |
| return false; | |
| } | |
| // Decode hex-encoded public key and signature to bytes | |
| final pkBytes = _hexToBytes(event.pubKey); | |
| final sigBytes = _hexToBytes(event.sig!); | |
| if (pkBytes == null || sigBytes == null) { | |
| return false; | |
| } | |
| // The message to verify is the event id (the hash of the serialized event) | |
| final msgBytes = _hexToBytes(expectedId); | |
| if (msgBytes == null) { | |
| return false; | |
| } | |
| // Allocate native memory | |
| final pkPtr = calloc<Uint8>(pkBytes.length); | |
| final msgPtr = calloc<Uint8>(msgBytes.length); | |
| final sigPtr = calloc<Uint8>(sigBytes.length); | |
| try { | |
| // Copy bytes to native memory | |
| pkPtr.asTypedList(pkBytes.length).setAll(0, pkBytes); | |
| msgPtr.asTypedList(msgBytes.length).setAll(0, msgBytes); | |
| sigPtr.asTypedList(sigBytes.length).setAll(0, sigBytes); | |
| final result = rust_lib.qsVerify( | |
| level, | |
| pkPtr, | |
| pkBytes.length, | |
| msgPtr, | |
| msgBytes.length, | |
| sigPtr, | |
| sigBytes.length, | |
| ); | |
| return result == 1; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/ndk/lib/data_layer/repositories/verifiers/qs_rust_event_verifier.dart`
around lines 22 - 62, The verifier currently trusts event.id; update
verify(Nip01Event event) to recompute the canonical event ID from event's
visible fields (content, tags, createdAt, kind, pubKey, etc.) using the same
canonical serialization and hashing used elsewhere in the codebase, compare the
recomputed digest (as hex) to event.id and return false if they differ, and only
then proceed to convert the recomputed hex id to bytes (via _hexToBytes) and
call rust_lib.qsVerify with that message bytes; ensure the comparison uses the
exact encoding/normalization rules the rest of the system expects so qsVerify
receives the authentic message bytes for signature verification.
Comment on lines
+169
to
+214
| let mut serialized_event = String::with_capacity(256); | ||
| serialized_event.push_str("[0,\""); | ||
| serialized_event.push_str(pubkey); | ||
| serialized_event.push_str("\","); | ||
| serialized_event.push_str(&created_at.to_string()); | ||
| serialized_event.push(','); | ||
| serialized_event.push_str(&kind.to_string()); | ||
| serialized_event.push_str(",["); | ||
|
|
||
| for (i, tag) in tags.iter().enumerate() { | ||
| if i > 0 { | ||
| serialized_event.push(','); | ||
| } | ||
| serialized_event.push('['); | ||
| for (j, item) in tag.iter().enumerate() { | ||
| if j > 0 { | ||
| serialized_event.push(','); | ||
| } | ||
| serialized_event.push('"'); | ||
| for c in item.chars() { | ||
| match c { | ||
| '"' => serialized_event.push_str("\\\""), | ||
| '\\' => serialized_event.push_str("\\\\"), | ||
| '\n' => serialized_event.push_str("\\n"), | ||
| '\r' => serialized_event.push_str("\\r"), | ||
| '\t' => serialized_event.push_str("\\t"), | ||
| _ => serialized_event.push(c), | ||
| } | ||
| } | ||
| serialized_event.push('"'); | ||
| } | ||
| serialized_event.push(']'); | ||
| } | ||
|
|
||
| serialized_event.push_str("],\""); | ||
| for c in content.chars() { | ||
| match c { | ||
| '"' => serialized_event.push_str("\\\""), | ||
| '\\' => serialized_event.push_str("\\\\"), | ||
| '\n' => serialized_event.push_str("\\n"), | ||
| '\r' => serialized_event.push_str("\\r"), | ||
| '\t' => serialized_event.push_str("\\t"), | ||
| _ => serialized_event.push(c), | ||
| } | ||
| } | ||
| serialized_event.push_str("\"]"); |
There was a problem hiding this comment.
Don’t hand-roll NIP-01 JSON serialization here.
Dart calculates event IDs with json.encode([0, pubKey, createdAt, kind, tags, content]), but this serializer only escapes ", \, \n, \r, and \t. Valid tag/content strings containing other control characters such as \b, \f, or \u0001 will hash to a different ID in Rust, so otherwise valid events will fail verification. Reuse serde_json for the exact array encoding instead of maintaining a partial escaper.
🔧 Proposed fix
fn hash_event_data_internal(
pubkey: &str,
created_at: u64,
kind: u16,
tags: &[Vec<String>],
content: &str,
) -> String {
- let mut serialized_event = String::with_capacity(256);
- serialized_event.push_str("[0,\"");
- serialized_event.push_str(pubkey);
- serialized_event.push_str("\",");
- serialized_event.push_str(&created_at.to_string());
- serialized_event.push(',');
- serialized_event.push_str(&kind.to_string());
- serialized_event.push_str(",[");
-
- for (i, tag) in tags.iter().enumerate() {
- if i > 0 {
- serialized_event.push(',');
- }
- serialized_event.push('[');
- for (j, item) in tag.iter().enumerate() {
- if j > 0 {
- serialized_event.push(',');
- }
- serialized_event.push('"');
- for c in item.chars() {
- match c {
- '"' => serialized_event.push_str("\\\""),
- '\\' => serialized_event.push_str("\\\\"),
- '\n' => serialized_event.push_str("\\n"),
- '\r' => serialized_event.push_str("\\r"),
- '\t' => serialized_event.push_str("\\t"),
- _ => serialized_event.push(c),
- }
- }
- serialized_event.push('"');
- }
- serialized_event.push(']');
- }
-
- serialized_event.push_str("],\"");
- for c in content.chars() {
- match c {
- '"' => serialized_event.push_str("\\\""),
- '\\' => serialized_event.push_str("\\\\"),
- '\n' => serialized_event.push_str("\\n"),
- '\r' => serialized_event.push_str("\\r"),
- '\t' => serialized_event.push_str("\\t"),
- _ => serialized_event.push(c),
- }
- }
- serialized_event.push_str("\"]");
+ let serialized_event = serde_json::to_string(&serde_json::json!([
+ 0,
+ pubkey,
+ created_at,
+ kind,
+ tags,
+ content
+ ]))
+ .unwrap();
let mut hasher = Sha256::new();
hasher.update(serialized_event.as_bytes());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ndk/rust/src/lib.rs` around lines 169 - 214, The manual serializer
building serialized_event must be replaced with serde_json to guarantee exact
NIP-01 JSON escaping; locate the code that constructs serialized_event (the
loop-based escaping of tags and content in lib.rs) and replace it by creating a
JSON array value like serde_json::json!([0, pubkey, created_at, kind, tags,
content]).to_string() (or otherwise build the same heterogeneous array as a
serde_json::Value) and use that string for the event ID hashing, removing the
hand-rolled escaping logic.
Comment on lines
+11
to
+14
| group( | ||
| "qs_sign + qs_verify", | ||
| skip: true, | ||
| () { |
There was a problem hiding this comment.
Unskip or relocate this QS scenario suite.
skip: true means dart test never exercises either of these paths, so this PR lands with zero runnable coverage for the new signer/verifier flow. If these are meant to stay manual experiments, move them out of test/; otherwise gate them behind an opt-in job instead of skipping the whole group.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ndk/test/scenarios/qs_sign_verify_test.dart` around lines 11 - 14,
The test group labelled "qs_sign + qs_verify" is skipped via skip: true so the
new signer/verifier flow isn't executed; either remove the skip flag from the
group declaration (i.e., delete skip: true) to run the tests under normal CI, or
relocate the scenario out of test/ into a manual/ or examples/ folder, or wrap
the group in a conditional guard that checks an opt-in environment variable
(e.g., Platform.environment['RUN_QS_SCENARIOS']) before declaring the group;
update the group invocation named "qs_sign + qs_verify" accordingly so the
scenarios become runnable when intended.
frnandu
requested changes
Mar 26, 2026
| const Tab(text: "Wallets"), | ||
| const Tab(text: 'Widgets'), | ||
| const Tab(text: 'Pending'), | ||
| const Tab(text: quantumSecureTabName), |
Collaborator
There was a problem hiding this comment.
I would not expose it as a tab in the sample-app. We need some kind of more hidden list of experimental things to test on the demo-app other that the tabs. I would want to have tabs exposing main usecases/features.
frnandu
approved these changes
Mar 26, 2026
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sample-app/lib/quantum_secure_page.dart`:
- Around line 50-53: Move the native setup (calls to
QsRustEventSigner.generateKeypair, QsRustEventVerifier, and QsRustEventSigner
construction) into the existing try block that wraps the button handler so any
exceptions are caught and trigger the snackbar; specifically, remove the
top-level calls to generateKeypair(level: _level), new
QsRustEventVerifier(level: _level), and new QsRustEventSigner(level: _level,
keypair: myKeyPair) and place them inside the same try that contains the
snackbar/error handling so thrown errors from generateKeypair or the
signer/verifier constructors are caught and handled.
- Around line 241-243: When the dropdown changes you must not only update _level
but also clear any cached benchmark state so the next action uses fresh
artifacts; inside the onChanged handler (the setState call that sets _level)
also reset/clear the cached signer and verifier, the keypair, timing results and
any stored signed events (for example variables like _signer, _verifier,
_keypair, _timings, _signedEvents or any similarly named fields), and reset any
in-progress/finished flags so the UI and logic reflect the new level.
- Around line 78-85: The batch is creating Nip01Event objects with pubKey: '' so
the constructor hashes an empty pubkey into the event id, causing signed events
to have ids that don't match their serialized payload; update the batch
construction to supply the real public key (the signer/keypair's public key)
into the Nip01Event pubKey field (or construct the event only after you have the
real pubKey), or if the class exposes an id recompute method, call that after
assigning the correct pubKey so the event.id matches the final serialized
payload; locate the batch creation that maps to Nip01Event and replace the empty
string pubKey with the actual public key source used by sign().
- Around line 91-93: The current use of unsignedEvents.map(...) into Future.wait
calls QsRustEventSigner.sign (and similarly QsRustEventVerifier.verify) which
perform synchronous FFI and therefore block the UI isolate; change this so the
native work is moved off the main isolate — either (A) create a batched FFI
entrypoint (e.g., signBatch / verifyBatch in the Rust side and expose it to
Dart) and call that from the Dart code instead of per-event sign/verify, or (B)
run per-event sign/verify calls inside a Dart isolate (e.g., using compute or
spawn/isolate) and await Future.wait on those isolate-backed Futures; update the
code that currently calls _signer!.sign(...) and the corresponding verifier
usage so the blocking FFI runs in the isolate or via the new batched API to
avoid freezing the UI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 057578c3-3203-4032-8ade-65877b84d720
📒 Files selected for processing (2)
packages/sample-app/lib/main.dartpackages/sample-app/lib/quantum_secure_page.dart
Comment on lines
+50
to
+53
| final myKeyPair = QsRustEventSigner.generateKeypair(level: _level); | ||
|
|
||
| final eventVerifier = QsRustEventVerifier(level: _level); | ||
| final signer = QsRustEventSigner(level: _level, keypair: myKeyPair); |
There was a problem hiding this comment.
Move the native setup inside the existing try.
Any exception from generateKeypair(...) or the signer/verifier construction here happens before Line 70, so it bypasses the snackbar handling and bubbles out of the button handler.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sample-app/lib/quantum_secure_page.dart` around lines 50 - 53, Move
the native setup (calls to QsRustEventSigner.generateKeypair,
QsRustEventVerifier, and QsRustEventSigner construction) into the existing try
block that wraps the button handler so any exceptions are caught and trigger the
snackbar; specifically, remove the top-level calls to generateKeypair(level:
_level), new QsRustEventVerifier(level: _level), and new
QsRustEventSigner(level: _level, keypair: myKeyPair) and place them inside the
same try that contains the snackbar/error handling so thrown errors from
generateKeypair or the signer/verifier constructors are caught and handled.
Comment on lines
+78
to
+85
| (index) => Nip01Event( | ||
| content: _messageController.text.isEmpty | ||
| ? 'hello message $index' | ||
| : _messageController.text, | ||
| kind: 1, | ||
| pubKey: '', | ||
| tags: [], | ||
| ), |
There was a problem hiding this comment.
Use the real pubkey when building the batch.
Nip01Event computes id in the constructor. With pubKey: '', every id is hashed against the empty pubkey, and sign() later swaps in the real public key without recomputing id. The demo therefore produces signed events whose id does not match their serialized Nostr payload.
🛠️ Suggested fix
final unsignedEvents = List.generate(
_eventCount,
(index) => Nip01Event(
content: _messageController.text.isEmpty
? 'hello message $index'
: _messageController.text,
kind: 1,
- pubKey: '',
+ pubKey: _keyPair!.publicKeyHex,
tags: [],
),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| (index) => Nip01Event( | |
| content: _messageController.text.isEmpty | |
| ? 'hello message $index' | |
| : _messageController.text, | |
| kind: 1, | |
| pubKey: '', | |
| tags: [], | |
| ), | |
| (index) => Nip01Event( | |
| content: _messageController.text.isEmpty | |
| ? 'hello message $index' | |
| : _messageController.text, | |
| kind: 1, | |
| pubKey: _keyPair!.publicKeyHex, | |
| tags: [], | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sample-app/lib/quantum_secure_page.dart` around lines 78 - 85, The
batch is creating Nip01Event objects with pubKey: '' so the constructor hashes
an empty pubkey into the event id, causing signed events to have ids that don't
match their serialized payload; update the batch construction to supply the real
public key (the signer/keypair's public key) into the Nip01Event pubKey field
(or construct the event only after you have the real pubKey), or if the class
exposes an id recompute method, call that after assigning the correct pubKey so
the event.id matches the final serialized payload; locate the batch creation
that maps to Nip01Event and replace the empty string pubKey with the actual
public key source used by sign().
Comment on lines
+91
to
+93
| final signedEvents = await Future.wait( | ||
| unsignedEvents.map((event) => _signer!.sign(event)), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "quantum_secure_page.dart" -type fRepository: relaystr/ndk
Length of output: 106
🏁 Script executed:
find . -name "*signer*" -o -name "*verifier*" | grep -E "\.(dart|rs)$"Repository: relaystr/ndk
Length of output: 2440
🏁 Script executed:
git ls-files | grep -E "(QsRustEventSigner|QsRustEventVerifier|signer|verifier)" | head -20Repository: relaystr/ndk
Length of output: 1370
🏁 Script executed:
cat -n packages/sample-app/lib/quantum_secure_page.dart | head -150Repository: relaystr/ndk
Length of output: 5295
🏁 Script executed:
cat -n packages/ndk/lib/data_layer/repositories/signers/qs_rust_event_signer.dartRepository: relaystr/ndk
Length of output: 7851
🏁 Script executed:
cat -n packages/ndk/lib/data_layer/repositories/verifiers/qs_rust_event_verifier.dartRepository: relaystr/ndk
Length of output: 2960
Future.wait does not parallelize the Rust FFI work here.
QsRustEventSigner.sign(...) and QsRustEventVerifier.verify(...) execute synchronous FFI calls without isolate wrapping. When map() iterates over events, each call immediately blocks the UI isolate on the native FFI operation (lines 109 and 52 respectively), so the entire batch runs sequentially and blocks before Future.wait can yield. This will freeze the page, and the reported timings measure blocked-main-isolate batch latency rather than concurrent throughput. Move this to an isolate or expose a batched native API if these numbers are meant to benchmark feasibility.
Also applies to: 147-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sample-app/lib/quantum_secure_page.dart` around lines 91 - 93, The
current use of unsignedEvents.map(...) into Future.wait calls
QsRustEventSigner.sign (and similarly QsRustEventVerifier.verify) which perform
synchronous FFI and therefore block the UI isolate; change this so the native
work is moved off the main isolate — either (A) create a batched FFI entrypoint
(e.g., signBatch / verifyBatch in the Rust side and expose it to Dart) and call
that from the Dart code instead of per-event sign/verify, or (B) run per-event
sign/verify calls inside a Dart isolate (e.g., using compute or spawn/isolate)
and await Future.wait on those isolate-backed Futures; update the code that
currently calls _signer!.sign(...) and the corresponding verifier usage so the
blocking FFI runs in the isolate or via the new batched API to avoid freezing
the UI.
Comment on lines
+241
to
+243
| onChanged: (value) { | ||
| setState(() => _level = value!); | ||
| }, |
There was a problem hiding this comment.
Reset the cached benchmark state when the level changes.
This handler only updates _level. The old signer/verifier, keypair, timings, and signed events remain live, so the next action can still use the previous level while the UI shows the new one.
🧹 Suggested fix
onChanged: (value) {
- setState(() => _level = value!);
+ setState(() {
+ _level = value!;
+ _signer = null;
+ _verifier = null;
+ _keyPair = null;
+ _signedEvents = [];
+ _lastEventJson = null;
+ _hasSignature = false;
+ _signTimeMs = null;
+ _verifyTimeMs = null;
+ _failedVerifications = null;
+ });
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onChanged: (value) { | |
| setState(() => _level = value!); | |
| }, | |
| onChanged: (value) { | |
| setState(() { | |
| _level = value!; | |
| _signer = null; | |
| _verifier = null; | |
| _keyPair = null; | |
| _signedEvents = []; | |
| _lastEventJson = null; | |
| _hasSignature = false; | |
| _signTimeMs = null; | |
| _verifyTimeMs = null; | |
| _failedVerifications = null; | |
| }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sample-app/lib/quantum_secure_page.dart` around lines 241 - 243,
When the dropdown changes you must not only update _level but also clear any
cached benchmark state so the next action uses fresh artifacts; inside the
onChanged handler (the setState call that sets _level) also reset/clear the
cached signer and verifier, the keypair, timing results and any stored signed
events (for example variables like _signer, _verifier, _keypair, _timings,
_signedEvents or any similarly named fields), and reset any in-progress/finished
flags so the UI and logic reflect the new level.
nogringo
approved these changes
Mar 28, 2026
nogringo
approved these changes
Apr 12, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sample-app/lib/main.dart`:
- Around line 143-145: The Quantum Secure tab label is hardcoded; replace the
static const String quantumSecureTabName = 'Quantum Secure' and any usage in
_buildTabs(BuildContext) with a localized lookup using the app's localization
helper (e.g., call AppLocalizations.of(context).quantumSecure or
S.of(context).quantumSecure) so the tab label comes from the localization
resources; update both the constant or remove it and use the localized string in
Tab creation inside _buildTabs and at the other occurrence noted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8551d977-26e9-4146-8181-b46503300859
⛔ Files ignored due to path filters (2)
packages/ndk/rust/Cargo.lockis excluded by!**/*.lockpackages/sample-app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/ndk/rust/Cargo.tomlpackages/sample-app/lib/main.dart
✅ Files skipped from review due to trivial changes (1)
- packages/ndk/rust/Cargo.toml
Comment on lines
+143
to
145
| static const String nwcTabName = 'NWC'; | ||
| static const String quantumSecureTabName = 'Quantum Secure'; | ||
| List<Tab> _buildTabs(BuildContext context) { |
There was a problem hiding this comment.
Localize the “Quantum Secure” tab label instead of hardcoding English text.
This tab label bypasses the app’s localization flow, so non-English locales will still see English.
Suggested change
- static const String quantumSecureTabName = 'Quantum Secure';
+ // Add `tabQuantumSecure` to localization resources and use it here.
List<Tab> _buildTabs(BuildContext context) {
final l10n = context.l10n;
return <Tab>[
Tab(text: l10n.tabAccounts),
Tab(text: l10n.tabProfile),
Tab(text: l10n.tabRelays),
Tab(text: l10n.tabBlossom),
Tab(text: l10n.tabWallets),
Tab(text: l10n.tabWidgets),
- const Tab(text: quantumSecureTabName),
+ Tab(text: l10n.tabQuantumSecure),
];
}Also applies to: 154-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sample-app/lib/main.dart` around lines 143 - 145, The Quantum Secure
tab label is hardcoded; replace the static const String quantumSecureTabName =
'Quantum Secure' and any usage in _buildTabs(BuildContext) with a localized
lookup using the app's localization helper (e.g., call
AppLocalizations.of(context).quantumSecure or S.of(context).quantumSecure) so
the tab label comes from the localization resources; update both the constant or
remove it and use the localized string in Tab creation inside _buildTabs and at
the other occurrence noted.
pats2sats
pushed a commit
to sudonym-btc/ndk
that referenced
this pull request
May 20, 2026
…tr-tests feat(experiment) quantum secure nostr tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Testing the feasibility and constraints of quantum secure algorithms for Nostr
Summary by CodeRabbit
New Features
Tests