Contracts migration v9 to v10 is failling

SailorSnoW commented 1 year ago

Is there an existing issue?

[X] I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

[X] This is not a support question.

Description of bug

We are trying to upgrade our chain from polkadot-v0.9.43 to polkadot-v1.0.0 and can't manage to pass the migrations for the contracts pallet.

The chain is working fine on the runtime using polkadot-v0.9.43 and use the v9 pallet contract storage. We actually have only 3 contracts on-chain.

When doing a try-runtime against our living chain with the new runtime and the v10, v11, v12 migrations effective in the pallet_contracts Config trait, here what is failing on the v10 migration:

ERROR main runtime: panicked at 'calledResult::unwrap()on anErrvalue: Other("deposit mismatch")', /Users/snow/.cargo/git/checkouts/substrate-7e08433d4c370a21/948fbd2/frame/contracts/src/migration.rs:296:35

Seems like the migration is failing cause something happen during the deposit changes. Here is the full logs with some debug datas that I have added about the what's going on with the deposits: logs.txt

(some deposits value are really big and abnormal, the unreserve part seems not working and the new contract have 0 as deposits value)

Steps to reproduce

clone our chain repo
build the new runtime: cargo build --release --package symphonie-runtime --features try-runtime
start a local node of our live testnet chain: cargo run --release --features try-runtime -- --chain=symphonie --sync warp --tmp
execute the try-runtime on_chain_upgrade: RUST_LOG=executor=trace,runtime=trace cargo run --release --features try-runtime -- try-runtime --runtime ./target/release/wbuild/symphonie-runtime/symphonie_runtime.compact.compressed.wasm on-runtime-upgrade --checks live --uri ws://localhost:9944

ggwpez commented 1 year ago

cc @pgherveou

pgherveou commented 1 year ago

Looking into it

pgherveou commented 1 year ago

Looks like the migration code is hitting an assert error because the deposit account is empty, let me try to figure out why and come back to you. Thanks for all the explanation for compiling and reproducing the bug, this makes my life much easier :)

pgherveou commented 1 year ago

@SailorSnoW was 0.9.43 the first version of the pallet or did you go through other migrations before?

pgherveou commented 1 year ago

Looks like we only introduced migrations at a later point, and because the code you are running on already includes the updates defined in v10.rs, the solution here is to change the migration code to:

@athei lmk if I got the history wrong here

M runtime/symphonie/src/lib.rs
@@ -777,7 +777,7 @@ impl pallet_contracts::Config for Runtime {
    type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
    #[cfg(not(feature = "runtime-benchmarks"))]
    type Migrations = (
-       pallet_contracts::migration::v10::Migration<Runtime>,
+       pallet_contracts::NoopMigration<10>,
        pallet_contracts::migration::v11::Migration<Runtime>,
        pallet_contracts::migration::v12::Migration<Runtime>,
    );

SailorSnoW commented 1 year ago

Perfect that worked great, thanks :) I wasn't aware of NoopMigration doing a dummy migration by just upgrading the pallet version so yeah that was what I needed.

paritytech / substrate