-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Conversation
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
53415c9
to
2508785
Compare
/benchmark runtime kusama runtime_parachains::configuration |
Benchmark Runtime Kusama Pallet for branch "pep-configuration-fixes" with command cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_configuration.rs Results
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
/benchmark runtime polkadot runtime_parachains::configuration |
Benchmark Runtime Polkadot Pallet for branch "pep-configuration-fixes" with command cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_configuration.rs Results
|
/benchmark runtime westend runtime_parachains::configuration |
Benchmark Runtime Westend Pallet for branch "pep-configuration-fixes" with command cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_parachains_configuration.rs Results
|
/benchmark runtime custom --chain=rococo-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/rococo/src/weights/runtime_parachains_configuration.rs |
Benchmark Runtime Custom for branch "pep-configuration-fixes" with command cargo run --quiet --release --features runtime-benchmarks -- benchmark --chain=rococo-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/rococo/src/weights/runtime_parachains_configuration.rs Results
|
pub(crate) type PendingConfigs<T: Config> = | ||
StorageValue<_, Vec<(SessionIndex, HostConfiguration<T::BlockNumber>)>, ValueQuery>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it can only contain 2 items, why not use a fixed size array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would that give though? At the same time, it would be more annoying to change if we were to change the SESSION_DELAY.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No specific reason, just thinking that if you are going to have some specific constraint about this storage item, that we can just program that in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I am generally pro making it more typesafe, but I believe here it does not provide much benefit while can be potential problem in the future (although I admit that it is also not super likely we will change SESSION_DELAY)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good to me
937b926
to
919c9f5
Compare
Closes #4529 Closes #4533 I figured that trying to avoid updates does not really worth it to keep. This is because we seem to not update the configuration often and when we do we approach this carefully. Thus possibility of a redundant update is really negligable. At the same time, if such a redundant update does happen then the effects of that are really small: just some wasted storage interactions. On the other hand, making it work was a little bit annoying. With the proper fix for the pending updates this would be even more annoying since now we would have to add combinatorically more cases to test this. So I figured that I will just scrap that and simplify the code.
…k --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_configuration.rs
…k --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_configuration.rs
…k --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_parachains_configuration.rs
919c9f5
to
4e47095
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
4e47095
to
1e40691
Compare
This PR doesn't introduce a new runtime migration but rather fixes the code in the previously introduced one. |
bot merge |
Waiting for commit status. |
* parachains: Fix configuration module Closes #4529 Closes #4533 I figured that trying to avoid updates does not really worth it to keep. This is because we seem to not update the configuration often and when we do we approach this carefully. Thus possibility of a redundant update is really negligable. At the same time, if such a redundant update does happen then the effects of that are really small: just some wasted storage interactions. On the other hand, making it work was a little bit annoying. With the proper fix for the pending updates this would be even more annoying since now we would have to add combinatorically more cases to test this. So I figured that I will just scrap that and simplify the code. * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_configuration.rs * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_configuration.rs * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_parachains_configuration.rs * review fixes Co-authored-by: Parity Bot <[email protected]>
* parachains: Fix configuration module Closes paritytech#4529 Closes paritytech#4533 I figured that trying to avoid updates does not really worth it to keep. This is because we seem to not update the configuration often and when we do we approach this carefully. Thus possibility of a redundant update is really negligable. At the same time, if such a redundant update does happen then the effects of that are really small: just some wasted storage interactions. On the other hand, making it work was a little bit annoying. With the proper fix for the pending updates this would be even more annoying since now we would have to add combinatorically more cases to test this. So I figured that I will just scrap that and simplify the code. * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_configuration.rs * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_configuration.rs * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_parachains::configuration --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_parachains_configuration.rs * review fixes Co-authored-by: Parity Bot <[email protected]>
This commit incorporates the changes made to the runtime in the following PRs: - paritytech#4408 - paritytech#4457 - paritytech#4540 - paritytech#4542 - paritytech#4581 Note that this PR does not include the description of the PVF pre-checker subsystem. This should be addressed within paritytech#4611 Co-authored-by: sandreim <[email protected]>
Closes #4529
Closes #4533
I figured that trying to avoid updates does not really worth it to keep.
This is because we seem to not update the configuration often and when
we do we approach this carefully. Thus possibility of a redundant update
is really negligable. At the same time, if such a redundant update does
happen then the effects of that are really small: just some wasted
storage interactions.
On the other hand, making it work was a little bit annoying. With the
proper fix for the pending updates this would be even more annoying
since now we would have to add combinatorically more cases to test this.
So I figured that I will just scrap that and simplify the code.
Runtime Migration
This PR doesn't introduce a new runtime migration but rather fixes the migration code introduced in #4420
skip check-dependent-cumulus