threefoldtech / tfchain

Threefold Chain.
Apache License 2.0
15 stars 11 forks source link

try-runtime Report Zero Weight Despite Successful Migration Execution #997

Open sameh-farouk opened 3 weeks ago

sameh-farouk commented 3 weeks ago

I'm currently testing a storage migration on my branch development-contracts-billing-refactor with try-runtime but encountered an issue where the weight returned by ʼExecutive::try_runtime_upgrade(checks).unwrap();ʼ is 0.

The migration logic itself seems to be working correctly, as logging within the migration function shows that the proper weight is being calculated and returned. However, when running try-runtime-cli, even though the migration logic is triggered, the reported runtime ref_time consumed is 0.

End of logs:

2024-08-21T23:30:31Z DEBUG wasm-heap] allocator dropped: AllocationStats { bytes_allocated: 56, bytes_allocated_peak: 696, bytes_allocated_sum: 336360, address_space_used: 1312 }
2024-08-21T23:30:31Z DEBUG try-runtime::cli] Proof: 0x5ed41e5e16056765bc84... / 1201 nodes
2024-08-21T23:30:31Z DEBUG try-runtime::cli] Encoded proof size: 147.0 KB
2024-08-21T23:30:31Z DEBUG try-runtime::cli] Compact proof size: 108.3 KB
2024-08-21T23:30:31Z INFO  try-runtime::cli] PoV size (zstd-compressed compact proof): 66.6 KB. For parachains, it's your responsibility to verify that a PoV of this size fits within any relaychain constraints.
2024-08-21T23:30:31Z INFO  try-runtime::cli] Consumed ref_time: 0s (0.00% of max 2s)
2024-08-21T23:30:31Z INFO  try-runtime::cli] ✅ No weight safety issues detected. Please note this does not guarantee a successful runtime upgrade. Always test your runtime upgrade with recent state, and ensure that the weight usage of your migrations will not drastically differ between testing and actual on-chain execution.

I tried to pass --checks=none and --checks=all to on-runtime-upgrade, but the result remains the same—0 weight is reported, even though the migration logic is executed always as expected.

sameh-farouk commented 3 weeks ago

I found that this issue is also reproducible in the development branch with any migration. I also verified that it has been there since we upgraded to Polkadot v1.0.0. It Seems the try-runtime is broken with that release.

sameh-farouk commented 3 weeks ago

@renauter brought good news that the migration issue is fixed in Polkadot v1.1.0. He also pinpointed the related PR containing this bug fix https://github.com/paritytech/substrate/pull/14793 from the v1.1.0 release notes.

He already has a draft pull request to upgrade to v1.1.0, so we will push it forward.

renauter commented 2 weeks ago

Indeed, https://github.com/threefoldtech/tfchain/pull/902 should fix this issue