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

[4844] Fix Data Gas Price Calculation #5680

Conversation

Gabriel-Trintinalia
Copy link
Contributor

@Gabriel-Trintinalia Gabriel-Trintinalia commented Jul 6, 2023

PR description

This pull request addresses two issues: fixing the data gas price calculation and adjusting the genesis state methods to verify forks. The details of each issue are described below:

Data Gas Price Calculation

The previous implementation of data gas price calculation had a flaw. It only considered the parent's excessDataGas to determine the dataPricePerGas, which overlooked the data gas used by the parent header itself. Consequently, when a large number of transactions were sent and the gas price increased, blocks were generated with transactions that should not have been included due to their price.

To rectify this, the calculation now incorporates both the parent's excessDataGas and the data gas used by the parent header. The code snippet from the specification is provided below:

def get_data_gasprice(header: Header) -> int:
    return fake_exponential(
        MIN_DATA_GASPRICE,
        header.excess_data_gas,
        DATA_GASPRICE_UPDATE_FRACTION
    )

This modification ensures that the assertion in block creation and transaction processing considers the correct data gas price, preventing inclusion of transactions with insufficient data gas price.

Genesis State Verification for Cancun Fork

For the Cancun fork, there was an issue with the genesis state not including WithdrawsRoot in the genesis block. The problematic code snippet was as follows:

return shanghaiTimestamp.getAsLong() == genesis.getTimestamp();
To address this issue, the code has been updated as follows:

return genesis.getTimestamp() >= shanghaiTimestamp.getAsLong();

This change ensures that the genesis block's timestamp is greater than or equal to the shanghaiTimestamp, correctly reflecting the inclusion of WithdrawsRoot in the genesis state for the Cancun fork.

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
@Gabriel-Trintinalia Gabriel-Trintinalia marked this pull request as ready for review July 6, 2023 13:15
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
@jflo jflo merged commit 2299ce6 into hyperledger:4844-devnet-5b Jul 6, 2023
5 checks passed
jflo pushed a commit to jflo/besu that referenced this pull request Jul 20, 2023
* [4844] Fix Data Gas Price Calculation (#1)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Fix spotless

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Fix mock unit tests

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

---------

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
@Gabriel-Trintinalia Gabriel-Trintinalia deleted the fix-data-gas-price-calculation-and-gengesis-forks-fix branch September 20, 2023 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants