Skip to content

refactor: removed all the machinery for database encryption#131

Merged
grunch merged 3 commits intomainfrom
remove-crypto
Mar 18, 2026
Merged

refactor: removed all the machinery for database encryption#131
grunch merged 3 commits intomainfrom
remove-crypto

Conversation

@arkanoider
Copy link
Collaborator

@arkanoider arkanoider commented Mar 12, 2026

@grunch ,

this is the pr for the complete removal of the crypto code needed for database encryption.
This is a BREAKING change that will be completed with a pr of mostro mirroring this one.

Summary by CodeRabbit

  • Refactor

    • Removed the cryptographic utilities module and simplified privacy-related APIs to remove password-based access.
  • Documentation

    • Updated project structure and testing guidelines to reflect the new module organization and broader edge-case coverage.
  • Chores

    • Version bumped to 0.7.1.
    • Removed several cryptography-related dependencies.

@arkanoider arkanoider requested a review from grunch March 12, 2026 13:40
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9e50fc6e-4a7a-4489-9146-f578c0e34dd3

📥 Commits

Reviewing files that changed from the base of the PR and between 6158524 and ac6fd54.

📒 Files selected for processing (1)
  • Cargo.toml

Walkthrough

Removed the crypto utility module and its public exports, deleted several crypto-related dependencies, and refactored order APIs to return PublicKey without password-based decryption; minor docs and inline comment edits accompany the change.

Changes

Cohort / File(s) Summary
Documentation & Comments
AGENTS.md, src/message.rs
Removed references to crypto helpers in project docs and adjusted inline comments for RestoredDisputeHelper fields.
Module & Dependency Removal
Cargo.toml, src/lib.rs, src/prelude.rs, src/crypto.rs
Deleted src/crypto.rs (all crypto utilities), removed crypto-related dependencies from Cargo.toml (e.g., argon2, chacha20poly1305, secrecy, blake3), removed pub mod crypto and prelude re-export; bumped package version.
API Refactor
src/order.rs
Changed get_master_buyer_pubkey/get_master_seller_pubkey to return PublicKey (removed password param) and updated is_full_privacy_order to remove SecretString usage and compare keys via string conversion.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Database columns encryption #102: Removes the crypto module, exports, dependencies, and password-based Order APIs — appears to be the inverse or closely related change.

Suggested reviewers

  • grunch
  • Catrya

Poem

🐇 I hopped through modules, chewed the argon vine,
Tossed chacha and blake3 from the burrow line,
Keys now stand plain as PublicKey light,
No whispered secrets in the moonlit night,
A joyful thump — code clean and bright! 🌱

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removal of database encryption machinery, which is reflected consistently across all modified files (removal of crypto.rs, crypto dependencies, related API changes).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-crypto
📝 Coding Plan
  • Generate coding plan for human review comments

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.

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

🧹 Nitpick comments (1)
Cargo.toml (1)

62-63: Minor: extra blank line after dependency removal.

There's an extra blank line after base64 (where the crypto dependencies were removed). Consider removing it for cleaner formatting.

Suggested fix
 base64 = "0.22.1"
-

 
 [features]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` around lines 62 - 63, Remove the stray blank line following the
base64 dependency entry in Cargo.toml; edit the section containing the base64 =
"0.22.1" line to eliminate the extra empty line so dependencies are
consecutively formatted and the file has no unintended blank gap after the
base64 entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Cargo.toml`:
- Line 3: The crate version in Cargo.toml incorrectly uses a patch bump
("version" = "0.7.1") for a breaking change; update the version field to a minor
bump "0.8.0" to reflect the removal of crypto APIs and communicate the breaking
change to consumers (edit the version = "..." line in Cargo.toml).

---

Nitpick comments:
In `@Cargo.toml`:
- Around line 62-63: Remove the stray blank line following the base64 dependency
entry in Cargo.toml; edit the section containing the base64 = "0.22.1" line to
eliminate the extra empty line so dependencies are consecutively formatted and
the file has no unintended blank gap after the base64 entry.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 38c9fcf5-6a80-4ba0-9d65-e1dbad7e5e7b

📥 Commits

Reviewing files that changed from the base of the PR and between 69080c0 and 6158524.

📒 Files selected for processing (7)
  • AGENTS.md
  • Cargo.toml
  • src/crypto.rs
  • src/lib.rs
  • src/message.rs
  • src/order.rs
  • src/prelude.rs
💤 Files with no reviewable changes (3)
  • src/prelude.rs
  • src/lib.rs
  • src/crypto.rs

[package]
name = "mostro-core"
version = "0.7.0"
version = "0.7.1"
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider a minor version bump for this breaking change.

The PR description declares this as a BREAKING change (removing crypto APIs), but the version is bumped from 0.7.0 to 0.7.1 (patch). Per semver, even in 0.x.y versions, breaking changes are typically signaled by bumping the minor version (0.7.0 → 0.8.0) to alert consumers of incompatible API changes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 3, The crate version in Cargo.toml incorrectly uses a
patch bump ("version" = "0.7.1") for a breaking change; update the version field
to a minor bump "0.8.0" to reflect the removal of crypto APIs and communicate
the breaking change to consumers (edit the version = "..." line in Cargo.toml).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will evaluate this with @grunch

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

LGTM

@grunch grunch merged commit f289ee6 into main Mar 18, 2026
11 checks passed
@grunch grunch deleted the remove-crypto branch March 18, 2026 18: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.

2 participants