Skip to content

Conversation

@jgmontoya
Copy link
Contributor

@jgmontoya jgmontoya commented Jan 22, 2026

This PR upgrades the workspace dependency nostr from 0.43 to 0.44 and updates timestamp usage across the codebase to the new Timestamp API, replacing deprecated Timestamp::as_u64() calls with Timestamp::as_secs() to use seconds-based representations. The change is an internal refactor of timestamp handling and storage serialization with no changes to public APIs or behavioral logic beyond timestamp units.

What changed:

  • Bumped nostr dependency in workspace Cargo.toml from "0.43" to "0.44".
  • mdk-core: Replaced Timestamp::as_u64() usages with as_secs() in message validation, timestamp comparisons, epoch snapshot code, error messages, and related tests.
  • mdk-sqlite-storage: Switched timestamp serialization for groups, messages, processed messages, and welcomes to as_secs() when persisting timestamps to the database.
  • mdk-uniffi: Updated FFI-facing conversions (Group and Message timestamp fields) to expose seconds via as_secs().
  • mdk-memory-storage: Adjusted internal timestamp handling to use as_secs() where applicable.
  • CHANGELOGs: Added entries documenting the nostr upgrade and the migration from as_u64() to as_secs().

Security impact:

  • No cryptographic, key handling, SQLCipher configuration, credential, or identity-binding changes were made; this PR only changes timestamp representations (no security-sensitive code modifications).

Protocol changes:

  • No changes to MLS implementation or MIPs; Nostr integration change limited to dependency upgrade and adapting to the nostr Timestamp API (use of seconds rather than deprecated method).

API surface:

  • No breaking changes to public function signatures, types, traits, or FFI boundaries were introduced; timestamp representation changes are internal and the UniFFI layer still exposes numeric timestamp fields (now in seconds).
  • Storage schema remains the same (same columns), but persisted timestamp values are now written as seconds.

Testing:

  • Tests and test helpers in mdk-core and related crates were updated to assert seconds-based timestamps (replacing prior as_u64-based expectations).

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

Replaced uses of Timestamp::as_u64() with Timestamp::as_secs() across core, storage, conversion, and tests, and bumped the workspace nostr dependency from 0.43 to 0.44.

Changes

Cohort / File(s) Summary
Dependency
Cargo.toml
Bumped nostr workspace dependency from 0.43 to 0.44.
Core logic & tests
crates/mdk-core/src/messages.rs, crates/mdk-core/src/test_util.rs
Replaced as_u64() with as_secs() in timestamp validation, epoch/commit handling, and test helpers; updated assertions and error messages to seconds.
Storage layer
crates/mdk-sqlite-storage/src/...
\crates/mdk-sqlite-storage/src/groups.rs`, `crates/mdk-sqlite-storage/src/messages.rs`, `crates/mdk-sqlite-storage/src/welcomes.rs``
Serialized timestamps (last_message_at, created_at, processed_at) using as_secs() instead of as_u64() in DB INSERT/UPDATE paths.
Public conversions
crates/mdk-uniffi/src/lib.rs
Conversions for public Group and Message types now use as_secs() for last_message_at / processed_at.
Changelogs
crates/mdk-core/CHANGELOG.md, crates/mdk-sqlite-storage/CHANGELOG.md, crates/mdk-uniffi/CHANGELOG.md
Added Unreleased entries documenting the nostr upgrade and replacement of as_u64() with as_secs().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

storage

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: upgrading the rust-nostr dependency from version 0.43 to 0.44, which is reflected throughout the changeset in Cargo.toml, timestamp handling updates, and changelog entries.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
No Sensitive Identifier Leakage ✅ Passed PR exclusively modifies timestamp handling via as_u64() to as_secs() conversion and nostr dependency bump; no error messages, logging, or Debug implementations expose sensitive identifiers.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

@github-actions
Copy link

✅ Coverage: 88.74% → 90.71% (+1.97%)

@jgmontoya jgmontoya force-pushed the chore/upgrade-rust-nostr branch from d853459 to 14ddf55 Compare January 22, 2026 21:21
@github-actions
Copy link

✅ Coverage: 88.74% → 90.71% (+1.97%)

Copy link

@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: 3

🤖 Fix all issues with AI agents
In `@crates/mdk-core/CHANGELOG.md`:
- Line 28: The CHANGELOG entry about upgrading the `nostr` dependency needs a PR
link appended; update the entry string ("Upgraded `nostr` dependency from 0.43
to 0.44, replacing deprecated `Timestamp::as_u64()` calls with
`Timestamp::as_secs()`") to include the PR reference in the required format
`([`#123`](https://github.com/marmot-protocol/mdk/pull/123))`, replacing 123 with
the actual PR number.

In `@crates/mdk-sqlite-storage/CHANGELOG.md`:
- Line 32: The changelog entry about upgrading the nostr dependency is missing
the required PR link; update the line mentioning "Upgraded `nostr` dependency
from 0.43 to 0.44, replacing deprecated `Timestamp::as_u64()` calls with
`Timestamp::as_secs()`" to append the PR reference in the standard format (e.g.,
add " (PR #<number>)" or the full PR URL) at the end of that entry so it
complies with CHANGELOG guidelines.

In `@crates/mdk-uniffi/CHANGELOG.md`:
- Line 34: The changelog entry mentioning the nostr dependency upgrade and
replacement of Timestamp::as_u64() with Timestamp::as_secs() is missing the
required PR link; update the entry in CHANGELOG.md (the line referencing "nostr"
and the method names Timestamp::as_u64() -> Timestamp::as_secs()) by appending
the corresponding PR URL (e.g., " (`#1234`)" or full link) to the end of the
sentence so it complies with the project's changelog guidelines.

@jgmontoya jgmontoya force-pushed the chore/upgrade-rust-nostr branch from 14ddf55 to 1f39347 Compare January 22, 2026 23:56
@github-actions
Copy link

✅ Coverage: 88.74% → 90.71% (+1.97%)

Copy link

@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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/mdk-uniffi/CHANGELOG.md (1)

32-41: Remove the duplicate empty ### Changed section.

Two ### Changed headings under ## Unreleased creates ambiguity and breaks the changelog structure. Keep a single ### Changed section. As per coding guidelines, Keep a Changelog structure should be consistent.

🧹 Proposed fix
@@
 ### Changed
 
 - Upgraded `nostr` dependency from 0.43 to 0.44, replacing deprecated `Timestamp::as_u64()` calls with `Timestamp::as_secs()` ([`#162`](https://github.com/marmot-protocol/mdk/pull/162))
 - Changed `get_messages()` to accept optional `limit` and `offset` parameters for pagination control. Existing calls must be updated to pass `None, None` for default behavior (limit: 1000, offset: 0), or specify values for custom pagination. ([`#111`](https://github.com/marmot-protocol/mdk/pull/111))
 - Changed `get_pending_welcomes()` to accept optional `limit` and `offset` parameters for pagination control. Existing calls must be updated to pass `None, None` for default behavior (limit: 1000, offset: 0), or specify values for custom pagination. ([`#119`](https://github.com/marmot-protocol/mdk/pull/119))
 - Changed `new_mdk()`, `new_mdk_with_key()`, and `new_mdk_unencrypted()` to accept an optional `MdkConfig` parameter for customizing MDK behavior. Existing calls must be updated to pass `None` for default behavior. ([`#155`](https://github.com/marmot-protocol/mdk/pull/155))
-
-### Changed

@jgmontoya jgmontoya force-pushed the chore/upgrade-rust-nostr branch from 1f39347 to 196eafc Compare January 23, 2026 01:38
@github-actions
Copy link

✅ Coverage: 88.74% → 90.71% (+1.97%)

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.

2 participants