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

Gas price to substract to sender in up-front cost #1479

Open
maximopalopoli opened this issue Dec 11, 2024 · 1 comment
Open

Gas price to substract to sender in up-front cost #1479

maximopalopoli opened this issue Dec 11, 2024 · 1 comment
Labels
ef-tests Hive tests, execution-spec-tests levm Lambda EVM implementation

Comments

@maximopalopoli
Copy link
Contributor

Currently, in our evm implementation when we get the up-front cost from the sender, we get it using the max_fee_per_gas_or_gasprice function, which takes the max_fee_per_gas, and if there is none, the gas price is used. That is used here:

        // (1) GASLIMIT_PRICE_PRODUCT_OVERFLOW
        let gaslimit_price_product = self
            .max_fee_per_gas_or_gasprice()
            .checked_mul(self.env.gas_limit)
            .ok_or(VMError::TxValidation(
                TxValidationError::GasLimitPriceProductOverflow,
            ))?;

But, when returning the not consumed gas to the user, the max_fee_per_gas_or_gasprice function is not used but the gas price:

        let gas_return_amount = self
            .env
            .gas_price
            .low_u64()
            .checked_mul(gas_to_return)
            .ok_or(VMError::Internal(InternalError::UndefinedState(1)))?;

I tried to change this by using max_fee_per_gas_or_gasprice in the returning of the gas to the user but the change does not make pass more tests than before.

The test being debugged is coinbaseT2.json from the stEIP2930 folder. I also tried using gas_price instead of max_fee_per_gas_or_gasprice when substracting the up-front cost at the beggining, and that makes this and other tests pass, but in the overall there are less tests passing. The diff summary is the following:

Without the change:

stEIP1153-transientStorage: 0/8 (0.00%)
stExample: 8/12 (66.67%)
stEIP2930: 5/7 (71.43%)
eip4844_blobs: 229/559 (40.97%)


With the change: 

stEIP1153-transientStorage: 5/8 (62.50%)
stExample: 11/12 (91.67%)
stEIP2930: 6/7 (85.71%)
eip4844_blobs: 215/559 (38.46%)
@maximopalopoli maximopalopoli added ef-tests Hive tests, execution-spec-tests levm Lambda EVM implementation labels Dec 11, 2024
@JereSalo
Copy link
Contributor

Good issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ef-tests Hive tests, execution-spec-tests levm Lambda EVM implementation
Projects
None yet
Development

No branches or pull requests

2 participants