Skip to content

contracts: Fix incorrect storage alias in mirgration #1687

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.

Sign up for GitHub

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

Merged
pgherveou merged 3 commits into from
Sep 25, 2023
Merged

contracts: Fix incorrect storage alias in mirgration #1687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be5f6a4
contracts: Fix incorrect storage alias in mirgration
kvinwang Sep 25, 2023
c72b767
Add integrity check
pgherveou Sep 25, 2023
db9bc87
Added integrity check
pgherveou Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions substrate/frame/contracts/src/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,32 @@ impl<T: Config, const TEST_ALL_STEPS: bool> OnRuntimeUpgrade for Migration<T, TE
);
Ok(Default::default())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), TryRuntimeError> {
if !TEST_ALL_STEPS {
return Ok(())
}

log::info!(target: LOG_TARGET, "=== POST UPGRADE CHECKS ===");

// Ensure that the hashing algorithm is correct for each storage map.
if let Some(hash) = crate::CodeInfoOf::<T>::iter_keys().next() {
Copy link
Copy Markdown
Contributor Author

@kvinwang kvinwang Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the iter_keys().next() returns an incorrect hash or just returns None in case of a hash algorithm mismatching?

Copy link
Copy Markdown
Contributor

@pgherveou pgherveou Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would return an incorrect hash since keys have been hashed with Twox64Concat during the migration

crate::CodeInfoOf::<T>::get(hash).expect("CodeInfo exists for hash; qed");
}
if let Some(hash) = crate::PristineCode::<T>::iter_keys().next() {
crate::PristineCode::<T>::get(hash).expect("PristineCode exists for hash; qed");
}
if let Some(account_id) = crate::ContractInfoOf::<T>::iter_keys().next() {
crate::ContractInfoOf::<T>::get(account_id)
.expect("ContractInfo exists for account_id; qed");
}
if let Some(nonce) = crate::DeletionQueue::<T>::iter_keys().next() {
crate::DeletionQueue::<T>::get(nonce).expect("DeletionQueue exists for nonce; qed");
}

Ok(())
}
}

/// The result of running the migration.
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/contracts/src/migration/v12.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ where

#[storage_alias]
pub type CodeInfoOf<T: Config, OldCurrency> =
StorageMap<Pallet<T>, Twox64Concat, CodeHash<T>, CodeInfo<T, OldCurrency>>;
StorageMap<Pallet<T>, Identity, CodeHash<T>, CodeInfo<T, OldCurrency>>;

#[storage_alias]
pub type PristineCode<T: Config> = StorageMap<Pallet<T>, Identity, CodeHash<T>, Vec<u8>>;
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/contracts/src/migration/v14.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ mod old {

#[storage_alias]
pub type CodeInfoOf<T: Config, OldCurrency> =
StorageMap<Pallet<T>, Twox64Concat, CodeHash<T>, CodeInfo<T, OldCurrency>>;
StorageMap<Pallet<T>, Identity, CodeHash<T>, CodeInfo<T, OldCurrency>>;
}

#[cfg(feature = "runtime-benchmarks")]
Expand Down