refactor(db): schema v28 — drop obsolete sync columns and clean up DB interface#653
Merged
lklimek merged 1 commit intoFeb 24, 2026
Conversation
… interface - Bump DEFAULT_DB_VERSION 27 → 28 - Drop last_terminal_block from wallet table (unused after sync simplification) - Drop last_full_sync_balance from platform_address_balances table - Rename last_platform_sync_checkpoint → last_platform_sync_height - Add runtime SQLite ≥3.35 check (required for DROP COLUMN) - Idempotent migration: checks column existence before each ALTER - Remove unused _is_sync_operation param from set_platform_address_info() - Remove set_platform_address_info_from_sync() wrapper - Fix redundant let-mut rebinding in fetch_platform_address_balances Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
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)
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 |
lklimek
force-pushed
the
fix/db-migration-v28-cleanup
branch
from
e6af495 to
1d1383b
Compare
lklimek
merged commit 65f2946
into
zk-extract/platform-sync-simplification
4 of 6 checks passed
lklimek
added a commit
that referenced
this pull request
Feb 25, 2026
…rmSyncMode (#635) * refactor(wallet): simplify platform sync by removing PlatformSyncMode Remove the PlatformSyncMode enum (Auto/ForceFull/TerminalOnly) and terminal sync logic (apply_recent_balance_changes, last_terminal_block, last_full_sync_balance). The SDK now handles incremental sync internally via AddressProvider::current_balances() and last_sync_height(). Key changes: - Remove PlatformSyncMode enum from backend_task::wallet - Simplify fetch_platform_address_balances to use new SDK API with stored state (with_stored_state, current_balances, last_sync_height) - Change CoreTask::RefreshWalletInfo to use bool instead of Option<PlatformSyncMode> - Remove last_full_sync_balance from PlatformAddressInfo - Simplify database sync info to 2-tuple (timestamp, height) - Remove set_last_terminal_block from database - Simplify RefreshMode enum (remove PlatformFull, PlatformTerminal, CoreAndPlatformFull, CoreAndPlatformTerminal variants) Note: requires updated dash-sdk with new sync_address_balances API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: update platform SDK to rev 0fa82e6652 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add manual test scenarios for platform sync simplification Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): address PR #635 audit findings and extract broadcast helper - DB schema v28: drop obsolete columns (last_terminal_block, last_full_sync_balance), rename last_platform_sync_checkpoint → last_platform_sync_height, with SQLite ≥3.35 runtime check - Store asset lock TX before broadcast to prevent SPV InstantSend race - Defer UTXO removal until after successful broadcast - Replace .unwrap() on RwLock with .map_err() to avoid panics - Remove unused _is_sync_operation param and set_platform_address_info_from_sync wrapper - Fix redundant let-mut rebinding in fetch_platform_address_balances - Extract broadcast_and_commit_asset_lock() on AppContext to consolidate the store→broadcast→cleanup→UTXO-removal pattern from 5 code paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(db): schema v28 — drop obsolete sync columns and clean up DB interface (#653) - Bump DEFAULT_DB_VERSION 27 → 28 - Drop last_terminal_block from wallet table (unused after sync simplification) - Drop last_full_sync_balance from platform_address_balances table - Rename last_platform_sync_checkpoint → last_platform_sync_height - Add runtime SQLite ≥3.35 check (required for DROP COLUMN) - Idempotent migration: checks column existence before each ALTER - Remove unused _is_sync_operation param from set_platform_address_info() - Remove set_platform_address_info_from_sync() wrapper - Fix redundant let-mut rebinding in fetch_platform_address_balances Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * revert(db): remove schema v28 migration from this branch The v28 schema changes (drop obsolete sync columns, rename last_platform_sync_checkpoint → last_platform_sync_height) will be applied separately and should not ship on this branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review comments — remove dead code and document risks - Remove dead `_is_sync_operation` parameter from `Database::set_platform_address_info` and all call sites (Finding #6) - Add doc comment on `set_platform_sync_info` explaining column name drift: `last_platform_sync_checkpoint` now stores SDK sync height (Finding #4) - Remove trivial `set_platform_address_info_from_sync` delegate and update callers to use `set_platform_address_info` directly (Finding #7) - Combine unnecessary `let provider` + `let mut provider = provider` rebinding into single `let mut` block (Finding #10) - Document UTXO selection race window on `broadcast_and_commit_asset_lock`: std::sync::RwLock guard is !Send so it cannot span async broadcast Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: apply nightly fmt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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
DEFAULT_DB_VERSION27 → 28last_terminal_blockfrom wallet table (unused after sync simplification)last_full_sync_balancefromplatform_address_balancestablelast_platform_sync_checkpoint→last_platform_sync_heightto match new semanticsDROP COLUMN); clear error message if too oldALTER(safe to re-run)_is_sync_operationparam fromset_platform_address_info()set_platform_address_info_from_sync()trivial wrapperlet mutrebinding infetch_platform_address_balancesTest plan
cargo test --all-features --workspacepassescargo clippy --all-features --all-targets -- -D warningsis clean🤖 Generated with Claude Code
🤖 Co-authored by Claudius the Magnificent AI Agent