-
Notifications
You must be signed in to change notification settings - Fork 641
Add EIP-7910: Add eth_config #8956
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
Conversation
emlautarom1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before approving we should add a couple of test cases, specifically related to serialization to ensure that the CRC-32 hashes are correct
it will be provided by hive. I mean, it's hard to figure out what's expected and it will anyway repeat hive tests, so I'd wait for confirmation using the external tests |
|
|
||
| public static Address Address { get; } = Address.FromNumber(7); | ||
|
|
||
| public static string Name => "BN256_MUL"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why BN256?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply due to unknown reason. This name is taken from https://eips.ethereum.org/EIPS/eip-7910
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So nothing will break if I change to BN254. It's a confusing mess everywhere with this naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will break hash function and therefore the entire purpose of eth_config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(unless you change both https://eips.ethereum.org/EIPS/eip-7910 and our code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this damn naming is coming from EIP. Missed that. So we check for Bn128... to add BN254... classes which return BN256... 😁 Damn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
((we provide hashes of fork configs, so fields order, fields names should be fully the same, and no whitespace))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, as always naming is the hardest problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(unless you change both https://eips.ethereum.org/EIPS/eip-7910 and our code)
I took it seriously: #9023 😄
* Update CLZ gas cost for devnet3 (#8953) * Update tx gas cap for devnet3 (#8954) * Don't block background tasks when syncing (#8971) * Don't block background tasks when syncing * fix test * fix compile warning * Concurrency 2 * Rename blob metrics (#8972) * Fix Taiko Engine Api (#8974) * Better null-ckecks in TryGetCanonicalTransaction (#8969) * Better null-ckecks in TryGetCanonicalTransaction * fix test * fix whitespace * fix test * Revise JSON-RPC docs generation (#8967) * Fix/OOM when downloading from genesis. (#8975) * Slight change in constructor * Stop putting more request when queue is high * Add unit test * Set budget as memory * Whitespace * Fix test * Cache PropertyInfo lookups for JsonRpc (#8976) * Auto-update fast sync settings (#8978) Co-authored-by: rubo <rubo@users.noreply.github.com> * Update OP Superchain chains (#8979) Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com> * Remove use of Linq.Sum from hot paths (#8977) * Remove use of Linq.Sum from hot paths * Simplify * Isolate Tracer in Proof Module and remove IVisitingWorldState (#8981) * Scope the tracer * Use state reader in Tracer * Inline dump state * Remove accept * Delete IVisitingWorldState * Move dump state to state reader * Whitespace * More hot code in ConnectNodes (#8982) * Only access "warmup" hashtable once per warmup (#8983) * Only access "warm" up hashtable once per warmup * Update src/Nethermind/Nethermind.Evm/Instructions/EvmInstructions.cs Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> --------- Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> * EIP-7594: Constant maxBlobsPerTx (#8940) * Const maxBlobsPerTx * WS * Add tests(improve for the Gnosis case); add link * Fail fast on Db corruption (#8986) * Fail fast on Db corruption * Fix tests * Update src/Nethermind/Nethermind.Db.Rocks/DbOnTheRocks.cs Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com> --------- Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com> * Configure EWC Zurich hard fork (#8985) * feat: Add RegexOptions.Compiled flag to NewPayloadJsonRpcValidator (#8984) * Improve locking when looking up RlpDecoders (#8987) * Remove locking in RlpDecoder fast path * Feedback * Fix naming of data cost in ExecutePrecompile (#8994) fix naming of data cost * Move precompiles (#8962) * Move precompiles * Fix build * Fix formating * EthereumCodeInfoRepository * Remove EthereumPrecompiles class * Move precompiles into separate project * Fix build * Add EIP-7910: Add eth_config (#8956) * Add eth_config * Update systemContracts, refactor * Fix next/last hash to be null * Replace `eth_pairings` with `mcl` (#8992) * Unify BN254 naming (#9004) * Refactor/Move init DB to DI (#8997) * Wait to read channel before reading * Fix BN254 point deserialization (#9009) * Optimize ModExp (#9008) * Optimize ModExp * Faaster * Moar * work as nuint * use nint * cast via int * Remove unneeded casts * Move OldRuns to tests * fix benchmarks * Feedback * Less branches * Less branches * fixed invalid * Remove last orphan readonly tx processing scope. (#9005) * Fix workerloop * Replace read all with try read * Update config for tests * Update winget command (#9011) * Auto-update fast sync settings (#9013) Co-authored-by: rubo <rubo@users.noreply.github.com> * Update OP Superchain chains (#9014) Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com> * Tidy up ecdsa overloads (#9015) * Tidy up ecdsa overloads * put back the increase * Tracing check to allow ilevm to be enabled for node and tests * Update verified * .. --------- Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> Co-authored-by: Alexey <me@flcl.me> Co-authored-by: Amirul Ashraf <asdacap@gmail.com> Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com> Co-authored-by: core-repository-dispatch-app[bot] <173070810+core-repository-dispatch-app[bot]@users.noreply.github.com> Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com> Co-authored-by: Micke <155267459+reallesee@users.noreply.github.com> Co-authored-by: Marcin Sobczak <77129288+marcindsobczak@users.noreply.github.com> Co-authored-by: Nikita Mescheryakov <root@nikitam.io>
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing