Add key package admin #621
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughReplaced the public opaque Event bridge with a non-opaque FlutterEvent across Rust and Dart. Added account key-package management APIs (list, publish, delete single/all) and updated generated FRB bindings/encoders. Adjusted multiple settings screens to use IconButton and explicit WnImage sizing; bumped whitenoise git rev. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DevUI as DeveloperSettingsScreen
participant DartAPI as Dart accounts.dart
participant FRB as frb_generated
participant RustAPI as rust::api::accounts
participant Core as whitenoise core
User->>DevUI: Tap "Inspect relay key packages"
DevUI->>DartAPI: accountKeyPackages(accountPubkey)
DartAPI->>FRB: FFI call (list FlutterEvent)
FRB->>RustAPI: account_key_packages(pubkey)
RustAPI->>Core: fetch key packages from relays
Core-->>RustAPI: Vec<Event>
RustAPI->>RustAPI: map Event -> FlutterEvent
RustAPI-->>FRB: Vec<FlutterEvent>
FRB-->>DartAPI: List<FlutterEvent>
DartAPI-->>DevUI: List<FlutterEvent>
DevUI-->>User: Render list / empty state
User->>DevUI: Tap "Publish new key package"
DevUI->>DartAPI: publishAccountKeyPackage(accountPubkey)
DartAPI->>FRB: FFI call (void)
FRB->>RustAPI: publish_account_key_package(pubkey)
RustAPI->>Core: publish to relays
Core-->>RustAPI: ()
RustAPI-->>FRB: ()
FRB-->>DartAPI: ()
DartAPI-->>DevUI: success (optional refresh)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/settings/app_settings/app_settings_screen.dart (1)
35-36: Fix user-facing typo in dialog title.Double “app” reads awkwardly.
- title: 'Delete app app data', + title: 'Delete app data',
🧹 Nitpick comments (15)
lib/ui/settings/profile/edit_profile_screen.dart (1)
116-124: Add a11y tooltip on the back button.Small usability win; aligns headers across screens.
Apply:
IconButton( - onPressed: () => context.pop(), + onPressed: () => context.pop(), + tooltip: 'Back', icon: WnImage( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, color: context.colors.primary, ), ),lib/ui/settings/network/network_screen.dart (2)
149-149: Standardize navigation: usecontext.pop()like other settings screens.Keeps back handling consistent with GoRouter usage elsewhere.
Apply:
- onPressed: () => Navigator.of(context).pop(), + onPressed: () => context.pop(),Add import at the top if missing:
import 'package:go_router/go_router.dart';
148-156: Add a11y tooltip on the back button.IconButton( - onPressed: () => context.pop(), + onPressed: () => context.pop(), + tooltip: 'Back', icon: WnImage( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, color: context.colors.primary, ), ),lib/ui/settings/profile_keys/profile_keys_screen.dart (1)
88-109: Header refactor looks good; add tooltip for better accessibility.IconButton( - onPressed: () => context.pop(), + onPressed: () => context.pop(), + tooltip: 'Back', icon: WnImage( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, color: context.colors.primary, ), ),lib/ui/settings/donate/donate_screen.dart (1)
42-65: Header migration to IconButton is solid; add tooltip for a11y.IconButton( - onPressed: () => context.pop(), + onPressed: () => context.pop(), + tooltip: 'Back', icon: WnImage( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, color: context.colors.primary, ), ),lib/ui/settings/app_settings/app_settings_screen.dart (1)
191-213: LGTM: consistent header pattern; consider adding tooltip.IconButton( - onPressed: () => context.pop(), + onPressed: () => context.pop(), + tooltip: 'Back', icon: WnImage( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, color: context.colors.primary, ), ),rust/src/api/accounts.rs (2)
27-36: FlutterEvent shape looks good for FRB; consider future-proofing tags.If tags need structured access in Flutter later, a typed struct may be preferable over
Vec<String>. Fine for now.Would you like a follow-up patch to introduce a lightweight FlutterTag type consistent with messages/groups APIs?
210-218: Publish endpoint is fine; consider emitting a status/result in future.Returning unit makes UI confirmation rely on side-effects; optional enhancement later.
lib/ui/settings/developer/developer_settings_screen.dart (5)
37-37: Annotate local variable types for clarity and to match our Dart guidelines.Add explicit types for these locals.
- final activePubkey = ref.read(activePubkeyProvider) ?? ''; + final String activePubkey = ref.read(activePubkeyProvider) ?? '';- final activePubkey = ref.read(activePubkeyProvider) ?? ''; + final String activePubkey = ref.read(activePubkeyProvider) ?? '';- final activePubkey = ref.read(activePubkeyProvider) ?? ''; + final String activePubkey = ref.read(activePubkeyProvider) ?? '';- final activePubkey = ref.read(activePubkeyProvider) ?? ''; + final String activePubkey = ref.read(activePubkeyProvider) ?? '';Also applies to: 106-106, 133-133, 161-161
44-52: Type the dialog result explicitly.
showDialog<bool>returnsFuture<bool?>. Make this explicit to avoid implicitdynamic.- final confirmed = await showDialog<bool>( + final bool? confirmed = await showDialog<bool>(- final confirmed = await showDialog<bool>( + final bool? confirmed = await showDialog<bool>(Also applies to: 168-176
79-83: Guard setState after awaited dialogs.If the widget was popped while the dialog was open, the subsequent
setStatecan hit a disposed state. Add a mounted check right before setting loading.- if (confirmed != true) return; - - setState(() => _isLoading = true); + if (confirmed != true) return; + if (!mounted) return; + setState(() => _isLoading = true);- if (confirmed != true) return; - - setState(() => _isLoading = true); + if (confirmed != true) return; + if (!mounted) return; + setState(() => _isLoading = true);Also applies to: 203-206
249-250: Use height scaling for vertical paddings/sizes.Minor consistency fix: prefer
.hfor vertical metrics.- width: 24.w, - height: 24.w, + width: 24.w, + height: 24.h,- padding: EdgeInsets.only( - left: 16.w, - right: 16.w, - bottom: 24.w, - ), + padding: EdgeInsets.only( + left: 16.w, + right: 16.w, + bottom: 24.h, + ),Also applies to: 267-270
1-13: Optional: rely on app_theme re-export instead of direct screenutil import.If you import
app_theme.dart, you can drop the directflutter_screenutilimport per our learnings; it keeps theming/sizing imports consistent project-wide.-import 'package:flutter_screenutil/flutter_screenutil.dart'; +import 'package:whitenoise/ui/core/themes/src/app_theme.dart';Note: keep
extensions.dartifcontext.colorsisn’t re-exported byapp_theme.dart.lib/src/rust/api/accounts.dart (1)
83-91: Key package APIs look good; minor param naming nit.Signatures are correct and align with FRB calls. Consider standardizing on either
pubkeyoraccountPubkeyacross accounts APIs for consistency.lib/src/rust/frb_generated.dart (1)
153-161: Parameter naming consistency (pubkey vs accountPubkey).New endpoints use accountPubkey while others use pubkey. If intentional for clarity, ok; otherwise consider aligning for consistency.
Also applies to: 248-251, 101-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
ios/Podfile.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lockrust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
lib/src/rust/api/accounts.dart(3 hunks)lib/src/rust/frb_generated.dart(74 hunks)lib/src/rust/frb_generated.io.dart(12 hunks)lib/ui/settings/app_settings/app_settings_screen.dart(2 hunks)lib/ui/settings/developer/developer_settings_screen.dart(4 hunks)lib/ui/settings/donate/donate_screen.dart(1 hunks)lib/ui/settings/network/network_screen.dart(1 hunks)lib/ui/settings/profile/edit_profile_screen.dart(1 hunks)lib/ui/settings/profile_keys/profile_keys_screen.dart(1 hunks)rust/Cargo.toml(1 hunks)rust/src/api/accounts.rs(3 hunks)rust/src/frb_generated.rs(13 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/settings/network/network_screen.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/profile_keys/profile_keys_screen.dartlib/ui/settings/profile/edit_profile_screen.dartlib/ui/settings/developer/developer_settings_screen.dartlib/src/rust/api/accounts.dartlib/src/rust/frb_generated.dartlib/ui/settings/donate/donate_screen.dartlib/src/rust/frb_generated.io.dart
🧠 Learnings (5)
📓 Common learnings
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use flutter_rust_bridge to access core functionality of the app.
📚 Learning: 2025-09-07T02:15:31.931Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/settings/developer/developer_settings_screen.dartlib/src/rust/api/accounts.dartlib/src/rust/frb_generated.dartlib/src/rust/frb_generated.io.dartrust/src/frb_generated.rs
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/settings/developer/developer_settings_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
rust/Cargo.tomllib/src/rust/frb_generated.io.dartrust/src/frb_generated.rs
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/src/rust/frb_generated.io.dartrust/src/frb_generated.rs
🧬 Code graph analysis (2)
rust/src/api/accounts.rs (2)
rust/src/api/groups.rs (4)
from(30-47)from(96-102)from(115-120)from(132-136)rust/src/api/messages.rs (8)
from(78-100)from(104-106)from(110-137)from(141-143)from(147-177)from(181-183)from(187-223)from(227-229)
rust/src/frb_generated.rs (1)
rust/src/api/accounts.rs (4)
account_key_packages(199-207)delete_account_key_package(221-233)delete_account_key_packages(236-244)publish_account_key_package(210-218)
🔇 Additional comments (18)
lib/ui/settings/profile/edit_profile_screen.dart (1)
120-121: LGTM: explicit back icon sizing is consistent with the updated header pattern.lib/ui/settings/network/network_screen.dart (1)
145-155: LGTM: header spacing and explicit back icon sizing are consistent with other screens.rust/src/api/accounts.rs (5)
38-54: Timestamp conversion matches existing pattern; OK.Uses
Utc.timestamp_optwith sane fallback.
198-207: Bulk fetch API is straightforward; nice addition.
221-233: Delete single uses validated EventId; good.Return bool is useful for UI feedback.
236-244: Delete-all endpoint looks good; consistent with single delete semantics.
190-196: Return-type swap to FlutterEvent confirmed — Dart callers updated. Grep shows accountKeyPackage and callers use FlutterEvent (examples: lib/src/rust/api/accounts.dart, lib/ui/settings/developer/developer_settings_screen.dart).lib/src/rust/api/accounts.dart (2)
92-109: Bulk publish/delete endpoints wired correctly.Return types (
void,bool,BigInt) align with Rust ((),bool,usize) via FRB. No issues spotted.
160-197: ConfirmFlutterEvent.tagsshape.
tagsisList<String>here andVec<String>in Rust. If these represent Nostr tags, they’re typicallyList<List<String>>. If flattened intentionally for key packages, all good—otherwise we may needList<List<String>>.rust/src/frb_generated.rs (3)
164-200: FFI wiring for key package admin added correctly.New wire funcs for list/publish/delete(single/all) match accounts API and use consistent param decoding. No issues.
Also applies to: 626-704, 1679-1716
2638-2656: FlutterEvent codec coverage (single, vec, option) looks complete.SseDecode/Encode for
FlutterEvent,Vec<FlutterEvent>, andOption<FlutterEvent>are coherent and match Dart types.Also applies to: 2830-2842, 2991-3012, 4130-4141, 4279-4288, 4401-4420
2424-2435: Datetime codec uses micros; verify Dart side matches.Rust uses
timestamp_micros. Ensure Dart decoding also uses microsecond precision to avoid skew.lib/src/rust/frb_generated.dart (5)
490-2330: funcId realignment: ensure native side is regenerated in lockstep.Many funcId values shifted to accommodate new endpoints. Any Dart/Rust mismatch will crash at runtime.
If CI doesn’t already do it, add a quick integration step that calls a no-arg API (e.g., relay_type_inbox) to sanity-check symbol/ID alignment on boot.
906-937: BigInt return for delete_account_key_packages: surface contract check.UI/services must treat the count as BigInt (not int). Format safely when displaying, and avoid arithmetic narrowing.
2838-2841: Optional Chrono_Utc encode/decode paths touched.Logic appears unchanged; flagging only to confirm no semantic shift in null handling.
Also applies to: 3677-3684, 4463-4472
80-80: rustContentHash updated.LGTM; ensure rebuilt native libs are packaged for all targets (iOS/Android/macos).
97-104: Approve: Event → FlutterEvent migration verified.
Dart call sites and UI reference FlutterEvent; generated decoders/encoders exist; rust/src/api/accounts.rs defines pub struct FlutterEvent { id, pubkey, created_at (maps to createdAt), kind, tags, content } and impl From plus IntoDart/SseEncode/SseDecode are present.lib/src/rust/frb_generated.io.dart (1)
151-153: IO platform wiring updated to FlutterEvent — FRB v2.11.1 present; web binding missingpubspec.yaml contains flutter_rust_bridge: 2.11.1 (matches). frb_generated.web.dart not found — confirm whether web binding is intentionally omitted or add frb_generated.web.dart to preserve IO/web parity. Affected: lib/src/rust/frb_generated.io.dart (lines 151–153; also applies to 169–171, 211–213, 248–252, 412–416, 436–438, 480–484, 529–535, 716–720, 742–744, 796–801, 854–858, 860–864).
| (dialogContext) => WnDialog( | ||
| title: 'Delete Key Package', | ||
| content: | ||
| 'This will delete key package #${index + 1}. Other users won\'t be able to use this key package to invite you to new encrypted conversations. This action cannot be undone.', |
There was a problem hiding this comment.
Just asking: In what cases use case we would want to delete key packages from this screen? Or is just for debugging purposes? I thought that key packages were (magically?) updated from the rust side.
If other user already hast this key package and want's to start a chat with me, it will fail and the other user will have to search again for my latest key package? The other user has a way to know that the keypackage he's trying to use is deleted? Or should we check that in the frontend when starting a chat?
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
I feel this it going to be really helpful to debug the start chat issue 🚀 great idea!
Works amazing with this commit from whitenoise crate (507be72bb828f040bcb7317dbb573764d7f4fea3) I tried it with an existing account that had lots of key packages (60) and worked perfectly 💯
I'm just nos approving to wait to have the commit on master in the wn crate
There was a problem hiding this comment.
So I think something is now broken in the rust crate... I can't go beyond the spalsh screen in this branch in an ios emulator that already had data. In the rust logs I see that it's related to the signer and suscriptions, so I don't know it if is really related to your PR or @jgmontoya 's .
This a example of the logs I see when running just rust-logs in this branch
2025-09-11T16:38:32.348960Z ERROR nostr_relay_pool::relay::inner: Connection failed. url=wss://relay.kamp.site error=IO error: invalid peer certificate: certificate expired: verification time 1757608712 (UNIX), but certificate is not valid after 1750036177 (7572535 seconds ago) 2025-09-11T16:38:32.865193Z INFO nostr_relay_pool::relay::inner: Connected to 'wss://auth.nostr1.com' 2025-09-11T16:38:33.189267Z INFO nostr_relay_pool::relay::inner: Authenticated to relay. url=wss://auth.nostr1.com 2025-09-11T16:38:33.491425Z ERROR nostr_relay_pool::relay::inner: Impossible to handle relay message. url=wss://auth.nostr1.com msg=["OK","",true,""] error=Invalid event ID 2025-09-11T16:38:35.484009Z ERROR nostr_relay_pool::relay::inner: Connection failed. url=wss://nostr.massmux.com/ error=IO error: invalid peer certificate: certificate expired: verification time 1757608715 (UNIX), but certificate is not valid after 1756962921 (645794 seconds ago) 2025-09-11T16:38:37.364906Z INFO nostr_relay_pool::relay::inner: Connected to 'wss://wheat.happytavern.co/' 2025-09-11T16:38:37.756094Z ERROR nostr_relay_pool::relay::inner: Connection failed. url=wss://nostr.myowndamnnode.com error=IO error: invalid peer certificate: certificate expired: verification time 1757608717 (UNIX), but certificate is not valid after 1708976578 (48632139 seconds ago)
|
I see there's a new merge into whitenoise crate master branch, witll test with that one and see if it works |
nope, not working with c67c19f85968c7bc123a87944e17cb3da63ea00a rev either in ios emulator with data |
|
In devices with no data it works fine (tested in in my iPhone and in emulator) |
|
TLDR: which cases I tested that can pass the splashcreen (✅) or get stuck in it (❌ )
|
|
for notes, |
|
I took rev c67c19f85968c7bc123a87944e17cb3da63ea00 from wn crate master branch to test it in this branch. Worked better than the rev in this branch which was 0bff7e76f34901a1a610a187567929dccd3ebac7 when I tested |
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
✅ 💯 I pulled the changes now, tried it locally and 👍🏻
|
It is still slow the time in the splashcreen in ios emulator (in my iPhone 11 is much faster) but it doesn't have to do with this changes but with the rust crate 🦀 |
This adds key package admin to the developer settings screen to help users clean things up and publish a new key package manually if needed.
Summary by CodeRabbit
New Features
Style