Upgrade to v0.9.40

[stiiifff]

Upgrade Trappist's runtime from v0.9.37 to v0.9.40.

• Upgrade DEX pallet to v0.9.40
• Consult Polkadot v0.9.38 release report for important changes that may impact Trappist's runtime.
• Consult Polkadot v0.9.39 release report for important changes that may impact Trappist's runtime.
• Consult Polkadot v0.9.40 release report for important changes that may impact Trappist's runtime.
• Upgrade Chess pallet to polkadot-v0.9.40
• Apply PR#2108 from Contracts on Rococo chain's runtime
• Upgrade Trappist runtime & node to v0.9.40. See Parachain template update to v0.9.40 PR for guidance.
• Review Runtime config for any misconfigurations.
• Review XCM config for any miconfigurations.
• Change WeightToFee struct to charge for proof size weight. [0.9.40] Change WeightToFee struct to charge for proof size weight. #169

[hbulgarini]

@valentinfernandez1 please take this one into account: #147

is an easy one though.

[hbulgarini]

@stiiifff , i wonder if we should perform each runtime upgrade from 0.9.37 to 0.9.40 and do not directly jump to 0.9.40. I think is not the case for these ones, but skipping versions in the runtime upgrade might lead to not run storage migrations present on specific releases.

[stiiifff]

@hbulgarini Can you verify the last assertion about storage migrations ? this sounds a bit scary to tbh.

[hbulgarini]

@hbulgarini Can you verify the last assertion about storage migrations ? this sounds a bit scary to tbh.

it is not the case for these upgrades but anyway i would try to avoid jumping versions in the future. At least i would recommend to go to 0.9.38 (xcm v3) and maybe jump to 0.9.40.

[valentinfernandez1]

That's a great question, there has to be a better approach once the chain is live on rococo. It makes sense to avoid skipping versions, but it seems odd once the chain is running to create a proposal and approve it only to increase the versions (if the upgrades are done though referendums of the democracy pallet) .

[stiiifff]

it is not the case for these upgrades but anyway i would try to avoid jumping versions in the future. At least i would recommend to go to 0.9.38 (xcm v3) and maybe jump to 0.9.40.

Let's upgrade to v0.9.40 in one go this time, we'll upgrade to every release after that when the chain is live.

[valentinfernandez1]

The upgrade requires to regenerate the weights.rs files since Weight::from_ref_time() was deprecated to include a call that also has the proof_size. Egg:

fn create_exchange() -> Weight {
  Weight::from_parts(25_000_000, u64::MAX)
  ...
}

Should this be done here or as part of #141 ?

[hbulgarini]

The upgrade requires to regenerate the weights.rs files since Weight::from_ref_time() was deprecated to include a call that also has the proof_size. Egg:

fn create_exchange() -> Weight {
  Weight::from_parts(25_000_000, u64::MAX)
  ...
}

Should this be done here or as part of #141 ?

IMHO this should be done in this issue because that benchmarking setup is required for the lunch and the changes here are going to happen after it. I would recommend you to take a look at this PR for doing the migration:

paritytech/substrate#11637

[valentinfernandez1]

What is the current purpose of pallet_randomness_collective_flipon trappist? Since this pallet changed name in 9.39, and now is marked as insecure.

https://substrate.stackexchange.com/questions/8268/why-has-the-pallet-randomness-collective-flip-been-removed-in-polkadot-v0-9-39/8269#8269

Can this be removed or should we keep it using pallet-insecure-randomness-collective-flip?

[stiiifff]

@valentinfernandez1 Good question.
It's only used to provide a source of randomness to the smart contracts layer (see here).
As mentioned in the pallet-insecure-randomness-collective-flip's readme, it should not be used for high-stake production use-cases. But as Trappist is only used as an experimental / development environment, I think this is acceptable. Even when deployed on Rococo, Trappist will not be an incentivised network. So, contracts deployed on it should be experimental / prototypal in nature, we just need to make that very clear. @hbulgarini do you agree ?
As mentioned in the PR that added the -insecure moniker to the pallet's name, to make it even more apparent, there is currently no alternative other than to bridge to a secure randomness source.

[hbulgarini]

@valentinfernandez1 Good question. It's only used to provide a source of randomness to the smart contracts layer (see here). As mentioned in the pallet-insecure-randomness-collective-flip's readme, it should not be used for high-stake production use-cases. But as Trappist is only used as an experimental / development environment, I think this is acceptable. Even when deployed on Rococo, Trappist will not be an incentivised network. So, contracts deployed on it should be experimental / prototypal in nature, we just need to make that very clear. @hbulgarini do you agree ? As mentioned in the PR that added the -insecure moniker to the pallet's name, to make it even more apparent, there is currently no alternative other than to bridge to a secure randomness source.

I agree that at this point there is no need to replace the pallet-insecure-randomness-collective-flip. The reason why it was renamed was because this randomness pallet was included in the node template and as a consequence most of the chains are using it without acknowledging the security implications.

In the future i think it would be a nice showcase to implement a custom pallet that is able to get the randomness from the relay chain/Babe. This code can work as inspiration:

fn relay_chain_state_proof() -> RelayChainStateProof {
	let relay_storage_root = ParachainSystem::validation_data()
		.expect("set in `set_validation_data`")
		.relay_parent_storage_root;
	let relay_chain_state =
		ParachainSystem::relay_state_proof().expect("set in `set_validation_data`");
	RelayChainStateProof::new(ParachainInfo::get(), relay_storage_root, relay_chain_state)
		.expect("Invalid relay chain state proof, already constructed in `set_validation_data`")
}

[valentinfernandez1]

In the future i think it would be a nice showcase to implement a custom pallet that is able to get the randomness from the relay chain/Babe. This code can work as inspiration:

If you could add that issue, I can work on it after this task is done.

[valentinfernandez1]

@hbulgarini I will be creating a new branch called xcmv3-dev to consolidate all the changes intended for the upcoming milestone. We should try to keep the main branch only for changes that will be on the initial deploy on Rococo .

This means that any modifications planned to be added via the runtime upgrade can now be worked on within the xcmv3-dev branch until Trappist is deployed on Rococo. After that we can merge the branch with main to prepare for the Runtime upgrade.

[stiiifff]

Closed by #171, PR merged into dev branch xcmv3-dev.