chore: upgrade stable2409 #412

Daanvdplas · 2024-12-18T14:13:28Z

Uplifts pop to 2409. Some noteworthy changes had to be made for ismp (upstream changes) and the sdk. I left comments at the relevant places to have a discussion.

Notes:

Pallet revive will be added in a separate PR.
In some Cargo.toml files dependencies were added to the features. This was done while trying to find the cause of the compilation error (pop common). I left them in as they might cause issues in the future.

Daanvdplas · 2024-12-19T16:55:35Z

pallets/api/src/fungibles/tests.rs

@@ -341,7 +341,7 @@ fn start_destroy_works() {
 		// Check that the token is not live after starting the destroy process.
 		assert_noop!(
 			Assets::mint(signed(ALICE), token, ALICE, 10 * UNIT),
-			AssetsError::AssetNotLive
+			TokenError::UnknownAsset


Test was failing due to the following paritytech/polkadot-sdk#3286

Concerning to me is that this would have broken a smart contract. Presumably contracts won't often match on a specific error but if so, the contract could behave unexpected.

Curious to hear what others think about this.

Good to see that the test worked. If someone were using this, we would need to map the error AssetNotLive to UnknownAsset as to not break the contract.

Only a problem if we don't catch it before a runtime upgrade.

This error only changed for the mint function.

The reason why it is concerning to me is that I don't have the feeling parity is seeing this as a breaking change, meaning that they'll make such changes more often, meaning that we have to create more versions.

codecov-commenter · 2024-12-20T13:43:44Z

Codecov Report

Attention: Patch coverage is 13.63636% with 19 lines in your changes missing coverage. Please review.

Please upload report for BASE (chungquantin/chore-polkadot-stable2407@ab0bb49). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
node/src/command.rs	0.00%	7 Missing ⚠️
node/src/service.rs	0.00%	5 Missing ⚠️
node/src/rpc.rs	0.00%	2 Missing ⚠️
runtime/devnet/src/config/ismp.rs	0.00%	2 Missing ⚠️
runtime/common/src/lib.rs	0.00%	1 Missing ⚠️
runtime/devnet/src/lib.rs	0.00%	1 Missing ⚠️
runtime/testnet/src/lib.rs	0.00%	1 Missing ⚠️

@@                            Coverage Diff                            @@
##             chungquantin/chore-polkadot-stable2407     #412   +/-   ##
=========================================================================
  Coverage                                          ?   68.24%           
=========================================================================
  Files                                             ?       70           
  Lines                                             ?    11841           
  Branches                                          ?    11841           
=========================================================================
  Hits                                              ?     8081           
  Misses                                            ?     3503           
  Partials                                          ?      257

Files with missing lines	Coverage Δ
pallets/api/src/fungibles/tests.rs	`99.69% <100.00%> (ø)`
runtime/devnet/src/config/contracts.rs	`66.66% <ø> (ø)`
runtime/mainnet/src/config/xcm.rs	`0.00% <ø> (ø)`
runtime/mainnet/src/lib.rs	`21.44% <100.00%> (ø)`
runtime/testnet/src/config/contracts.rs	`66.66% <ø> (ø)`
runtime/common/src/lib.rs	`0.00% <0.00%> (ø)`
runtime/devnet/src/lib.rs	`5.44% <0.00%> (ø)`
runtime/testnet/src/lib.rs	`6.29% <0.00%> (ø)`
node/src/rpc.rs	`0.00% <0.00%> (ø)`
runtime/devnet/src/config/ismp.rs	`0.00% <0.00%> (ø)`
... and 2 more

Daanvdplas · 2024-12-20T15:15:15Z

pop-api/integration-tests/src/fungibles/mod.rs

 		assets::start_destroy(&addr, token);
-		assert_eq!(mint(&addr, token, &BOB, amount), Err(Module { index: 52, error: [16, 0] }));
+		assert_eq!(mint(&addr, token, &BOB, amount), Err(Token(UnknownAsset)));


Same error that changed as above.

Daanvdplas · 2024-12-20T15:22:49Z

runtime/devnet/src/config/contracts.rs

@@ -16,6 +16,7 @@ fn schedule<T: pallet_contracts::Config>() -> pallet_contracts::Schedule<T> {
 	pallet_contracts::Schedule {
 		limits: pallet_contracts::Limits {
 			runtime_memory: 1024 * 1024 * 1024,
+			validator_runtime_memory: 2 * 1024 * 1024 * 1024,


The integrity test was failing:

failures: ---- __construct_runtime_integrity_test::runtime_integrity_tests stdout ---- thread '__construct_runtime_integrity_test::runtime_integrity_tests' panicked at /Users/R0GUE/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pallet-contracts-38.0.0/src/lib.rs:707:13: Maximal storage size 143942352 exceeds the storage limit 0 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@peterwht do you remember why you put this in the runtimes. The SDK uses the default, yet an other error occurs when applying:

failures: ---- __construct_runtime_integrity_test::runtime_integrity_tests stdout ---- thread '__construct_runtime_integrity_test::runtime_integrity_tests' panicked at /Users/R0GUE/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pallet-contracts-38.0.0/src/lib.rs:657:13: Given `CallStack` height 24, `MaxCodeLen` should be set less than 8995 (current value is 262144), to avoid possible runtime oom issues. note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I see we have double the MaxCodeLen value configured as the sdk runtime linked.

I don't remember why, unfortunately. I guess it just gives us the flexibility to modify those as needed.

The reason the default isn't working is due to this: https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/contracts/src/lib.rs#L649

We would need to reduce the MaxCodeLen if we want to use the default schedule.

also, do you have a link for where this value is coming from: " validator_runtime_memory: 2 * 1024 * 1024 * 1024,"?

It had to be bigger than the other value so not much thought put towards it.

I changed the contracts configuration back to default. As we might transition to only using revive I suggest we look into this when we add revive to the runtime, and take a closer look at those configurations, in stead of now.

Daanvdplas · 2024-12-20T15:23:56Z

runtime/devnet/src/config/ismp.rs

@@ -17,7 +17,7 @@ impl pallet_ismp::Config for Runtime {
 	type Coprocessor = Coprocessor;
 	type Currency = Balances;
 	type HostStateMachine = HostStateMachine;
-	type Mmr = pallet_ismp::NoOpMmrTree<Self>;
+	type OffchainDB = ();


Confirmed with polytope labs team that this is oke to use in stead of the Mmr pallet

Daanvdplas · 2024-12-20T15:24:02Z

runtime/devnet/src/config/ismp.rs

@@ -17,7 +17,7 @@ impl pallet_ismp::Config for Runtime {
 	type Coprocessor = Coprocessor;
 	type Currency = Balances;
 	type HostStateMachine = HostStateMachine;
-	type Mmr = pallet_ismp::NoOpMmrTree<Self>;
+	type OffchainDB = ();


Confirmed with polytope labs team that this is oke to use in stead of the Mmr pallet

Daanvdplas · 2024-12-20T15:24:37Z

runtime/devnet/src/config/ismp.rs

-	fn module_for_id(&self, id: Vec<u8>) -> Result<Box<dyn IsmpModule>, Error> {
-		Err(Error::ModuleNotFound(id))
+	fn module_for_id(&self, id: Vec<u8>) -> Result<Box<dyn IsmpModule>, anyhow::Error> {
+		Err(anyhow::anyhow!("Module not found: {:?}", id))


Changed to anyhow error type

Daanvdplas · 2024-12-20T15:27:11Z

runtime/mainnet/src/config/xcm.rs

@@ -134,7 +134,7 @@ impl xcm_executor::Config for XcmConfig {
 	type CallDispatcher = RuntimeCall;
 	type FeeManager = XcmFeeManagerFromComponents<
 		WaivedLocations,
-		XcmFeeToAccount<Self::AssetTransactor, AccountId, SudoAddress>,
+		SendXcmFeeToAccount<Self::AssetTransactor, SudoAddress>,


paritytech/polkadot-sdk#4959

runtime/devnet/src/config/contracts.rs

peterwht · 2024-12-20T21:22:11Z

runtime/devnet/src/config/contracts.rs

@@ -16,6 +16,7 @@ fn schedule<T: pallet_contracts::Config>() -> pallet_contracts::Schedule<T> {
 	pallet_contracts::Schedule {
 		limits: pallet_contracts::Limits {
 			runtime_memory: 1024 * 1024 * 1024,
+			validator_runtime_memory: 2 * 1024 * 1024 * 1024,


I don't remember why, unfortunately. I guess it just gives us the flexibility to modify those as needed.

peterwht · 2024-12-20T21:35:14Z

runtime/devnet/src/config/contracts.rs

@@ -16,6 +16,7 @@ fn schedule<T: pallet_contracts::Config>() -> pallet_contracts::Schedule<T> {
 	pallet_contracts::Schedule {
 		limits: pallet_contracts::Limits {
 			runtime_memory: 1024 * 1024 * 1024,
+			validator_runtime_memory: 2 * 1024 * 1024 * 1024,


The reason the default isn't working is due to this: https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/contracts/src/lib.rs#L649

We would need to reduce the MaxCodeLen if we want to use the default schedule.

peterwht · 2024-12-20T22:30:10Z

pallets/api/src/fungibles/tests.rs

@@ -341,7 +341,7 @@ fn start_destroy_works() {
 		// Check that the token is not live after starting the destroy process.
 		assert_noop!(
 			Assets::mint(signed(ALICE), token, ALICE, 10 * UNIT),
-			AssetsError::AssetNotLive
+			TokenError::UnknownAsset


Good to see that the test worked. If someone were using this, we would need to map the error AssetNotLive to UnknownAsset as to not break the contract.

Only a problem if we don't catch it before a runtime upgrade.

runtime/common/src/lib.rs

peterwht · 2024-12-20T23:28:30Z

runtime/devnet/src/config/contracts.rs

@@ -16,6 +16,7 @@ fn schedule<T: pallet_contracts::Config>() -> pallet_contracts::Schedule<T> {
 	pallet_contracts::Schedule {
 		limits: pallet_contracts::Limits {
 			runtime_memory: 1024 * 1024 * 1024,
+			validator_runtime_memory: 2 * 1024 * 1024 * 1024,


also, do you have a link for where this value is coming from: " validator_runtime_memory: 2 * 1024 * 1024 * 1024,"?

AlexD10S

Thank you for the comment clarifying the key changes. Great work!

AlexD10S · 2024-12-27T14:29:06Z

Cargo.toml

@@ -45,14 +46,14 @@ futures = "0.3.30"
 hex = "0.4.3"
 hex-literal = "0.4.1"
 impl-trait-for-tuples = "0.2.2"
-jsonrpsee = { version = "0.23.2", features = [ "server" ] }
-log = { version = "0.4.21", default-features = false }
+jsonrpsee = { version = "0.24.3" }


Is the removal of features = [ "server" ] intentional, as mentioned in the description?

Daanvdplas force-pushed the chungquantin/chore-polkadot-stable2407 branch from 45b2cf6 to e7abcd9 Compare December 19, 2024 14:08

Daanvdplas mentioned this pull request Dec 19, 2024

chore: upgrade to stable2407 #234

Open

Daanvdplas and others added 9 commits December 19, 2024 15:55

refactor: sdk runtimes

2495e3d

refactor: relay folder

9fc485c

chore: upgrade to stable2407

a82e5f4

chore: 2409

59c6aa2

chore: dependencies propagation

bf0206d

chore: westend dep

5409bd1

chore: ismp to stable2409

197f182

fix: Xcm FeeManager

3fe17a2

fix: mistake after rebase

5a52b05

Daanvdplas force-pushed the daan/chore-stable2409 branch from 39144ac to 5a52b05 Compare December 19, 2024 14:59

Daanvdplas added 2 commits December 19, 2024 16:01

fix: remove local file made during upgrade

94e12f8

fix: assets error

c974cba

Daanvdplas commented Dec 19, 2024

View reviewed changes

Daanvdplas added 3 commits December 20, 2024 12:26

chore: bump jsonrpsee

f2c3131

chore: Cargo.lock

17627f4

fix: integrity tests

b93494f

Daanvdplas commented Dec 20, 2024

View reviewed changes

style: docs

013173b

peterwht reviewed Dec 21, 2024

View reviewed changes

Daanvdplas added 2 commits December 21, 2024 11:05

style: remove unnecessary multiplier

373225e

chore: default contracts config

9e2e592

Daanvdplas mentioned this pull request Dec 21, 2024

feat: revive #415

Open

docs: time in blocks

3836f2d

AlexD10S approved these changes Dec 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade stable2409 #412

chore: upgrade stable2409 #412

Daanvdplas commented Dec 18, 2024 •

edited

Loading

Daanvdplas Dec 19, 2024 •

edited

Loading

peterwht Dec 20, 2024

Daanvdplas Dec 21, 2024

codecov-commenter commented Dec 20, 2024 •

edited

Loading

Daanvdplas Dec 20, 2024

Daanvdplas Dec 20, 2024 •

edited

Loading

peterwht Dec 20, 2024

peterwht Dec 20, 2024

peterwht Dec 20, 2024

Daanvdplas Dec 21, 2024 •

edited

Loading

Daanvdplas Dec 21, 2024

Daanvdplas Dec 20, 2024

Daanvdplas Dec 20, 2024

Daanvdplas Dec 20, 2024

Daanvdplas Dec 20, 2024

peterwht Dec 20, 2024

peterwht Dec 20, 2024

peterwht Dec 20, 2024

peterwht Dec 20, 2024

AlexD10S left a comment

AlexD10S Dec 27, 2024

chore: upgrade stable2409 #412

Are you sure you want to change the base?

chore: upgrade stable2409 #412

Conversation

Daanvdplas commented Dec 18, 2024 • edited Loading

Daanvdplas Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 20, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Daanvdplas Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daanvdplas Dec 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexD10S left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daanvdplas commented Dec 18, 2024 •

edited

Loading

Daanvdplas Dec 19, 2024 •

edited

Loading

codecov-commenter commented Dec 20, 2024 •

edited

Loading

Daanvdplas Dec 20, 2024 •

edited

Loading

Daanvdplas Dec 21, 2024 •

edited

Loading