Skip to content

Conversation

@dannym-arx
Copy link
Contributor

@dannym-arx dannym-arx commented Feb 2, 2026

This PR adds a processed_at timestamp to messages and enforces deterministic message ordering to fix timing-related inconsistencies (e.g., when created_at ties or devices have clock skew). It updates core message handling, memory and SQLite storage, storage traits, and UniFFI bindings so messages include processing time and are consistently ordered by created_at, processed_at, then id.

What changed:

  • Added processed_at: Timestamp to the public Message struct and threaded it through mdk-core, mdk-memory-storage, mdk-sqlite-storage, and mdk-storage-traits.
  • mdk-core now sets processed_at when creating/processing messages to record local reception time.
  • mdk-memory-storage and mdk-sqlite-storage now sort messages by created_at DESC, processed_at DESC, id DESC to preserve reception order and ensure determinism.
  • mdk-sqlite-storage includes a V002 migration adding a processed_at column, backfilling existing rows with created_at, and creating a composite index idx_messages_sorting to support the new ORDER BY.
  • mdk-uniffi exposes processed_at as a u64 in the FFI Message type so foreign-language bindings receive the new field.
  • Documentation and changelogs updated across affected crates to describe the new timestamp and ordering rationale.

Security impact:

  • None identified; no changes to cryptography, key handling, SQLCipher configuration, or sensitive error messages.

Protocol changes:

  • No MLS protocol or cryptographic protocol changes; this is a client-side ordering/metadata change that helps mitigate clock-skew effects when displaying messages.
  • Minor Nostr-related timestamp propagation: created_at remains the event timestamp while processed_at captures local processing time.

API surface:

  • Breaking change: Message struct in mdk-storage-traits (and therefore public storage APIs) now requires processed_at: Timestamp, so all constructors of Message must supply this field.
  • GroupStorage::messages() ordering semantics changed to created_at DESC, processed_at DESC, id DESC (behavioral change, same signature).
  • Storage schema change: SQLite migration V002 adds processed_at column and composite index.
  • UniFFI/FFI change: public Message now includes processed_at: u64.

Testing:

  • Tests across crates were updated to use deterministic timestamps (shared now variables) and to initialize/validate processed_at, reducing nondeterminism in test data and aligning expectations with the new ordering.

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

📝 Walkthrough

Walkthrough

Adds a processed_at timestamp to Message structs across core, storage traits, implementations, and UniFFI; updates storage queries and SQLite migration to order messages by created_at DESC, processed_at DESC, id DESC and backfills processed_at from created_at.

Changes

Cohort / File(s) Summary
Core message handling
crates/mdk-core/src/messages/create.rs, crates/mdk-core/src/messages/application.rs, crates/mdk-core/src/messages/process.rs
Populate processed_at when creating/processing messages; tests use a shared timestamp variable.
Public API / Storage traits
crates/mdk-storage-traits/src/messages/types.rs, crates/mdk-storage-traits/src/groups/mod.rs, crates/mdk-storage-traits/src/test_utils.rs, crates/mdk-storage-traits/tests/shared/mod.rs
Add processed_at: Timestamp to public Message type; update docs to state ordering created_at DESC, processed_at DESC, id DESC; test helpers set processed_at.
Memory storage
crates/mdk-memory-storage/src/groups.rs, crates/mdk-memory-storage/src/messages.rs, crates/mdk-memory-storage/src/lib.rs
Change message sorting to created_at DESC, processed_at DESC, id DESC; tests refactored to reuse a single timestamp for related fields.
SQLite schema & migration
crates/mdk-sqlite-storage/migrations/V002__add_processed_at_to_messages.sql
Add processed_at INTEGER column, backfill from created_at, and create composite index idx_messages_sorting with (mls_group_id, created_at DESC, processed_at DESC, id DESC).
SQLite storage implementation
crates/mdk-sqlite-storage/src/db.rs, crates/mdk-sqlite-storage/src/groups.rs, crates/mdk-sqlite-storage/src/messages.rs
Read/write processed_at in queries and row mapping; fallback processed_at = created_at for legacy rows; update ORDER BY to created_at DESC, processed_at DESC, id DESC; persist processed_at in INSERT/UPDATE.
UniFFI bindings
crates/mdk-uniffi/src/lib.rs
Expose new processed_at: u64 in the public UniFFI Message struct and map it from internal processed_at.
Changelogs
crates/mdk-core/CHANGELOG.md, crates/mdk-memory-storage/CHANGELOG.md, crates/mdk-sqlite-storage/CHANGELOG.md, crates/mdk-storage-traits/CHANGELOG.md, crates/mdk-uniffi/CHANGELOG.md
Document the added processed_at field, deterministic ordering change to created_at DESC, processed_at DESC, id DESC, and the SQLite migration/index.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

breaking-change, storage

Suggested reviewers

  • erskingardner
  • jgmontoya
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix timing issues' is overly vague and does not convey the specific technical changes made (introducing processed_at timestamps and updating message sorting logic). Consider a more descriptive title such as 'Add message processed_at timestamp and update sorting logic' to clearly communicate the main changes to reviewers.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
No Sensitive Identifier Leakage ✅ Passed No sensitive identifiers found in logging, error messages, or debug implementations across modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/message-timing-issues-157

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Feb 2, 2026

✅ Coverage: 90.77% → 90.83% (+0.06%)

@github-actions
Copy link

github-actions bot commented Feb 2, 2026

⚠️ Coverage: 90.79% → 90.79% (0.00%)

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