[DO NOT MERGE] make post-migration check happy transforming failing assertions in to log errors#926
Closed
sigurpol wants to merge 10 commits into
Conversation
… and AH post-migration
…t not match the AH value when None from RC
kianenigma
reviewed
Sep 24, 2025
| pallet_rc_migrator::RcMigrationStage::<RcRuntime>::get(), | ||
| pallet_rc_migrator::MigrationStage::MigrationDone | ||
| ); | ||
| // assert_eq!( |
Contributor
There was a problem hiding this comment.
yeah this one was stuck at MigrationSignal or similar, probably because our snapshot, for whatever reason, is still not taken at the right time :(
Contributor
There was a problem hiding this comment.
I tested locally with the snaps from this run and works as expected.
kianenigma
reviewed
Sep 24, 2025
| rc_pre_translated, ah_messages, | ||
| "Bags list data mismatch: Asset Hub data differs from original Relay Chain data" | ||
| ); | ||
| if rc_pre_translated != ah_messages { |
Contributor
There was a problem hiding this comment.
This one maybe you can actually debug because it is close to home :)
I think the on-idle hooks is messing with us.
Contributor
Author
There was a problem hiding this comment.
I have verified that we are preserving the bag structure 1:1 with the difference being only in the score.
Now if I am not mistaken, we are actually calculating the score (see https://github.com/paritytech/polkadot-sdk/blob/master/substrate/primitives/staking/src/currency_to_vote.rs#L73) typically like this:
score = active stake / total issuance
What we do in ah-migrator/src/staking/bags_list.rs is to preserve the original RC score to maintain the same bag list structure post-migration.
During migration, total issuance changes between RC and AH ( RC -> include all accounts, AH -> some accounts might be filtered out e.g. if they have total balance < ED and not only these, see https://github.com/sigurpol/runtimes/blob/948252980fabe9f90c4121f6dd0b38a4f79b090d/pallets/rc-migrator/src/accounts.rs#L1224-L1230).
This means that any subsequent operation that triggers score recalculation (like a rebag) will use the new total issuance, causing the score to update ( will be higher since total issuance is lower on AH).
bags-list's on_idle() performs a limited number of automatic rebag per block. But we use MaxAutoRebagPerBlock = RebagIffMigrationDone which is 0 during migration so no rebag via on_idle() during migration!
I need to verify what other kind of operations allowed during migration might cause a score recalculation.
Contributor
Author
|
Some interesting logs in the test.log I've attached to the PR: balances issue �[2m2025-09-24T14:10:33.171742Z�[0m �[31mERROR�[0m �[2mruntime::rc-migrator�[0m�[2m:�[0m RC accounts post-check: Change on total issuance on the relay chain after migration is not as expected. Total issuance: 20400495729870217, Expected (rc_total_before - migrated): 17081906260466912616, RC total before: 17081906260466912616, Migrated: 0
�[2m2025-09-24T14:10:33.171777Z�[0m �[31mERROR�[0m �[2mruntime::rc-migrator�[0m�[2m:�[0m RC accounts post-check: Kept balance on the relay chain after migration is not as expected. Total issuance: 20400495729870217, Kept: 0
�[2m2025-09-24T14:10:35.893821Z�[0m �[31mERROR�[0m �[2mruntime::ah-migrator�[0m�[2m:�[0m Total balance mismatch for account "5EYCAe5cKPAoFh2HnQQvpKqRYZGqBpaA87u4Zzw89qPE58is" between RC pre-migration and AH post-migration. RC migrated: 1000000000000, AH migrated: 0, Difference: 1000000000000, AH ED: 3333333
�[2m2025-09-24T14:10:37.606075Z�[0m �[31mERROR�[0m �[2mruntime::ah-migrator�[0m�[2m:�[0m Total balance mismatch for account "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY" between RC pre-migration and AH post-migration. RC migrated: 10000000000000, AH migrated: 9999542244355, Difference: 457755645, AH ED: 3333333 �bags list issue (Kian suggests me to look into this one ☝️, will do!) Couple of failed to decode RC calls I don't know how to interpret Some defensive failures �[2m2025-09-24T14:10:43.237600Z�[0m �[31mERROR�[0m �[2mruntime::defensive�[0m�[2m:�[0m a defensive failure has been triggered; please report the block number at https://github.com/paritytech/polkadot-sdk/issues: "Storage entry must exist"
�[2m2025-09-24T14:10:43.238059Z�[0m �[31mERROR�[0m �[2mruntime::defensive�[0m�[2m:�[0m a defensive failure has been triggered; please report the block number at https://github.com/paritytech/polkadot-sdk/issues: "Storage entry must exist"
�[2m2025-09-24T14:10:43.238491Z�[0m �[31mERROR�[0m �[2mruntime::defensive�[0m�[2m:�[0m a defensive failure has been triggered; please report the block number at https://github.com/paritytech/polkadot-sdk/issues: "Storage entry must exist" Nomination pools (here I didn't have to do anything kianenigma , no assertion commented out ) �[2m2025-09-24T14:10:43.752532Z�[0m �[31mERROR�[0m �[2mruntime::frame-support�[0m�[2m:�[0m ❌ "NominationPools" try_state checks failed: Other("Some pools do not have correct ED frozen") |
ggwpez
reviewed
Sep 24, 2025
| ah.sort_by_encoded(); | ||
|
|
||
| for (i, (r, a)) in rc.iter().zip(ah.iter()).enumerate() { | ||
| assert_eq!(r, a, "Entry #{i} mismatch: {r:?} != {a:?}"); |
Contributor
Author
There was a problem hiding this comment.
will double-check tomorrow, still haven't investigated!
Member
There was a problem hiding this comment.
Maybe block authorship rewards
sigurpol
force-pushed
the
make_post_migration_checks_happy
branch
from
da31c9f to
ca5fc73
Compare
sigurpol
force-pushed
the
make_post_migration_checks_happy
branch
from
ca5fc73 to
fb625e7
Compare
… zero nonces" This reverts commit fb625e7.
Contributor
Author
|
Retested with current latest main (c2a76dac06d838299cb3af387d1ba0c7b2f66b3d) and snapshots from local build. To make it pass now I commented out the following assertion in the following files: modified: pallets/ah-migrator/src/account.rs
modified: pallets/ah-migrator/src/scheduler.rs
modified: pallets/ah-migrator/src/staking/bags_list.rs
modified: pallets/ah-migrator/src/staking/checks.rs
modified: pallets/rc-migrator/src/scheduler.rsdiff --git a/pallets/ah-migrator/src/account.rs b/pallets/ah-migrator/src/account.rs
index f59d2494d..228375bd9 100644
--- a/pallets/ah-migrator/src/account.rs
+++ b/pallets/ah-migrator/src/account.rs
@@ -379,11 +379,11 @@ pub mod tests {
// Therefore, we just check that the difference between the balance migrated from
// the RC to AH and the balance delta on AH before and after migration is less than
// AH existential deposit.
- assert!(
- rc_migrated_balance.saturating_sub(ah_migrated_balance) < ah_ed,
- "Total balance mismatch for account {:?} between RC pre-migration and AH post-migration",
- who.to_ss58check()
- );
+ // assert!(
+ // rc_migrated_balance.saturating_sub(ah_migrated_balance) < ah_ed,
+ // "Total balance mismatch for account {:?} between RC pre-migration and AH
+ // post-migration", who.to_ss58check()
+ // );
// There are several `unreserve` operations on AH after migration (e.g., unreserve
// deposits for multisigs because they are not migrated to AH, adjust deposits for
@@ -391,7 +391,7 @@ pub mod tests {
// AH after migration is less than the migrated reserved balance from RC.
assert!(
ah_reserved_post.saturating_sub(ah_reserved_before) <= summary.migrated_reserved,
- "Change in reserved balance on AH after migration for account {:?} is greater than the migrated reserved balance from RC",
+ "Change in reserved balance on AH after migration for account {:?} is greater than the migrated reserved balance from RC",
who.to_ss58check()
);
diff --git a/pallets/ah-migrator/src/scheduler.rs b/pallets/ah-migrator/src/scheduler.rs
index 528e99975..93b6fb8da 100644
--- a/pallets/ah-migrator/src/scheduler.rs
+++ b/pallets/ah-migrator/src/scheduler.rs
@@ -195,11 +195,11 @@ impl<T: Config> crate::types::AhMigrationCheck for SchedulerMigrator<T> {
// Assert storage 'Scheduler::IncompleteSince::ah_post::correct'
if rc_payload.incomplete_since.is_some() {
- assert_eq!(
- pallet_scheduler::IncompleteSince::<T>::get(),
- rc_payload.incomplete_since,
- "IncompleteSince on Asset Hub should match the RC value"
- );
+ // assert_eq!(
+ // pallet_scheduler::IncompleteSince::<T>::get(),
+ // rc_payload.incomplete_since,
+ // "IncompleteSince on Asset Hub should match the RC value"
+ // );
} else {
assert_eq!(
pallet_scheduler::IncompleteSince::<T>::get(),
diff --git a/pallets/ah-migrator/src/staking/bags_list.rs b/pallets/ah-migrator/src/staking/bags_list.rs
index 042761d8a..684753536 100644
--- a/pallets/ah-migrator/src/staking/bags_list.rs
+++ b/pallets/ah-migrator/src/staking/bags_list.rs
@@ -203,10 +203,10 @@ impl<T: Config> crate::types::AhMigrationCheck for BagsListMigrator<T> {
// Assert storage "VoterList::ListNodes::ah_post::consistent"
// Assert storage "VoterList::ListBags::ah_post::correct"
// Assert storage "VoterList::ListBags::ah_post::consistent"
- assert_eq!(
- rc_pre_translated, ah_messages,
- "Bags list data mismatch: Asset Hub data differs from original Relay Chain data"
- );
+ // assert_eq!(
+ // rc_pre_translated, ah_messages,
+ // "Bags list data mismatch: Asset Hub data differs from original Relay Chain data"
+ // );
// Run bags-list pallet integrity check
#[cfg(feature = "try-runtime")]
diff --git a/pallets/ah-migrator/src/staking/checks.rs b/pallets/ah-migrator/src/staking/checks.rs
index 762df4220..2eede6aed 100644
--- a/pallets/ah-migrator/src/staking/checks.rs
+++ b/pallets/ah-migrator/src/staking/checks.rs
@@ -214,6 +214,6 @@ fn assert_equal_items<
ah.sort_by_encoded();
for (i, (r, a)) in rc.iter().zip(ah.iter()).enumerate() {
- assert_eq!(r, a, "Entry #{i} mismatch: {r:?} != {a:?}");
+ // assert_eq!(r, a, "Entry #{i} mismatch: {r:?} != {a:?}");
}
}
diff --git a/pallets/rc-migrator/src/scheduler.rs b/pallets/rc-migrator/src/scheduler.rs
index 581965b7f..4e8b092fa 100644
--- a/pallets/rc-migrator/src/scheduler.rs
+++ b/pallets/rc-migrator/src/scheduler.rs
@@ -378,10 +378,10 @@ impl<T: Config> crate::types::RcMigrationCheck for SchedulerMigrator<T> {
fn post_check(_rc_pre_payload: Self::RcPrePayload) {
// Assert storage 'Scheduler::IncompleteSince::rc_post::empty'
- assert!(
- pallet_scheduler::IncompleteSince::<T>::get().is_none(),
- "IncompleteSince should be None on RC after migration"
- );
+ // assert!(
+ // pallet_scheduler::IncompleteSince::<T>::get().is_none(),
+ // "IncompleteSince should be None on RC after migration"
+ // );
// Assert storage 'Scheduler::Agenda::rc_post::empty'
assert!( NOTE for Nomination pools try-state* 2025-09-26T10:11:05.228866Z INFO runtime::frame-support: 🩺 Running "NominationPools" try-state checks
2025-09-26T10:11:05.229601Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 196: 0 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.229637Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 54: 3 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.229774Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 37: 1 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230045Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 188: 3 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230219Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 198: 16 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230264Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 202: 0 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230341Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 192: 0 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230413Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 132: 18 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230546Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 119: 0 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230699Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 159: 0 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230743Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 69: 2 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230885Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 46: 2 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.230930Z WARN runtime::nomination-pools: [11008326] 🏊♂️ reward pool of 174: 0 (ed = 3333333), should only happen because ED has changed recently. Pool operators should be notified to top up the reward account
2025-09-26T10:11:05.328431Z ERROR runtime::frame-support: ❌ "NominationPools" try_state checks failed: Other("Some pools do not have correct ED frozen") @kianenigma are we fine with that? As discussed already, we haven't backported SDK #9415 to 2507, are we fine with that for KAHM? |
Contributor
Author
|
Rust tests outcome vs latest snapshots https://github.com/paritytech/ahm-dryrun/actions/runs/18082107836 with 1.9 release: RUST_LOG=runtime::ah-migrator=debug,runtime::rc-migrator=debug,proxy_check=debug,remote-ext=info,runtime=warn,runtime=info SNAP_RC_PRE="/Users/paolo/github/ahm-dryrun/ci-bite/kusama-rc-pre.snap" SNAP_AH_PRE="/Users/paolo/github/ahm-dryrun/ci-bite/kusama-ah-pre.snap" SNAP_RC_POST="/Users/paolo/github/ahm-dryrun/ci-bite/kusama-rc-post.snap" SNAP_AH_POST="/Users/paolo/github/ahm-dryrun/ci-bite/kusama-ah-post.snap" cargo test -p polkadot-integration-tests-ahm --release --features kusama-ahm --features try-runtime post_migration_checks_only -- --include-ignored --nocapture --test-threads 1We need to comment out the following to make test pass, same result as #926 (comment) diff --git a/pallets/ah-migrator/src/account.rs b/pallets/ah-migrator/src/account.rs
index f59d2494d..228375bd9 100644
--- a/pallets/ah-migrator/src/account.rs
+++ b/pallets/ah-migrator/src/account.rs
@@ -379,11 +379,11 @@ pub mod tests {
// Therefore, we just check that the difference between the balance migrated from
// the RC to AH and the balance delta on AH before and after migration is less than
// AH existential deposit.
- assert!(
- rc_migrated_balance.saturating_sub(ah_migrated_balance) < ah_ed,
- "Total balance mismatch for account {:?} between RC pre-migration and AH post-migration",
- who.to_ss58check()
- );
+ // assert!(
+ // rc_migrated_balance.saturating_sub(ah_migrated_balance) < ah_ed,
+ // "Total balance mismatch for account {:?} between RC pre-migration and AH
+ // post-migration", who.to_ss58check()
+ // );
// There are several `unreserve` operations on AH after migration (e.g., unreserve
// deposits for multisigs because they are not migrated to AH, adjust deposits for
@@ -391,7 +391,7 @@ pub mod tests {
// AH after migration is less than the migrated reserved balance from RC.
assert!(
ah_reserved_post.saturating_sub(ah_reserved_before) <= summary.migrated_reserved,
- "Change in reserved balance on AH after migration for account {:?} is greater than the migrated reserved balance from RC",
+ "Change in reserved balance on AH after migration for account {:?} is greater than the migrated reserved balance from RC",
who.to_ss58check()
);
diff --git a/pallets/ah-migrator/src/scheduler.rs b/pallets/ah-migrator/src/scheduler.rs
index 528e99975..93b6fb8da 100644
--- a/pallets/ah-migrator/src/scheduler.rs
+++ b/pallets/ah-migrator/src/scheduler.rs
@@ -195,11 +195,11 @@ impl<T: Config> crate::types::AhMigrationCheck for SchedulerMigrator<T> {
// Assert storage 'Scheduler::IncompleteSince::ah_post::correct'
if rc_payload.incomplete_since.is_some() {
- assert_eq!(
- pallet_scheduler::IncompleteSince::<T>::get(),
- rc_payload.incomplete_since,
- "IncompleteSince on Asset Hub should match the RC value"
- );
+ // assert_eq!(
+ // pallet_scheduler::IncompleteSince::<T>::get(),
+ // rc_payload.incomplete_since,
+ // "IncompleteSince on Asset Hub should match the RC value"
+ // );
} else {
assert_eq!(
pallet_scheduler::IncompleteSince::<T>::get(),
diff --git a/pallets/ah-migrator/src/staking/bags_list.rs b/pallets/ah-migrator/src/staking/bags_list.rs
index 042761d8a..684753536 100644
--- a/pallets/ah-migrator/src/staking/bags_list.rs
+++ b/pallets/ah-migrator/src/staking/bags_list.rs
@@ -203,10 +203,10 @@ impl<T: Config> crate::types::AhMigrationCheck for BagsListMigrator<T> {
// Assert storage "VoterList::ListNodes::ah_post::consistent"
// Assert storage "VoterList::ListBags::ah_post::correct"
// Assert storage "VoterList::ListBags::ah_post::consistent"
- assert_eq!(
- rc_pre_translated, ah_messages,
- "Bags list data mismatch: Asset Hub data differs from original Relay Chain data"
- );
+ // assert_eq!(
+ // rc_pre_translated, ah_messages,
+ // "Bags list data mismatch: Asset Hub data differs from original Relay Chain data"
+ // );
// Run bags-list pallet integrity check
#[cfg(feature = "try-runtime")]
diff --git a/pallets/ah-migrator/src/staking/checks.rs b/pallets/ah-migrator/src/staking/checks.rs
index 762df4220..2eede6aed 100644
--- a/pallets/ah-migrator/src/staking/checks.rs
+++ b/pallets/ah-migrator/src/staking/checks.rs
@@ -214,6 +214,6 @@ fn assert_equal_items<
ah.sort_by_encoded();
for (i, (r, a)) in rc.iter().zip(ah.iter()).enumerate() {
- assert_eq!(r, a, "Entry #{i} mismatch: {r:?} != {a:?}");
+ // assert_eq!(r, a, "Entry #{i} mismatch: {r:?} != {a:?}");
}
}
diff --git a/pallets/rc-migrator/src/accounts.rs b/pallets/rc-migrator/src/accounts.rs
index efc176303..edf891bd0 100644
--- a/pallets/rc-migrator/src/accounts.rs
+++ b/pallets/rc-migrator/src/accounts.rs
@@ -1293,11 +1293,11 @@ pub mod tests {
let total_issuance = <T as Config>::Currency::total_issuance();
let tracker = RcMigratedBalanceArchive::<T>::get();
- assert_eq!(
- total_issuance,
- rc_total_issuance_before.saturating_sub(tracker.migrated),
- "Change on total issuance on the relay chain after migration is not as expected"
- );
+ // assert_eq!(
+ // total_issuance,
+ // rc_total_issuance_before.saturating_sub(tracker.migrated),
+ // "Change on total issuance on the relay chain after migration is not as expected"
+ // );
assert_eq!(
total_issuance, tracker.kept,
"Kept balance on the relay chain after migration is not as expected"@kianenigma : I've backported paritytech/polkadot-sdk#9453 to avoid false positive / noise from NominationPools when we will test PAHM vs 2507. |
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
4 participants
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.
Transform all assertions that cause the post-migration check to fail during the AHM dry run into log errors.
Command used to test:
test.log