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

Introduce a gas-based storage limit [MBIP-5] #2452

Merged
merged 26 commits into from
Aug 30, 2023
Merged

Introduce a gas-based storage limit [MBIP-5] #2452

merged 26 commits into from
Aug 30, 2023

Conversation

ahmadkaouk
Copy link
Contributor

@ahmadkaouk ahmadkaouk commented Aug 25, 2023

Introduction

This PR implements a new mechanism as detailed in MBIP-5. The aim is to manage the unsustainable storage growth of the state by setting a block storage limit and increasing gas usage when a transaction results in an increase in storage state. Key features include:

  • Block Storage Growth Limit: A single block cannot contain transactions that collectively increase the storage state by more than 40 KB.
  • Modified Gas Computation Mechanism: Transactions resulting in storage state expansion will be subject to a new, generally higher, required gas to execute.

This feature only enabled on Moonbase.

Example

Given:

  • BLOCK_GAS_LIMIT = 15,000,000 (Maximum gas per block)
  • BLOCK_STORAGE_LIMIT = 40 * 1024 bytes (40 KB per block)

The ratio between gas and storage is calculated as:

  • RATIO = 15,000,000 / (40 * 1024) ≈ 366

Deploying a Smart Contract of 24 kB will cost:

  • GAS_COST = 24 * 1024 * 366 = 8,994,816

Executing a transaction that increases the storage by 500 bytes will cost:

  • GAS_COST = 500 * 366 = 183,000

Runtime Changes

  • GasLimitStorageGrowthRatio: A new parameter in pallet-evm that sets the ratio of gas to storage growth, defaulting to 366.
  • Precompile Adaptations: Modifications have been made to precompiles to accommodate the new changes. A new parameter storage_growth: Option<u64> has been added to the try_dispatch function to record the storage increase.

⚠️ Breaking Changes ⚠️

  • New Gas Cost: This update introduces key adjustments to how gas fees are calculated, specifically accounting for the increase of the storage state. Transactions may run out of gas even if they meet the initial gasometer requirements. Here are the key points to note:
    • Increased Costs for Contract Deployments: Contract deployments that add to the state will see a noticeable rise in gas costs.
    • Gas increased for Specific Precompiled Contract Calls: Transactions that trigger precompiled contracts, that can possibly create new accounts, will have a minimum gas fee of 54,168.(The result of the storage increase associated with a new account (148 bytes) and the established gas-to-storage ratio of 366).
    • Transactions Impacting Storage Growth: Transactions that lead to the creation of new storage entries will also experience increased gas costs. This includes, but is not limited to, transactions that add new records or key-value pairs to the state.

Despite these changes, gas estimation will continue to function as intended, incorporating the new storage-related gas costs in their calculations.

Relevant PRs:

Cargo.toml Outdated Show resolved Hide resolved
@librelois librelois mentioned this pull request Aug 25, 2023
28 tasks
@ahmadkaouk ahmadkaouk added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited breaking Needs to be mentioned in breaking changes labels Aug 28, 2023
@ahmadkaouk ahmadkaouk added the A0-pleasereview Pull request needs code review. label Aug 29, 2023
@ahmadkaouk ahmadkaouk marked this pull request as ready for review August 29, 2023 11:20
@github-actions
Copy link
Contributor

github-actions bot commented Aug 29, 2023

Coverage generated "Tue Aug 29 23:23:22 UTC 2023":
https://d3ifz9vhxc2wtb.cloudfront.net/pulls/2452/html/index.html

Master coverage: 87.39%
Pull coverage:

Copy link
Collaborator

@crystalin crystalin left a comment

Choose a reason for hiding this comment

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

please wait for a review from @librelois or @notlesh before merging

Copy link
Contributor

@notlesh notlesh left a comment

Choose a reason for hiding this comment

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

Looks good 🎉

Ideally we could see some test coverage that shows this is properly disabled for moonriver and moonbeam (or a test showing that a ratio of 0 does indeed disable the feature). However, I can clearly see here that it should work as intended.

@ahmadkaouk ahmadkaouk merged commit 7759b61 into master Aug 30, 2023
27 checks passed
@ahmadkaouk ahmadkaouk deleted the ahmad-mbip-5 branch August 30, 2023 08:29
@ahmadkaouk ahmadkaouk removed the A0-pleasereview Pull request needs code review. label Aug 30, 2023
@noandrea noandrea changed the title MBIP 5 Introduce a gas-based storage limit [MBIP-5] Aug 30, 2023
@@ -58,9 +62,10 @@ where
Runtime::RuntimeCall: Dispatchable<PostInfo = PostDispatchInfo> + GetDispatchInfo,
{
#[inline(always)]
pub fn record_weight_v2_cost(
pub fn reocrd_external_cost(

Choose a reason for hiding this comment

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

typo in function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants