substrate icon indicating copy to clipboard operation
substrate copied to clipboard

Execute runtime sanity-checks in `follow-chain` command

Open kianenigma opened this issue 2 years ago • 10 comments

This PR adds a new hook to the try-runtime follow-chain subcommand to execute a custom hook called sanity_check per block. This allows us to avoid dumping too much testing code into debug_assertion, and instead test them in real life. To see more about what follow-chain itself is doing, see #9788.

Polkadot companion: https://github.com/paritytech/polkadot/compare/kiz-companion-sanity-check?expand=1

(Updated) Context:

I have pulled this PR out of the grave again, since I need it for my nomination pools. Reviews can be easy going, since this is all offchain code, but, please be aware:

  1. I genuinely dislike the name sanity_check now. I think we should rename it to on_block_check, and rename integrity_testto on_construct_runtime_check. If all agree, I will do it in this PR.
  2. I had to add a new type AllPalletWithSystemFlat, which is basically the same thing as before but tuples are not nested. This is needed because PalletInfoAccess is implemented on each individual pallet, but not on tuples. In reviews, please double check for me that this has no side-effect, other than us needing to increase some of the limits on impl_trait_for_tuples from 30 to 100.

Currently, the follow-chain command is an okay-ish way to continuously run these sanity checks, but honestly it is kinda slow and sluggish. As a follow-up, I will work on integrating this into the main client, so you can import blocks but use the try-runtime api instead of Core for block import.

TODO:

  1. [x] think of a wrapper trait to hide the second parameter.
  2. [ ] feature gate functions again.
  3. [x] fix all tests.
  4. [x] double check that AllPalletsWithSystemFlat has not side-effect.

kianenigma avatar Nov 04 '21 11:11 kianenigma

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 04 '21 12:12 stale[bot]

yes there is.

kianenigma avatar Dec 05 '21 09:12 kianenigma

stale but I will get to it soon.

kianenigma avatar Dec 16 '21 06:12 kianenigma

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jan 15 '22 06:01 stale[bot]

want this.

kianenigma avatar Feb 03 '22 13:02 kianenigma

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 05 '22 14:03 stale[bot]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 22 '22 07:04 stale[bot]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 05 '22 23:06 stale[bot]

YES.

kianenigma avatar Jun 06 '22 09:06 kianenigma

This is blocked by https://github.com/paritytech/substrate/pull/11813

kianenigma avatar Jul 22 '22 20:07 kianenigma

/cmd queue -c fmt $ 1

kianenigma avatar Aug 14 '22 19:08 kianenigma

@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1743776 was started for your command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 2-73259ac6-bb53-4172-a58a-fa4ec998e82c to cancel this command or /cmd cancel to cancel all commands in this pull request.

command-bot[bot] avatar Aug 14 '22 19:08 command-bot[bot]

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1 has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1743776 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1743776/artifacts/download.

command-bot[bot] avatar Aug 14 '22 19:08 command-bot[bot]

The CI pipeline was cancelled due to failure one of the required jobs. The job name - test-linux-stable The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1743788

paritytech-cicd-pr avatar Aug 14 '22 19:08 paritytech-cicd-pr

The CI pipeline was cancelled due to failure one of the required jobs. The job name - test-linux-stable The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1749329

paritytech-cicd-pr avatar Aug 16 '22 13:08 paritytech-cicd-pr

@bkchr @ggwpez @muharem can you review the companion as well?

kianenigma avatar Aug 28 '22 13:08 kianenigma

This will be merged, but it will be very buggy until https://github.com/paritytech/substrate/pull/12048 is merged.

kianenigma avatar Aug 29 '22 10:08 kianenigma

bot merge

kianenigma avatar Aug 30 '22 10:08 kianenigma

Error: Github API says https://github.com/paritytech/polkadot/pull/5907 is not mergeable

bot rebase

ggwpez avatar Aug 30 '22 11:08 ggwpez

Rebased

/cmd queue -c fmt $ 1

kianenigma avatar Sep 01 '22 09:09 kianenigma

@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1797449 was started for your command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 44-2cc79314-8b64-4c29-8eaa-5f8258928587 to cancel this command or /cmd cancel to cancel all commands in this pull request.

command-bot[bot] avatar Sep 01 '22 09:09 command-bot[bot]

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1 has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1797449 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1797449/artifacts/download.

command-bot[bot] avatar Sep 01 '22 09:09 command-bot[bot]

bot merge

kianenigma avatar Sep 01 '22 10:09 kianenigma

NOTE FOR DEVELOPERS:

This PR triggers a lot of errors of the try-runtime feature flag is not correctly set up on your runtime.

Make sure each custom pallet has at least a try-runtime feature flag:

try-runtime = [ "frame-support/try-runtime" ]

and that all of these features are enabled across the pallets used in your runtime:

See: https://github.com/paritytech/cumulus/pull/1551/commits/480b977ac7c9a7e94c0ce3b4599d71c22546ab84

Otherwise you will get errors like:

error: failed to run custom build command for `statemint-runtime v1.0.0 (/Users/shawntabrizi/Documents/GitHub/cumulus/parachains/runtimes/assets/statemint)`

Caused by:
  process didn't exit successfully: `/Users/shawntabrizi/Documents/GitHub/cumulus/target/debug/build/statemint-runtime-7ab48e0f89a28cba/build-script-build` (exit status: 1)
  --- stdout
  Information that should be included in a bug report.
  Executing build command: "/Users/shawntabrizi/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/cargo" "rustc" "--target=wasm32-unknown-unknown" "--manifest-path=/Users/shawntabrizi/Documents/GitHub/cumulus/target/debug/wbuild/statemint-runtime/Cargo.toml" "--color=always" "--profile" "release"
  Using rustc version: rustc 1.65.0-nightly (34a6cae28 2022-08-09)


  --- stderr
     Compiling pallet-utility v4.0.0-dev (https://github.com/paritytech/substrate?branch=master#5b0f999d)
     Compiling statemint-runtime v1.0.0 (/Users/shawntabrizi/Documents/GitHub/cumulus/parachains/runtimes/assets/statemint)
  error[E0599]: the function or associated item `try_runtime_upgrade` exists for struct `frame_executive::Executive<Runtime, sp_runtime::generic::Block<sp_runtime::generic::Header<u32, BlakeTwo256>, sp_runtime::generic::UncheckedExtrinsic<MultiAddress<sp_runtime::AccountId32, ()>, Call, MultiSignature, (CheckNonZeroSender<Runtime>, CheckSpecVersion<Runtime>, CheckTxVersion<Runtime>, CheckGenesis<Runtime>, CheckEra<Runtime>, CheckNonce<Runtime>, CheckWeight<Runtime>, ChargeAssetTxPayment<Runtime>)>>, ChainContext<Runtime>, Runtime, (frame_system::Pallet<Runtime>, cumulus_pallet_parachain_system::Pallet<Runtime>, pallet_timestamp::Pallet<Runtime>, parachain_info::Pallet<Runtime>, pallet_balances::Pallet<Runtime>, pallet_transaction_payment::Pallet<Runtime>, pallet_asset_tx_payment::Pallet<Runtime>, pallet_authorship::Pallet<Runtime>, pallet_collator_selection::Pallet<Runtime>, pallet_session::Pallet<Runtime>, pallet_aura::Pallet<Runtime>, cumulus_pallet_aura_ext::Pallet<Runtime>, cumulus_pallet_xcmp_queue::Pallet<Runtime>, pallet_xcm::Pallet<Runtime>, cumulus_pallet_xcm::Pallet<Runtime>, cumulus_pallet_dmp_queue::Pallet<Runtime>, pallet_utility::Pallet<Runtime>, pallet_multisig::Pallet<Runtime>, pallet_proxy::Pallet<Runtime>, pallet_assets::Pallet<Runtime>, pallet_uniques::Pallet<Runtime>)>`, but its trait bounds were not satisfied
     --> /Users/shawntabrizi/Documents/GitHub/cumulus/parachains/runtimes/assets/statemint/src/lib.rs:805:28
      |
  805 |             let weight = Executive::try_runtime_upgrade().unwrap();
      |                                     ^^^^^^^^^^^^^^^^^^^ function or associated item cannot be called on `frame_executive::Executive<Runtime, sp_runtime::generic::Block<sp_runtime::generic::Header<u32, BlakeTwo256>, sp_runtime::generic::UncheckedExtrinsic<MultiAddress<sp_runtime::AccountId32, ()>, Call, MultiSignature, (CheckNonZeroSender<Runtime>, CheckSpecVersion<Runtime>, CheckTxVersion<Runtime>, CheckGenesis<Runtime>, CheckEra<Runtime>, CheckNonce<Runtime>, CheckWeight<Runtime>, ChargeAssetTxPayment<Runtime>)>>, ChainContext<Runtime>, Runtime, (frame_system::Pallet<Runtime>, cumulus_pallet_parachain_system::Pallet<Runtime>, pallet_timestamp::Pallet<Runtime>, parachain_info::Pallet<Runtime>, pallet_balances::Pallet<Runtime>, pallet_transaction_payment::Pallet<Runtime>, pallet_asset_tx_payment::Pallet<Runtime>, pallet_authorship::Pallet<Runtime>, pallet_collator_selection::Pallet<Runtime>, pallet_session::Pallet<Runtime>, pallet_aura::Pallet<Runtime>, cumulus_pallet_aura_ext::Pallet<Runtime>, cumulus_pallet_xcmp_queue::Pallet<Runtime>, pallet_xcm::Pallet<Runtime>, cumulus_pallet_xcm::Pallet<Runtime>, cumulus_pallet_dmp_queue::Pallet<Runtime>, pallet_utility::Pallet<Runtime>, pallet_multisig::Pallet<Runtime>, pallet_proxy::Pallet<Runtime>, pallet_assets::Pallet<Runtime>, pallet_uniques::Pallet<Runtime>)>` due to unsatisfied trait bounds
      |
      = note: the following trait bounds were not satisfied:
              `(frame_system::Pallet<Runtime>, cumulus_pallet_parachain_system::Pallet<Runtime>, pallet_timestamp::Pallet<Runtime>, parachain_info::Pallet<Runtime>, pallet_balances::Pallet<Runtime>, pallet_transaction_payment::Pallet<Runtime>, pallet_asset_tx_payment::Pallet<Runtime>, pallet_authorship::Pallet<Runtime>, pallet_collator_selection::Pallet<Runtime>, pallet_session::Pallet<Runtime>, pallet_aura::Pallet<Runtime>, cumulus_pallet_aura_ext::Pallet<Runtime>, cumulus_pallet_xcmp_queue::Pallet<Runtime>, pallet_xcm::Pallet<Runtime>, cumulus_pallet_xcm::Pallet<Runtime>, cumulus_pallet_dmp_queue::Pallet<Runtime>, pallet_utility::Pallet<Runtime>, pallet_multisig::Pallet<Runtime>, pallet_proxy::Pallet<Runtime>, pallet_assets::Pallet<Runtime>, pallet_uniques::Pallet<Runtime>): TryState<u32>`

  error[E0599]: the function or associated item `try_execute_block` exists for struct `frame_executive::Executive<Runtime, sp_runtime::generic::Block<sp_runtime::generic::Header<u32, BlakeTwo256>, sp_runtime::generic::UncheckedExtrinsic<MultiAddress<sp_runtime::AccountId32, ()>, Call, MultiSignature, (CheckNonZeroSender<Runtime>, CheckSpecVersion<Runtime>, CheckTxVersion<Runtime>, CheckGenesis<Runtime>, CheckEra<Runtime>, CheckNonce<Runtime>, CheckWeight<Runtime>, ChargeAssetTxPayment<Runtime>)>>, ChainContext<Runtime>, Runtime, (frame_system::Pallet<Runtime>, cumulus_pallet_parachain_system::Pallet<Runtime>, pallet_timestamp::Pallet<Runtime>, parachain_info::Pallet<Runtime>, pallet_balances::Pallet<Runtime>, pallet_transaction_payment::Pallet<Runtime>, pallet_asset_tx_payment::Pallet<Runtime>, pallet_authorship::Pallet<Runtime>, pallet_collator_selection::Pallet<Runtime>, pallet_session::Pallet<Runtime>, pallet_aura::Pallet<Runtime>, cumulus_pallet_aura_ext::Pallet<Runtime>, cumulus_pallet_xcmp_queue::Pallet<Runtime>, pallet_xcm::Pallet<Runtime>, cumulus_pallet_xcm::Pallet<Runtime>, cumulus_pallet_dmp_queue::Pallet<Runtime>, pallet_utility::Pallet<Runtime>, pallet_multisig::Pallet<Runtime>, pallet_proxy::Pallet<Runtime>, pallet_assets::Pallet<Runtime>, pallet_uniques::Pallet<Runtime>)>`, but its trait bounds were not satisfied
     --> /Users/shawntabrizi/Documents/GitHub/cumulus/parachains/runtimes/assets/statemint/src/lib.rs:817:15
      |
  817 |             Executive::try_execute_block(block, state_root_check, select).expect("try_execute_block failed")
      |                        ^^^^^^^^^^^^^^^^^ function or associated item cannot be called on `frame_executive::Executive<Runtime, sp_runtime::generic::Block<sp_runtime::generic::Header<u32, BlakeTwo256>, sp_runtime::generic::UncheckedExtrinsic<MultiAddress<sp_runtime::AccountId32, ()>, Call, MultiSignature, (CheckNonZeroSender<Runtime>, CheckSpecVersion<Runtime>, CheckTxVersion<Runtime>, CheckGenesis<Runtime>, CheckEra<Runtime>, CheckNonce<Runtime>, CheckWeight<Runtime>, ChargeAssetTxPayment<Runtime>)>>, ChainContext<Runtime>, Runtime, (frame_system::Pallet<Runtime>, cumulus_pallet_parachain_system::Pallet<Runtime>, pallet_timestamp::Pallet<Runtime>, parachain_info::Pallet<Runtime>, pallet_balances::Pallet<Runtime>, pallet_transaction_payment::Pallet<Runtime>, pallet_asset_tx_payment::Pallet<Runtime>, pallet_authorship::Pallet<Runtime>, pallet_collator_selection::Pallet<Runtime>, pallet_session::Pallet<Runtime>, pallet_aura::Pallet<Runtime>, cumulus_pallet_aura_ext::Pallet<Runtime>, cumulus_pallet_xcmp_queue::Pallet<Runtime>, pallet_xcm::Pallet<Runtime>, cumulus_pallet_xcm::Pallet<Runtime>, cumulus_pallet_dmp_queue::Pallet<Runtime>, pallet_utility::Pallet<Runtime>, pallet_multisig::Pallet<Runtime>, pallet_proxy::Pallet<Runtime>, pallet_assets::Pallet<Runtime>, pallet_uniques::Pallet<Runtime>)>` due to unsatisfied trait bounds
      |
      = note: the following trait bounds were not satisfied:
              `(frame_system::Pallet<Runtime>, cumulus_pallet_parachain_system::Pallet<Runtime>, pallet_timestamp::Pallet<Runtime>, parachain_info::Pallet<Runtime>, pallet_balances::Pallet<Runtime>, pallet_transaction_payment::Pallet<Runtime>, pallet_asset_tx_payment::Pallet<Runtime>, pallet_authorship::Pallet<Runtime>, pallet_collator_selection::Pallet<Runtime>, pallet_session::Pallet<Runtime>, pallet_aura::Pallet<Runtime>, cumulus_pallet_aura_ext::Pallet<Runtime>, cumulus_pallet_xcmp_queue::Pallet<Runtime>, pallet_xcm::Pallet<Runtime>, cumulus_pallet_xcm::Pallet<Runtime>, cumulus_pallet_dmp_queue::Pallet<Runtime>, pallet_utility::Pallet<Runtime>, pallet_multisig::Pallet<Runtime>, pallet_proxy::Pallet<Runtime>, pallet_assets::Pallet<Runtime>, pallet_uniques::Pallet<Runtime>): TryState<u32>`

  For more information about this error, try `rustc --explain E0599`.
  error: could not compile `statemint-runtime` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

shawntabrizi avatar Sep 01 '22 16:09 shawntabrizi

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/testing-complext-frame-pallets-discussion-tools/356/1

Polkadot-Forum avatar Sep 13 '22 10:09 Polkadot-Forum