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

[pallet-revive] Update gas encoding #6689

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

Conversation

pgherveou
Copy link
Contributor

@pgherveou pgherveou commented Nov 28, 2024

Update the current approach to attach the ref_time, pov and deposit parameters to an Ethereum transaction.
Previously we will pass these 3 parameters along with the signed payload, and check that the fees resulting from gas x gas_price match the actual fees paid by the user for the extrinsic.

This approach unfortunately can be attacked. A malicious actor could force such a transaction to fail by injecting low values for some of these extra parameters as they are not part of the signed payload.

The new approach encodes these 3 extra parameters in the lower digits of the transaction gas, approximating the the log2 of the actual values to encode each components on 2 digits

@pgherveou pgherveou changed the base branch from master to pg/fix-geth-diff November 28, 2024 10:55
Base automatically changed from pg/fix-geth-diff to master December 1, 2024 17:21
@pgherveou pgherveou self-assigned this Dec 10, 2024
@pgherveou
Copy link
Contributor Author

/cmd prdoc --audience runtime_dev --bump minor

@pgherveou pgherveou added T7-smart_contracts This PR/Issue is related to smart contracts. R0-silent Changes should not be mentioned in any release notes labels Dec 18, 2024
@pgherveou pgherveou requested review from athei and xermicus and removed request for athei January 9, 2025 15:55
@smiasojed
Copy link
Contributor

Now I have the option to set the gas limit for a ETH transaction either manually or by estimating it (dry-run). Will both option work after this change?

prdoc/pr_6689.prdoc Outdated Show resolved Hide resolved
- audience: Runtime Dev
description: |-
Update the current approach to attach the `ref_time`, `pov` and `deposit` parameters to an Ethereum transaction.
Previously we will pass these 3 parameters along with the signed payload, and check that the fees resulting from `gas x gas_price` match the actual fees paid by the user for the extrinsic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously, these three parameters were passed along with the signed payload, and the fees resulting from gas × gas_price were checked to ensure they matched the actual fees paid by the user for the extrinsic ?

@pgherveou
Copy link
Contributor Author

Now I have the option to set the gas limit for a ETH transaction either manually or by estimating it (dry-run). Will both option work after this change?

You can but you need to setting sufficient values for the 6 lowers digits that encode the weights and deposit

prdoc/pr_6689.prdoc Outdated Show resolved Hide resolved
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12712453563
Failed job name: test-linux-stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
Status: Minimal Feature Launch
Development

Successfully merging this pull request may close these issues.

4 participants