Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BaseFeeElasticity storage value update to migration code #1744

Merged
merged 1 commit into from Aug 30, 2022

Conversation

tgmichel
Copy link
Contributor

What does it do?

We need to update the storage value for BaseFee::Elasticity to Permill::zero before or at the time we introduce the upstream changes in polkadot-evm/frontier#794

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@tgmichel tgmichel added A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D1-runtime-migration PR introduces code that might require downstream chains to run a runtime upgrade. D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Aug 12, 2022
@tgmichel tgmichel requested a review from nanocryk August 12, 2022 08:17
@notlesh
Copy link
Contributor

notlesh commented Aug 12, 2022

Do we want to call set_elasticity instead of set_elasticity_inner so we get the event emitted?

get_storage_value::<Permill>(module, item, &[]).unwrap_or(Permill::zero());
if !current_value.is_zero() {
// Set Elasticity to zero, which results in constant BaseFeePerGas
let write = pallet_base_fee::Pallet::<T>::set_elasticity_inner(Permill::zero());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgmichel After this, maybe it's time to change the fixed gas price to the base-fee pallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes thank you, will do that on a separate PR

@tgmichel tgmichel merged commit 224f958 into master Aug 30, 2022
@tgmichel tgmichel deleted the tgm-elasticity-migration branch August 30, 2022 08:57
@crystalin
Copy link
Collaborator

Is this for RT1800 or RT1900 ?

@tgmichel
Copy link
Contributor Author

tgmichel commented Aug 30, 2022

@crystalin RT1900, which will most likely include polkadot-evm/frontier#794

@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited D1-runtime-migration PR introduces code that might require downstream chains to run a runtime upgrade.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants