contracts: Fix incorrect storage alias in mirgration #1687
Conversation
There was a problem hiding this comment.
This might break everyone else
Looks like you might have deployed a sha in between these 2 commits where the regression was introduced
introduced here: paritytech/substrate#14661
rolled back here paritytech/substrate#14079
Let's discuss to find the best way to fix that
-- my bad fix looks good I was looking at the wrong storage
There was a problem hiding this comment.
Thanks!
I am uncertain about the proper approach for adding tests to this change. Would the team consider taking over this PR to bring it to completion?
This is an interesting question. try-runtime testing of the migrations currently can't catch this. We should think on how to improve this.
agryaznov
added
R0-silent
pgherveou
force-pushed
the
fix-code-info
branch
from
58ab603
to
d579920
Compare
pgherveou
force-pushed
the
fix-code-info
branch
from
d579920
to
c72b767
Compare
Added some integrity check in the last commit, to ensure that storage map items are encoded with the proper hashing algorithm |
|
@pgherveou could you backport to the |
|
sure let met push a PR for this |
# Description We are recently trying to upgrade our Substrate version from `polkadot-v0.9.43` to `polkadot-v1.0.0` and we noticed a critical issue: all deployed contracts seem to be experiencing a `CodeNotFound` error. After a thorough investigation, it appears that the root cause of this issue lies in the mismatch between the storage alias of `CodeInfoOf<T>` in the migration and its original definition. This PR corrects the storage alias to align it with its original definition I am uncertain about the proper approach for adding tests to this change. Would the team consider taking over this PR to bring it to completion? --------- Co-authored-by: pgherveou <pgherveou@gmail.com>
|
@jasl made a PR here, waiting for the release team to approve and merge it |
Thank you! |
| 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() { |
There was a problem hiding this comment.
Would the iter_keys().next() returns an incorrect hash or just returns None in case of a hash algorithm mismatching?
There was a problem hiding this comment.
it would return an incorrect hash since keys have been hashed with Twox64Concat during the migration
* master: (61 commits) OpenGov in Westend and Rococo (#1177) Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720) Migrate polkadot-primitives to v6 (#1543) genesis-builder: implemented for all runtimes (#1492) `BlockId` removal: `tx-pool` refactor (#1678) Bump directories from 4.0.1 to 5.0.1 (#1656) Allow debug_assertions in short-benchmarks CI job (#1711) chainHead/storage: Fix storage iteration using the query key (#1665) Implement more useful traits in `Slot` type (#1595) Make downloads in parallel and give more time to complete (#1699) Bump actions/checkout from 4.0.0 to 4.1.0 (#1688) contracts: Fix incorrect storage alias in mirgration (#1687) Fix documentation about justification and `finalized == true` requirement (#1607) tweak pallet macro (genesis_config etc) to cater for RA users as well. (#1689) Uncoupling pallet-xcm from frame-system's RuntimeCall (#1684) Bump aes-gcm from 0.10.2 to 0.10.3 (#1681) docs / Update PR template to reflect monorepo (#1674) update contributing guide and ui-tests scripts (#1668) pallet epm: add `TrimmingStatus` to the mined solution (#1659) Update HRMP pallet benchmarking to use benchmarks v2 (#1676) ...
* tsv-disabling-node-side: (69 commits) runtime-api: cleanup after v7 stabilization (#1729) Move requests-responses and polling from `ChainSync` to `SyncingEngine` (#1650) Add custom error message for `StorageNoopGuard` (#1727) Clarify docs cargo fmt add a CAVEAT comment implement disabled_validators correctly remove unnecessary hash string (#1722) OpenGov in Westend and Rococo (#1177) Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720) Migrate polkadot-primitives to v6 (#1543) genesis-builder: implemented for all runtimes (#1492) `BlockId` removal: `tx-pool` refactor (#1678) Bump directories from 4.0.1 to 5.0.1 (#1656) Allow debug_assertions in short-benchmarks CI job (#1711) chainHead/storage: Fix storage iteration using the query key (#1665) Implement more useful traits in `Slot` type (#1595) Make downloads in parallel and give more time to complete (#1699) Bump actions/checkout from 4.0.0 to 4.1.0 (#1688) contracts: Fix incorrect storage alias in mirgration (#1687) ...
# Description We are recently trying to upgrade our Substrate version from `polkadot-v0.9.43` to `polkadot-v1.0.0` and we noticed a critical issue: all deployed contracts seem to be experiencing a `CodeNotFound` error. After a thorough investigation, it appears that the root cause of this issue lies in the mismatch between the storage alias of `CodeInfoOf<T>` in the migration and its original definition. This PR corrects the storage alias to align it with its original definition I am uncertain about the proper approach for adding tests to this change. Would the team consider taking over this PR to bring it to completion? --------- Co-authored-by: pgherveou <pgherveou@gmail.com>
Description
We are recently trying to upgrade our Substrate version from
polkadot-v0.9.43topolkadot-v1.0.0and we noticed a critical issue: all deployed contracts seem to be experiencing aCodeNotFounderror. After a thorough investigation, it appears that the root cause of this issue lies in the mismatch between the storage alias ofCodeInfoOf<T>in the migration and its original definition.This PR corrects the storage alias to align it with its original definition
I am uncertain about the proper approach for adding tests to this change. Would the team consider taking over this PR to bring it to completion?