CSPP Phase 2: Database encryption & migration#597
Draft
praveenperera wants to merge 12 commits intomasterfrom
Draft
CSPP Phase 2: Database encryption & migration#597praveenperera wants to merge 12 commits intomasterfrom
praveenperera wants to merge 12 commits intomasterfrom
Conversation
- Fix compile error: use master_key.sensitive_data_key() instead of cspp.sensitive_data_key() - Add open_or_create_database() helper that handles encrypted, plaintext, and new database files - Simplify database.rs and wallet_data.rs to use the new helper - Add backward compatibility for legacy plaintext databases
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
…validation - Add .bak-only recovery path for interrupted migrations where only backup remains - Replace silent `let _ = remove_file()` with logged error handling across all migration files - Fix SQL injection via single-quote in file paths during BDK migration - Switch EncryptedBackend to parking_lot::Mutex, promote write_disk_block assert to release - Add partial-block truncation detection in read_disk_block - Change open_or_create_database to return Result, propagate errors through wallet_data - Replace .flatten() on directory iterators with explicit error logging - Add post-migration verification (reopen + read check) before atomic swap - Improve panic messages at app startup for migration failures - Fix set_encryption_key doc comment, add sync_data eventual=true explanation - Fix WalletDataDb::new_test to return TempDir so it survives test lifetime
Replace io::Result with DatabaseError in open_or_create_database and
propagate through Database::init instead of .expect()ing at every step.
Add EncryptionKeyNotSet, BackendOpen, and CorruptBlock variants to
distinguish error categories. Improve migration panic messages in
App::new with path/phase context and full error chains via {e:#}.
Restrict test-only helpers to test builds: remove the test-only reset_cache from the generic Cspp impl and add a Cspp<MockStore>::reset_cache inside the tests module so it is only available for tests. Also relocate WalletDataDb::new_test into a #[cfg(test)] impl block at the bottom of the file to avoid exposing test helper code in non-test builds. These changes keep production APIs clean of test utilities.
Use FileExt::read_exact_at/write_all_at instead of seek+read/write, eliminating the need for a mutex to protect the shared file position. Also add create_or_open to handle TOCTOU race, and fix sync_data write barrier to always fsync.
875a192 to
1785828
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EncryptedBackend(XChaCha20-Poly1305 block-level AEAD)Commits
EncryptedBackendimplementingredb::StorageBackendwith per-block encryption, block-index AAD, and COVE magic headeropen_or_create_database()helper with backward compatibility for plaintext legacy DBscopy_tablehelper, main DB migration (7 tables), per-wallet migration (5 tables), crash recovery via atomic renamerusqlitewithbundled-sqlcipher-vendored-openssl, apply PRAGMA key inBdkStore::try_new()sqlcipher_exportbased migration, crash recovery, wired into app startupStartup sequence (after all changes)
CSPP Phase 2 progress
Closes #589, closes #591, closes #592, closes #590, closes #559, closes #560
Remaining phase 2 issue: #561 (cloud backup config flags to GlobalConfig)
Test plan
cargo test -p cove— 111 tests pass (encrypted backend, migration, BDK migration, crash recovery)just fmt && just clippy— clean