fix(deps): update Platform to 63cf57f with database compatibility - #981
Conversation
Pin Platform v4.2-dev at e3cd7cf (4.2.0-dev.8) and adapt wallet APIs. Automatically bridge the previous PR's database lineage for both app and network stores with retained backups, strict staged loading and atomic reconstruction. Preserve live identity metadata during ownership promotion. Handle swept transaction events and enable the passing Max-send regression. Document retained functionality, remaining upstream gaps and audit results. Validation: cargo fmt --all; 2520 library tests passed; exact CI Clippy flags --all-features --all-targets -- -D warnings passed. Co-Authored-By: Codex <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Include the pending contact-crypto persistence and identity-removal serialization fix. Migration history and schema remain unchanged; retain the verified database compatibility bridge and other lockfile resolutions. Validation: formatter, 2520 library tests, and CI Clippy flags passed with the locked dependency graph. Co-Authored-By: Codex <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
|
Warning Review limit reachedNext included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (35)
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 |
|
🕓 Queued for automated review — 12th in line, estimated start in ~2.2 h (commit 4f5fc04)
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 2 only (queue backlog)
The compatibility migration is carefully scoped and includes durable backups, staged validation, rollback coverage, and identity metadata preservation. One in-scope architecture issue remains: validation failures erase their concrete error types behind a blanket trait object, weakening the typed error chain used by the application.
🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
- Triage:
criticalbygpt-6-astra(effort low) — The large, intricate diff directly changes storage migration and compatibility logic in src/wallet_backend/platform_compatibility/engine.rs and related context/database paths, a critical storage-migration surface that can affect preservation and recovery of wallet, identity, and preference data. - Phase 1 reviewers: not run (skipped for throughput: 12 PRs queued, above the 10 limit)
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/wallet_backend/platform_compatibility/engine.rs`:
- [SUGGESTION] src/wallet_backend/platform_compatibility/engine.rs:39-42: Preserve concrete validation error types instead of boxing dyn Error
`TypedValidation` stores validation failures as `Box<dyn std::error::Error + Send + Sync>`. The caller in `platform_compatibility/mod.rs` wraps failures from staging the database, loading wallet data, and loading unowned identities into this variant, so downstream code can no longer structurally distinguish the originating storage or persistence error. This conflicts with the repository's typed-error convention and makes diagnostics and future error handling depend on downcasting a blanket trait object. Introduce a dedicated validation error enum with concrete source variants for each validation operation, then use that enum as the source of `UpgradeError::TypedValidation` while retaining the existing user-facing message.
| "The updated application could not read your wallet data. Keep your data folder and reopen the previous application version." | ||
| )] | ||
| TypedValidation(#[source] Box<dyn std::error::Error + Send + Sync>), | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Preserve concrete validation error types instead of boxing dyn Error
TypedValidation stores validation failures as Box<dyn std::error::Error + Send + Sync>. The caller in platform_compatibility/mod.rs wraps failures from staging the database, loading wallet data, and loading unowned identities into this variant, so downstream code can no longer structurally distinguish the originating storage or persistence error. This conflicts with the repository's typed-error convention and makes diagnostics and future error handling depend on downcasting a blanket trait object. Introduce a dedicated validation error enum with concrete source variants for each validation operation, then use that enum as the source of UpgradeError::TypedValidation while retaining the existing user-facing message.
source: gpt-6-astra (phase2-reviewer: rust-quality)
Control the SPV startup future in the cancellation test and separately verify registration remains queued after a fast network switch. Preserve the production network-switch behavior. Wait for the masternode removal result before checking rendered cards, and register the password-prompt fixture after storage preparation. Validation: 2521 library tests, 329 UI tests, 11 E2E tests and 7 doctests passed; formatter and Clippy passed. Network-dependent tests remain ignored. Co-Authored-By: Codex <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
2 participants
TL;DR: Update Platform while keeping existing wallets, identities and preferences readable, with retained backups for recovery.
User story
As a wallet user, I want to upgrade without losing my saved data or having to recreate my profile.
Scenario
Base flow
Open the updated application with a profile created by the previously pinned development version.
Actual behavior
A direct dependency update cannot open the old databases. An identity ownership change can also delete saved identity metadata.
Expected behavior
The application upgrades recognized old databases automatically, verifies their contents before committing, and preserves live identity metadata.
Detailed discussion
What was done
63cf57f40d0000bf3b2b26026c8fa1c71162852d(4.2.0-dev.8), including the pending contact-crypto/identity-removal serialization fix in fix(platform-wallet): serialize pending crypto writes with identity removal platform#4649.e3cd7cf; the existing destination schema guard remains valid.Testing
cargo fmt --allcargo clippy --locked --all-features --all-targets -- -D warnings: passed.Remaining limitations
The contact-account scan-generation fix from dashpay/platform#4587 remains absent at this pin. The old FFI proof-size gate from dashpay/platform#4585 is also absent, though DET uses the Rust API. Hosts with memory pages larger than 16 KiB are unsupported by the new secret storage. Live-network and live-desktop GUI checks were not run; headless UI integration tests passed.
The dependency audit found the same five pre-existing advisories before and after the upgrade. See the compatibility review for details.
Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent