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

Adds Gas to Weight mapping #174

Merged
merged 10 commits into from
Jan 14, 2021
Merged

Adds Gas to Weight mapping #174

merged 10 commits into from
Jan 14, 2021

Conversation

crystalin
Copy link
Collaborator

@crystalin crystalin commented Jan 12, 2021

What does it do?

It adds a better GasWeightMapper than the default one (1-1 mapping).
The new GasWeightMapper compute the gas/weight based on a given ratio.

From our benchmarking, the EVM computation can consume up to 10M Gas/second (on a 4.4ghz) with some optimizations (no trace log, using compiled WASM).

To stay on a very safe path and considering that our current targets are under the required specs, we are targeting a consumption of 4M Gas/second. The GasToWeight conversation is doing: gas * WEIGHT_PER_SECOND / GAS_PER_SECOND

Considering that our current block limit is 500ms Weight (requirement to be a parachain), it means we will have 4M Gas limit per block.
However, the current computation of block weight consider that 25% for operations (including 10% for block initialization) and 75% for the transactions (so 3M gas here)

This diagram gives an overview of the gas consumption:
Moonbeam Gas_Weight Consumption - Alphanet v5

This gives us a approximate effective gas per block of 3M/block (so 0.5M effective gas/s, ~50% of Ethereum mainnet)

Also, additional note for now, current extrinsics are consuming 125_000_000 weight extra (which is 1000 gas). We might want to merge that into the 21_000 already consumed by the EVM

Another addition is the fact that the maximum extrinsic weight is equal to the maximum normal class weight (75%) minux the data block initialization (10%, not sure why it is included as it doesn't belong to the normal class), minus the extrinsic base weight (125_000), (which is roughly around 64.9% of the block weight). In term of gas it means a Tx is limited to 2_599_000 gas

Is there something left for follow-up PRs?

There will be some tweaking PR to increase those numbers once our benchmark are more accurate and once we have defined our final specs.

Another PR to adapt the Ethereum Gas Limit to reflect the actual gas limit is needed. That will be possible after polkadot-evm/frontier#265 lands.

What value does it bring to the blockchain users?

It gives an accurate limit of the possibilities of the current parachain.
It prevents the network to stall when a TX saturates the EVM and is too long

Copy link
Contributor

@JoshOrndorff JoshOrndorff left a comment

Choose a reason for hiding this comment

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

I think this is a good approach to start. And I appreciate the thorough description in the PR.

The Weight type is an alias for u64. Why are we using u128 for these calculations? https://crates.parity.io/frame_support/weights/type.Weight.html

Like I'm

runtime/src/lib.rs Outdated Show resolved Hide resolved
tests/tests/test-gas.ts Outdated Show resolved Hide resolved
runtime/src/lib.rs Outdated Show resolved Hide resolved
runtime/src/lib.rs Outdated Show resolved Hide resolved
runtime/src/lib.rs Outdated Show resolved Hide resolved
crystalin and others added 3 commits January 12, 2021 11:10
Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
@crystalin
Copy link
Collaborator Author

With 1.3M effective gas per block, we might be under some contract limit.
@albertov19 can you tell me what is the highest gas you ever used ?

@albertov19
Copy link
Contributor

Yes its a bit low, like I mentioned in the meeting, the Oracle contract from Chainlink needs 1.8M to be deployed, and its just one example at the top of my mind.

One ERC721 (NFT token) token contract that I have (it is one with a lots of add-ons) needs 2.4M of gas. We will talk with Rarible soon (an NFT marketplace), so it is somewhat important we are able to showcase compatibility with NFTs.

With 1.3M effective gas per block, we might be under some contract limit.
@albertov19 can you tell me what is the highest gas you ever used ?

@crystalin
Copy link
Collaborator Author

Yes its a bit low, like I mentioned in the meeting, the Oracle contract from Chainlink needs 1.8M to be deployed, and its just one example at the top of my mind.

One ERC721 (NFT token) token contract that I have (it is one with a lots of add-ons) needs 2.4M of gas. We will talk with Rarible soon (an NFT marketplace), so it is somewhat important we are able to showcase compatibility with NFTs.

With 1.3M effective gas per block, we might be under some contract limit.
@albertov19 can you tell me what is the highest gas you ever used ?

Thank you. I'll raise to 2.6M effective gas / block (Equivalent to 8M gas/s on 500ms Weight)

@crystalin
Copy link
Collaborator Author

I did some changes, It might need another review @JoshOrndorff

runtime/src/lib.rs Show resolved Hide resolved
tests/tests/test-gas.ts Outdated Show resolved Hide resolved
runtime/src/lib.rs Show resolved Hide resolved
@crystalin crystalin merged commit 091dedd into master Jan 14, 2021
@crystalin crystalin deleted the crystalin-gas-to-weight branch January 14, 2021 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants