fix(migrations): make release 1's migration set correct, and verify it against a Mainnet fork - #739
Open
jaxter03 wants to merge 2 commits into
Open
fix(migrations): make release 1's migration set correct, and verify it against a Mainnet fork#739jaxter03 wants to merge 2 commits into
jaxter03 wants to merge 2 commits into
Conversation
feat/port-pallets predates ddc-payouts#79, so this branch resolved the old revision and would have evaluated without the billing-report decode shim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FdHHPhppLJhGdBfySiJiJY
Six changes, each traceable to a dry run against a Mainnet fork. See docs/MAINNET_STORAGE_MIGRATION_DRYRUN.md and the evaluation page in the memory bank. 1. ddc-customers v4_mbm: remove the source ledger after copying. `ledgers_step` inserted into ClusterLedger and never removed from Ledger, so all 535 entries stayed under a prefix the post-migration runtime no longer declares -- orphaned where nothing reads them and no metadata-driven query can even see them. Confirmed by a raw twox128 prefix count: 535 keys left behind. 2. ddc-customers v4_mbm: a failed balance sweep must not advance the migration. `transfer_balance_step` logged the error and returned Finished regardless, bumping the storage version with 27,642 CERE of customer deposits still in the pallet account -- a migration reporting success having moved no money. It now returns Err(SteppedMigrationError::Failed). FailedMigrationHandler is FreezeChainOnFailedMigration, so this halts rather than proceeding on false state. A frozen chain is recoverable; silently misplaced deposits are not. 3. UnreleasedMultiblock: release 1 stops at customers v4. v5_mbm drains ClusterLedger into a per-cluster deposit contract. Mainnet has no contracts at all -- ContractInfoOf, CodeInfoOf and PristineCode are all empty -- and the address cannot be bound until a referendum sets it. Wired now it empties all 535 ledgers into nothing. v4 leaves storage coherent at version 4, so v5 ships once the contract is live. 4. Wire both migration lists into Executive and pallet_migrations. They were defined and referenced nowhere, so nothing ran. This also removes the need for the scratch patch every evaluation round has been applying, meaning the harness now tests the branch rather than a modified copy of it. 5. ddc-clusters v3: guard on `current_version >= 3`, not `== 3`. The pallet declares STORAGE_VERSION = 6, so equality against 3 can never hold. v3 silently no-oped, leaving the chain at version 2 while the runtime decoded its storage as 6, and stalling v4/v5/v6 which each guard on the previous version. v3 is the first link, so one comparison operator held the entire 2 -> 6 chain. 6. ddc-nodes: enable v0_v2. Mainnet's storage version key is absent, ie. 0, while the pallet declares 2. StorageNode and StorageNodeProps are unchanged between refs apart from an append-only StorageNodeMode variant, so v1 (added total_usage) and v2_mbm (removed it) net out on a chain that had neither. v0_v2 bumps 0 -> 2 and touches no data. Without it the version counter lies and the next migration guarding on `on_chain == 2` would silently skip. cargo check --workspace: clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FdHHPhppLJhGdBfySiJiJY
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.
Release 1's migration set was wired incorrectly and contained two data-loss
defects. This makes it correct, and verifies the result against a fork of
Mainnet at #26866694.
Stacked on #734 (
feat/port-pallets), which is still open — review that first.What was wrong
1.
ddc-customersv4 MBM copied without removing.ledgers_stepinsertedinto
ClusterLedgerbut never removed the source entry, stranding all 535Ledgerentries under the old prefix. Nothing reads them and nothing reportsthem — the migration looks complete and the count under the new key is right.
2. A failed balance transfer in
transfer_balance_stepwas swallowed. Itreturned the next state regardless, so a transfer failure advanced the state
machine and the migration reported success having moved no funds. It now
returns
Err(SteppedMigrationError::Failed), which the runtime'sFreezeChainOnFailedMigrationturns into a halt rather than silent loss.3.
UnreleasedMultiblockincluded customers v5. v5 belongs to release 2(it needs the deposit contract). Worse, v5 drains what v4 fills, so running
both left
ClusterLedgerempty. Scoped to v4 only.4. Neither list was wired.
UnreleasedandUnreleasedMultiblockexistedbut nothing referenced them, so no migration ran at all.
5.
ddc-clustersv3 guard was unsatisfiable.on_chain == 2 && current == 3can never hold — the pallet declares
STORAGE_VERSION = 6. Nowcurrent >= 3.6.
ddc-nodesv0→v2 was disabled. Mainnet'sDdcNodesversion key isabsent (0) while the pallet declares 2. v1 added
total_usageand v2 removedit, so they net out on a chain that had neither;
v0_v2bumps the counter andtouches no data. Without it the counter lies and the next version-guarded
migration silently skips.
Also pulls
ddc-payouts8178000cinto the lock, which carries the payoutsv1–v5 decode fixes.
v1::CustomerChargeand its successor differ by one fieldbut both encode to 167 bytes, so the old code decoded misaligned bytes cleanly
and inflated reward totals ~2.1e12x — no error, just wrong numbers.
Verification
Forked Mainnet at #26866694, applied the candidate runtime via
:code, droveblocks until the MBM cursor cleared. No Mainnet state was touched.
The MBM completes inside the upgrade block at 11.79% of block capacity, leaving
~88% for extrinsics. Earlier runs needed two blocks and peaked at 81.19%; the
drop is v5's removal, not an optimisation.
Balances::Holdsis asserted becauseRuntimeHoldReason's outer SCALE variantis the pallet index. Appending
DdcVerificationrather than inserting it keepsall 914 entries decodable — inserting would have silently zeroed 17.16M CERE
of holds, since the storage item is
ValueQueryand a failed decode returnsDefaultrather than erroring.Reproducing this requires both this branch and
feat/migration-eval-tool(#737) checked out together — the harness lives on that branch and the fixes
on this one, so neither alone can produce the result above:
Open question, not addressed here
runtime/cere-devhastype Migrations = ()for MBMs and aMigrationstuplecontaining only the two
RemovePallets — none of the DDC migrations above. Sodev/QA chains running
cere-devwill not apply them, and their DDC storageversion counters will disagree with what the pallets declare. That may be
deliberate (those chains may already be migrated via the staging line, or get
reset), but the port record does not say, and it is outside this PR's
Mainnet-release-1 scope. Flagging rather than guessing.
Out of scope
DdcVerificationand restoringUpgradeSessionKeys— release 3