Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Configuration module migration may be broken in presence of PendingConfig #4533

Closed
pepyakin opened this issue Dec 15, 2021 · 0 comments · Fixed by #4540
Closed

Configuration module migration may be broken in presence of PendingConfig #4533

pepyakin opened this issue Dec 15, 2021 · 0 comments · Fixed by #4540
Milestone

Comments

@pepyakin
Copy link
Contributor

pepyakin commented Dec 15, 2021

The configuration upgrade to version 2, introduced by 10f3c85, may be broken in case there are non empty PendingConfig entries. If this is the case, then querying such a pending configuration change will lead to panic in the runtime.

@pepyakin pepyakin added this to the v0.9.14 milestone Dec 15, 2021
@ordian ordian modified the milestones: v0.9.14, v0.9.15 Dec 15, 2021
pepyakin added a commit that referenced this issue Dec 16, 2021
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.
pepyakin added a commit that referenced this issue Dec 16, 2021
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.
pepyakin added a commit that referenced this issue Dec 16, 2021
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.
pepyakin added a commit that referenced this issue Dec 17, 2021
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.
@s3krit s3krit modified the milestones: v0.9.15, v0.9.16 Dec 17, 2021
paritytech-processbot bot pushed a commit that referenced this issue Dec 21, 2021
* 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 <admin@parity.io>
@ordian ordian modified the milestones: v0.9.16, v0.9.15 Dec 23, 2021
drahnr pushed a commit that referenced this issue Jan 4, 2022
* 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 <admin@parity.io>
@chevdor chevdor modified the milestones: v0.9.15, v0.9.16 Jan 11, 2022
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this issue Feb 3, 2022
* 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 <admin@parity.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants