Skip to content

Conversation

@erskingardner
Copy link
Member

@erskingardner erskingardner commented Oct 2, 2025

This updates the rust edition to the latest 2024. This should have no effect on the app.

Summary by CodeRabbit

  • New Features
    • Added support for group images, including secure metadata fields to enable setting, syncing, and validating group avatars.
  • Improvements
    • Enhanced data handling to reliably transmit optional image information across platforms.
  • Chores
    • Updated underlying toolchain to the latest Rust edition for future compatibility and stability.
  • Refactor
    • Minor internal cleanups to maintain consistency (no user-facing changes).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

Adds optional image fields (key, hash, nonce) to FlutterGroupDataUpdate across Dart and Rust, updates constructors, equality/hashCode, and FRB encoding/decoding for DCO/SSE paths, introduces U8Array12 optional (de)serializers, propagates fields in Rust conversions, and switches Rust edition to 2024.

Changes

Cohort / File(s) Summary of edits
Dart API: Group update model
lib/src/rust/api/groups.dart
Added optional fields imageKey (U8Array32?), imageHash (U8Array32?), imageNonce (U8Array12?); updated constructor, ==, and hashCode.
Dart FRB bridge (gen)
lib/src/rust/frb_generated.dart
Extended DCO/SSE (de)serialization for FlutterGroupDataUpdate to include imageKey/imageHash/imageNonce; added opt U8Array12 encoders/decoders; adjusted DCO array length (4→7).
Dart FRB platform decls
lib/src/rust/frb_generated.io.dart
Declared new protected methods: dco_decode_opt_u_8_array_12, sse_decode_opt_u_8_array_12, sse_encode_opt_u_8_array_12.
Rust API: groups
rust/src/api/groups.rs
Added Option<[u8;32]> image_key/image_hash and Option<[u8;12]> image_nonce to FlutterGroupDataUpdate; propagated to NostrGroupDataUpdate.
Rust FRB bridge (gen)
rust/src/frb_generated.rs
Included new fields in sse_decode/sse_encode/IntoDart; added Option<[u8;12]> (de)serialization support; minor import reordering.
Rust workspace config
rust/Cargo.toml
Changed edition from 2021 to 2024.
Rust API: users
rust/src/api/users.rs
Reordered imports; no functional changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dart as Dart (Flutter)
  participant FRB_D as FRB Dart
  participant FRB_R as FRB Rust
  participant Rust as Rust API

  Note over Dart,FRB_R: Create/receive FlutterGroupDataUpdate with imageKey/imageHash/imageNonce (optional)

  Dart->>FRB_D: Encode FlutterGroupDataUpdate (SSE/DCO)
  FRB_D->>FRB_R: Transmit fields incl. imageKey/hash/nonce
  FRB_R->>Rust: sse_decode → FlutterGroupDataUpdate { image_* }
  Rust->>Rust: Convert to NostrGroupDataUpdate (propagate image_*)

  Rust-->>FRB_R: sse_encode FlutterGroupDataUpdate with image_*
  FRB_R-->>FRB_D: Return serialized data
  FRB_D-->>Dart: Decode incl. imageKey/hash/nonce
  Note over Dart: Fields available in Dart model
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • Quwaysim
  • codeswot
  • josefinalliende
  • untreu2

Poem

A whisk of bytes, a nonce so fine,
I stash the key, the hash aligns.
From Dart to Rust I hop with glee,
Seven fields now dance with three.
Bridge rebuilt, editions new—
Carrots compiled, and tests hop through. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run `@coderabbitai generate docstrings` to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit's high-level summary is enabled.
Title Check ✅ Passed The title succinctly references both key updates—the Rust edition bump to 2024 and the addition of GroupData image fields—clearly reflecting the two main aspects of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch edition-2024

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 and usage tips.

josefinalliende
josefinalliende previously approved these changes Oct 2, 2025
Copy link
Contributor

@josefinalliende josefinalliende left a comment

Choose a reason for hiding this comment

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

✅ Built it for ios and tried it in my iPhone. Everything seems to be working fine!

@erskingardner erskingardner changed the title Edition 2024 Edition 2024 & GroupData image fields Oct 3, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b02b7c and 36997fb.

⛔ Files ignored due to path filters (1)
  • ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • lib/src/rust/api/groups.dart (2 hunks)
  • lib/src/rust/frb_generated.dart (6 hunks)
  • lib/src/rust/frb_generated.io.dart (3 hunks)
  • rust/Cargo.toml (1 hunks)
  • rust/src/api/groups.rs (1 hunks)
  • rust/src/api/users.rs (1 hunks)
  • rust/src/frb_generated.rs (7 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/src/rust/frb_generated.io.dart
  • lib/src/rust/frb_generated.dart
  • lib/src/rust/api/groups.dart
🧠 Learnings (2)
📚 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:

  • rust/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 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/src/frb_generated.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Flutter CI
🔇 Additional comments (5)
rust/Cargo.toml (1)

4-4: Confirm toolchain is on a Rust release that supports edition 2024.
Edition 2024 requires at least the toolchain that shipped it (Rust 1.85+). Please double-check CI, build scripts, and developer setup to ensure they’re already on that release; otherwise this change will break builds.

lib/src/rust/frb_generated.dart (4)

2754-2767: LGTM! DCO decoder correctly handles the new image fields.

The decoder properly:

  • Updates the array length check to 7 to accommodate the three new fields
  • Decodes imageKey, imageHash (32-byte arrays), and imageNonce (12-byte array) as optional fields
  • Maintains field order consistency with the SSE decoder

2991-2995: LGTM! New U8Array12 optional encoder/decoder functions are correctly implemented.

The three new helper functions (DCO decoder, SSE decoder, SSE encoder) for opt_u_8_array_12 follow the established pattern for optional array handling and are consistent with the existing opt_u_8_array_32 implementations.

Also applies to: 3913-3922, 4770-4778


3520-3541: LGTM! SSE decoder correctly handles the new image fields.

The SSE decoder properly:

  • Decodes all three new fields (imageKey, imageHash, imageNonce) in the correct order
  • Maintains consistency with the DCO decoder field order
  • Uses appropriate optional decoders for the nullable array types

4445-4458: LGTM! SSE encoder correctly serializes the new image fields.

The SSE encoder properly:

  • Encodes all three new fields in the correct order
  • Maintains field order consistency with both decoders
  • Uses appropriate optional encoders matching the decoder types

The encode/decode symmetry is maintained across DCO and SSE codecs.

Comment on lines +57 to 72
pub image_key: Option<[u8; 32]>,
pub image_hash: Option<[u8; 32]>,
pub image_nonce: Option<[u8; 12]>,
}

impl From<FlutterGroupDataUpdate> for NostrGroupDataUpdate {
fn from(group_data: FlutterGroupDataUpdate) -> Self {
Self {
name: group_data.name,
description: group_data.description,
image_key: None, // TODO: Support image updates
image_hash: None, // TODO: Support image updates
image_nonce: None, // TODO: Support image updates
// Wrap in Some() to convert Option<T> to Option<Option<T>>
// None means don't update, Some(value) means set to value
image_key: group_data.image_key.map(Some),
image_hash: group_data.image_hash.map(Some),
image_nonce: group_data.image_nonce.map(Some),
// Will silently drop invalid relay inputs
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Cannot clear image fields with the current mapping.
FlutterGroupDataUpdate only carries Option<[u8; _]>, and the .map(Some) here collapses “set to null” into “don’t touch”. There is no way for Flutter to emit Some(None) for NostrGroupDataUpdate, so once an image key/hash/nonce is set it can never be cleared via this API. Please extend the DTO to carry a real tri-state (e.g. Option<Option<_>> or an explicit enum) so we can send Some(None) when the user wants to remove the image.

🤖 Prompt for AI Agents
In rust/src/api/groups.rs around lines 57 to 72, the current conversion maps
FlutterGroupDataUpdate's Option<[u8; _]> to NostrGroupDataUpdate using
.map(Some), which prevents sending a "clear" instruction (Some(None)) and makes
image fields impossible to unset; update the DTO to carry a tri-state (change
FlutterGroupDataUpdate fields to Option<Option<[u8; N]>> or an explicit enum) so
the outer Option represents "update vs no-op" and the inner Option represents
"set value vs clear", and adjust the From impl to map the new tri-state fields
directly into NostrGroupDataUpdate without collapsing None into no-op (respect
outer Some(None) as a clear).

@erskingardner erskingardner merged commit 8c399c4 into master Oct 3, 2025
2 checks passed
@erskingardner erskingardner deleted the edition-2024 branch October 3, 2025 05:56
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.

3 participants