-
Notifications
You must be signed in to change notification settings - Fork 56
feat(platform-wallet): rebuild tracked asset locks after restore; honest scan-derived shielded history #4342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6557d9b
84b7f89
716165b
03b8613
a6cfb13
9df80e3
3f28a74
0a2031a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| //! Widen the `asset_locks.status` CHECK domain with | ||
| //! `recovered_from_chain` — the status the restore-scan reconstruction | ||
| //! assigns to asset locks rebuilt from chain-locked on-chain records | ||
| //! (Core finality proven, Platform-side consumption unknown). | ||
| //! | ||
| //! SQLite cannot alter a CHECK constraint in place, so this rebuilds | ||
| //! the table: create the widened twin, copy every row, drop the old | ||
| //! table, rename. `asset_locks` is a leaf table (it references | ||
| //! `wallet_metadata`; nothing references it), so the drop/rename is | ||
| //! safe under `PRAGMA foreign_keys = ON`, and the copied rows satisfy | ||
| //! the re-declared FK because they came from a table with the same | ||
| //! constraint. | ||
| //! | ||
| //! The status list below is FROZEN — like V001's, it must never track | ||
| //! the live `ASSET_LOCK_STATUS_LABELS` const, or a future variant | ||
| //! addition would rewrite this migration's generated SQL and break its | ||
| //! Refinery checksum on databases that already applied it. The | ||
| //! `asset_lock_status_labels_frozen_in_latest_migration` unit test in | ||
| //! `sqlite::schema::asset_locks` pins the live const to this list so a | ||
| //! new variant fails compilation of intent loudly: append V005+ with | ||
| //! another rebuild, never edit this file. | ||
|
|
||
| pub fn migration() -> String { | ||
| "\ | ||
| CREATE TABLE asset_locks_v4 ( | ||
| wallet_id BLOB NOT NULL, | ||
| outpoint BLOB NOT NULL, | ||
| status TEXT NOT NULL CHECK (status IN ('built', 'broadcast', 'is_locked', 'chain_locked', 'consumed', 'recovered_from_chain')), | ||
| account_index INTEGER NOT NULL, | ||
| identity_index INTEGER NOT NULL, | ||
| amount_duffs INTEGER NOT NULL, | ||
| lifecycle_blob BLOB NOT NULL, | ||
| PRIMARY KEY (wallet_id, outpoint), | ||
| FOREIGN KEY (wallet_id) REFERENCES wallet_metadata(wallet_id) ON DELETE CASCADE | ||
| ); | ||
|
|
||
| -- Orphan policy: a row whose wallet was deleted while FK enforcement | ||
| -- happened to be off is unreachable garbage (every read path keys | ||
| -- through wallet_metadata), but copying it into the FK-declared twin | ||
| -- under PRAGMA foreign_keys = ON would abort this whole migration with | ||
| -- 'FOREIGN KEY constraint failed'. Drop such rows explicitly — the | ||
| -- same outcome the declared ON DELETE CASCADE would have produced had | ||
| -- enforcement been on when the wallet was deleted. | ||
| DELETE FROM asset_locks | ||
| WHERE wallet_id NOT IN (SELECT wallet_id FROM wallet_metadata); | ||
|
|
||
| INSERT INTO asset_locks_v4 (wallet_id, outpoint, status, account_index, identity_index, amount_duffs, lifecycle_blob) | ||
| SELECT wallet_id, outpoint, status, account_index, identity_index, amount_duffs, lifecycle_blob FROM asset_locks; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| DROP TABLE asset_locks; | ||
|
|
||
| ALTER TABLE asset_locks_v4 RENAME TO asset_locks;" | ||
| .to_string() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,25 +66,38 @@ pub fn apply( | |
| Ok(()) | ||
| } | ||
|
|
||
| /// Single source of truth for the `asset_locks.status` TEXT-column | ||
| /// domain. | ||
| /// Test-only drift guard for the `asset_locks.status` TEXT-column | ||
| /// domain **as the writer sees it** (production code never reads this | ||
| /// — the writer maps through [`status_str`] and the on-disk CHECK | ||
| /// lives frozen inside the migrations). | ||
| /// | ||
| /// Mirrors every variant of | ||
| /// [`platform_wallet::wallet::asset_lock::tracked::AssetLockStatus`] | ||
| /// (writer side: [`status_str`]). The migration in | ||
| /// `migrations/V001__initial.rs` interpolates this array into the | ||
| /// `CHECK (status IN (...))` clause so an unknown label is rejected at | ||
| /// insert time rather than landing as silent garbage. The | ||
| /// `asset_lock_status_labels_match_enum` unit test below enforces | ||
| /// set-equality between this array and the writer's output — drift (a | ||
| /// renamed/added variant) becomes a failing test, not a runtime | ||
| /// divergence between Rust and SQLite. | ||
| /// (writer side: [`status_str`]). The on-disk `CHECK (status IN (...))` | ||
| /// clause rejects an unknown label at insert time rather than letting | ||
| /// it land as silent garbage — but the migrations do NOT interpolate | ||
| /// this const: each migration freezes its own copy of the domain, | ||
| /// because a generated-SQL change breaks that migration's Refinery | ||
| /// checksum on every database that already applied it | ||
| /// (`abort_divergent` default). `V001__initial.rs` carries the original | ||
| /// five labels; `V004__asset_lock_recovered_status.rs` rebuilt the | ||
| /// table with the current six. | ||
| /// | ||
| /// Two unit tests below keep the three copies honest: | ||
| /// - `asset_lock_status_labels_match_enum` — this array ⇔ the writer's | ||
| /// codomain ([`status_str`]); | ||
| /// - `asset_lock_status_labels_frozen_in_latest_migration` — this array | ||
| /// ⇔ the latest migration's frozen list, so ADDING a variant fails | ||
| /// with instructions to append a new table-rebuild migration (V005+) | ||
| /// instead of editing a shipped one. | ||
| #[cfg(test)] | ||
| pub(crate) const ASSET_LOCK_STATUS_LABELS: &[&str] = &[ | ||
| "built", | ||
| "broadcast", | ||
| "is_locked", | ||
| "chain_locked", | ||
| "consumed", | ||
| "recovered_from_chain", | ||
|
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
97
to
+100
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking: Append a migration instead of changing V001's generated CHECK constraint
source: ['codex']
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in a6cfb13: V001 is frozen back to its original five-label CHECK (byte-identical generated SQL, so existing databases' Refinery checksums verify), and the domain widens via an appended |
||
| ]; | ||
|
|
||
| fn status_str(s: &AssetLockStatus) -> &'static str { | ||
|
|
@@ -94,6 +107,7 @@ fn status_str(s: &AssetLockStatus) -> &'static str { | |
| AssetLockStatus::InstantSendLocked => "is_locked", | ||
| AssetLockStatus::ChainLocked => "chain_locked", | ||
| AssetLockStatus::Consumed => "consumed", | ||
| AssetLockStatus::RecoveredFromChain => "recovered_from_chain", | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -198,14 +212,16 @@ mod tests { | |
| AssetLockStatus::InstantSendLocked, | ||
| AssetLockStatus::ChainLocked, | ||
| AssetLockStatus::Consumed, | ||
| AssetLockStatus::RecoveredFromChain, | ||
| ]; | ||
| for v in &variants { | ||
| match v { | ||
| AssetLockStatus::Built | ||
| | AssetLockStatus::Broadcast | ||
| | AssetLockStatus::InstantSendLocked | ||
| | AssetLockStatus::ChainLocked | ||
| | AssetLockStatus::Consumed => {} | ||
| | AssetLockStatus::Consumed | ||
| | AssetLockStatus::RecoveredFromChain => {} | ||
| } | ||
| } | ||
| variants | ||
|
|
@@ -224,4 +240,28 @@ mod tests { | |
| from_const, from_writer | ||
| ); | ||
| } | ||
|
|
||
| /// Pins the live label set to the domain frozen in the LATEST | ||
| /// asset-lock migration (`V004__asset_lock_recovered_status.rs`). | ||
| /// Shipped migrations interpolate nothing — their generated SQL is | ||
| /// checksummed by Refinery, so widening the domain means APPENDING | ||
| /// a new table-rebuild migration (V005+) with the new frozen list | ||
| /// and updating this pin, never editing V001/V004 in place. | ||
| /// | ||
| /// IF THIS FAILS: do NOT edit a shipped migration (its Refinery | ||
| /// checksum would diverge on already-migrated databases). Append a | ||
| /// new migration that rebuilds `asset_locks` with the widened | ||
| /// CHECK, then update this pin to the new migration's list. | ||
| #[test] | ||
| fn asset_lock_status_labels_frozen_in_latest_migration() { | ||
| let frozen_in_v004 = [ | ||
| "built", | ||
| "broadcast", | ||
| "is_locked", | ||
| "chain_locked", | ||
| "consumed", | ||
| "recovered_from_chain", | ||
| ]; | ||
| assert_eq!(ASSET_LOCK_STATUS_LABELS, &frozen_in_v004); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.