eip 7840 implementation #7964

yerke26 · 2024-12-23T11:27:11Z

Closes #9129

What types of changes does your code introduce?

Bugfix (a non-breaking change that fixes an issue)
New feature (a non-breaking change that adds functionality)
Breaking change (a change that causes existing functionality not to work as expected)
Optimization
Refactoring
Documentation update
Build-related changes
Other: Description

Testing

Requires testing

Yes
No

If yes, did you write tests?

Yes
No

Notes on testing

Optional. Remove if not applicable.

Documentation

Requires documentation update

Yes
No

If yes, link the PR to the docs update or the issue with the details labeled docs. Remove if not applicable.

Requires explanation in Release Notes

Yes
No

If yes, fill in the details here. Remove if not applicable.

Remarks

Optional. Remove if not applicable.

smartprogrammer93

looks good, but please add tests.

smartprogrammer93

Also, as discussed. i dont think we need the EIP7840 enabled. we can just replace the constants with whatever we push in the chainspec for the cancun through the same mechanism

src/Nethermind/Nethermind.Core/Specs/ReleaseSpecDecorator.cs

# Conflicts: # src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainParameters.cs

src/Nethermind/Nethermind.Core/Specs/ReleaseSpecDecorator.cs

src/Nethermind/Nethermind.Specs.Test/OverridableReleaseSpec.cs

src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecBasedSpecProvider.cs

src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecLoader.cs

src/Nethermind/Nethermind.Blockchain.Test/TransactionSelectorTests.cs

src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs

src/Nethermind/Nethermind.Blockchain.Test/Validators/ShardBlobBlockValidatorTests.cs

src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs

src/Nethermind/Nethermind.Core/Specs/ReleaseSpecDecorator.cs

src/Nethermind/Nethermind.Evm.Test/TransactionProcessorEip4844Tests.cs

src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs

…into eip-7840

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/PooledTxsRequestor.cs

tanishqjasoria · 2025-01-08T07:05:53Z

tools/SendBlobs/BlobSender.cs

@@ -282,10 +283,10 @@ public async Task SendData(

        if (defaultMaxFeePerBlobGas is null)
        {
-            ulong excessBlobsReserve = 2 * Eip4844Constants.TargetBlobGasPerBlock;
+            ulong excessBlobsReserve = 2 * Cancun.Instance.TargetBlobCount;


we might need to change this

maybe we add spec as command line param to this tool

src/Nethermind/Nethermind.Consensus/Messages/BlockErrorMessages.cs

src/Nethermind/Nethermind.Consensus/Messages/TxErrorMessages.cs

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/PooledTxsRequestor.cs

yerke26 · 2025-01-09T04:16:03Z

src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecBasedSpecProvider.cs

+                spec.TargetBlobCount = 3;
+                spec.MaxBlobCount = 6;


I think they should be zero as written here

yes, according to the EIP it should be zero

changed back to zero

@MarekM25 did this i think to fix hive tests. For hive, i think we should specifiy them in its chainspec

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/PooledTxsRequestor.cs

src/Nethermind/Chains/gnosis.json

LukaszRozmej · 2025-01-14T07:40:55Z

src/Nethermind/Chains/chiado.json

-    "terminalTotalDifficulty": "231707791542740786049188744689299064356246512"
+    "terminalTotalDifficulty": "231707791542740786049188744689299064356246512",
+    "blobSchedule": {
+      "cancun": {


Should it be by fork name? Normally we are using timestamps now to mark changes.

yes, as per EIp it should be by fork name

Co-authored-by: yerke26 <[email protected]>

…into eip-7840

MarekM25 · 2025-01-15T12:09:19Z

src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs

    {
        int blobCount = transaction.BlobVersionedHashes!.Length;
        ulong totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount);
-        return totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction ? TxErrorMessages.BlobTxGasLimitExceeded
+        // maxBlobGasPerTxn = maxBlobGasPerBlock
+        var maxBlobGasPerTxn = spec.MaxBlobCount * Eip4844Constants.GasPerBlob;


spec.MaxBlobCount * Eip4844Constants.GasPerBlob is a pattern repeated in many places. @yerke26, you could consider creating an extension method in ReleaseSpecExtensions (create the class if it doesn't exist) and adding methods like MaxBlobGas => spec.MaxBlobCount * Eip4844Constants.GasPerBlob. This approach will make the code more elegant and eliminate the need to calculate it in multiple places

or even you can think of method that take blobCount as an input and return if it is valid or not. The same pattern I have seen in BlockValidator

MarekM25 · 2025-01-16T19:26:54Z

src/Nethermind/Chains/foundation.json

+      "prague": {
+        "target": 6,
+        "max": 9,
+        "baseFeeUpdateFraction":  5007716


mixed hex (chiado) and decimals

MarekM25 · 2025-01-16T19:28:47Z

src/Nethermind/Nethermind.Consensus/Producers/TxPoolTxSource.cs

                {
                    if (_logger.IsTrace) _logger.Trace($"Declining {blobTx.ToShortString()}, no more blob space. Block already have {blobGasCounter} blob gas which is max value allowed.");
                    break;
                }

                checkedBlobTransactions++;

-                ulong txBlobGas = (ulong)(blobTx.BlobVersionedHashes?.Length ?? 0) * _eip4844Config.GasPerBlob;
-                if (txBlobGas > _eip4844Config.MaxBlobGasPerBlock - blobGasCounter)
+                ulong txBlobGas = (ulong)(blobTx.BlobVersionedHashes?.Length ?? 0) * Eip4844Constants.GasPerBlob;


MarekM25 · 2025-01-16T19:31:02Z

src/Nethermind/Nethermind.Evm/BlobGasCalculator.cs

@@ -102,8 +102,8 @@ static bool FakeExponentialOverflow(UInt256 factor, UInt256 num, UInt256 denomin

        ulong excessBlobGas = parentBlockHeader.ExcessBlobGas ?? 0;
        excessBlobGas += parentBlockHeader.BlobGasUsed ?? 0;
-        return excessBlobGas < Eip4844Constants.TargetBlobGasPerBlock
+        return excessBlobGas < releaseSpec.TargetBlobCount * Eip4844Constants.GasPerBlob


MarekM25 · 2025-01-16T19:35:57Z

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/PooledTxsRequestor.cs

+            ulong maxBlobCount = specProvider.TransitionActivations
+                .Select(transitionActivation => specProvider.GetSpec(transitionActivation).MaxBlobCount)
+                .DefaultIfEmpty(0ul)
+                .Max();


Should we use head here?

MarekM25 · 2025-01-16T19:38:15Z

src/bench_precompiles

add MaxBlobCount, TargetBlobCount to chainSpec

c38a641

yerke26 requested a review from smartprogrammer93 December 23, 2024 11:27

Change FeeOracleHistory to use right MaxBlobCount

10342cc

smartprogrammer93 requested changes Dec 23, 2024

View reviewed changes

smartprogrammer93 reviewed Dec 23, 2024

View reviewed changes

fix BlobGasUsedRatio

5ef5d36

LukaszRozmej reviewed Dec 30, 2024

View reviewed changes

src/Nethermind/Nethermind.Core/Specs/ReleaseSpecDecorator.cs Outdated Show resolved Hide resolved

yerke26 requested a review from rubo as a code owner December 30, 2024 12:02

yerke26 added 3 commits December 30, 2024 20:02

remove TargetBlobCount, MaxBlobCount constants

6f45bc6

add PragueTransitionTimestamp

0035b43

Merge branch 'master' into eip-7840

d68374c

# Conflicts: # src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainParameters.cs

MarekM25 reviewed Jan 6, 2025

View reviewed changes

yerke26 added 10 commits January 7, 2025 11:11

remove PragueTransitionTimestamp, instead use Eip7002TransitionTimestamp

b668cdf

remove Eip4844MaxBlobCount, Eip4844TargetBlobCount from chainSpecParams

eb77784

fix TxValidatorTests

b8bef08

fix TxValidatorTests

7059480

fix CancunStateTests

b638d63

fix TransactionProcessorEip4844Tests

0d5a076

fix TxPoolTests

91f1b1f

fix tests

e494aa9

fix FeeHistoryOracleTests

44481ba

fix whitespaces

05772a0

tanishqjasoria requested changes Jan 7, 2025

View reviewed changes

MarekM25 and others added 7 commits January 7, 2025 13:25

Merge branch 'master' into eip-7840

b6b50b7

remove hardcoded values

09a914d

Fix build errors

1d271bd

Merge branch 'eip-7840' of https://github.com/nethermindeth/nethermind …

87acb12

…into eip-7840

Fix OverridableReleaseSpec

0aa4346

remove hardcoded values

87fc57d

add blobSchedule to chainSpec

f28e72a

tanishqjasoria and others added 4 commits January 8, 2025 11:54

fix fee history oracle pre cancun

22b7d1d

fix gnosis and chiado specs

4fbd890

Merge branch 'master' into eip-7840

6952229

fix whitespaces

5b4c0d1

yerke26 requested a review from smartprogrammer93 January 8, 2025 06:55

tanishqjasoria reviewed Jan 8, 2025

View reviewed changes

yerke26 requested review from LukaszRozmej and MarekM25 January 8, 2025 08:47

Merge branch 'master' into eip-7840

cb445ee

smartprogrammer93 approved these changes Jan 8, 2025

View reviewed changes

src/Nethermind/Nethermind.Consensus/Messages/BlockErrorMessages.cs Show resolved Hide resolved

src/Nethermind/Nethermind.Consensus/Messages/TxErrorMessages.cs Show resolved Hide resolved

smartprogrammer93 reviewed Jan 8, 2025

View reviewed changes

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/PooledTxsRequestor.cs Outdated Show resolved Hide resolved

MarekM25 and others added 2 commits January 8, 2025 19:27

fix default blobCount and targetCount

e181b43

fix PooledTxsRequestor to use spec at current head

43e1655

yerke26 commented Jan 9, 2025

View reviewed changes

tanishqjasoria reviewed Jan 9, 2025

View reviewed changes

src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/PooledTxsRequestor.cs Outdated Show resolved Hide resolved

yerke26 and others added 7 commits January 9, 2025 13:00

fix PooledTxsRequestorTests

0c309ea

CalculateMaxBlobTxSize through transitionActivations

c2737fc

set default TargetBlobCount, MaxBlobCount to zero

362f18c

fix defaults

2f1f6f2

set default value for gnosisChain

43bda5a

add Eip7840DefaultBlobCountFraction for gnosis chain

1ecf556

fix gnosis, chiado chains json

3872a10

MarekM25 reviewed Jan 10, 2025

View reviewed changes

src/Nethermind/Chains/gnosis.json Outdated Show resolved Hide resolved

revert default value changes

58a3ff9

LukaszRozmej reviewed Jan 14, 2025

View reviewed changes

tanishqjasoria and others added 4 commits January 15, 2025 12:07

Merge branch 'master' into eip-7840

a528f1e

add blob price update fraction (#8053)

f36e6c4

Co-authored-by: yerke26 <[email protected]>

cosmetic

78910ec

Merge branch 'eip-7840' of https://github.com/nethermindeth/nethermind …

b288921

…into eip-7840

MarekM25 reviewed Jan 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eip 7840 implementation #7964

eip 7840 implementation #7964

yerke26 commented Dec 23, 2024

smartprogrammer93 left a comment

smartprogrammer93 left a comment

tanishqjasoria Jan 8, 2025

yerke26 Jan 8, 2025

yerke26 Jan 9, 2025

tanishqjasoria Jan 9, 2025

yerke26 Jan 9, 2025

smartprogrammer93 Jan 9, 2025

LukaszRozmej Jan 14, 2025

yerke26 Jan 14, 2025

MarekM25 Jan 15, 2025

MarekM25 Jan 15, 2025

MarekM25 Jan 16, 2025

MarekM25 Jan 16, 2025

MarekM25 Jan 16, 2025

MarekM25 Jan 16, 2025

MarekM25 Jan 16, 2025

eip 7840 implementation #7964

Are you sure you want to change the base?

eip 7840 implementation #7964

Conversation

yerke26 commented Dec 23, 2024

What types of changes does your code introduce?

Testing

Requires testing

If yes, did you write tests?

Notes on testing

Documentation

Requires documentation update

Requires explanation in Release Notes

Remarks

smartprogrammer93 left a comment

Choose a reason for hiding this comment

smartprogrammer93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment