Fix eth_sumulate defaults by deffrian · Pull Request #9529 · NethermindEth/nethermind

deffrian · 2025-10-21T19:41:26Z

Fixes Closes Resolves #

eth_simulateV1/ethSimulate-simple-more-params-validate (nethermind)
eth_estimateGas/estimate-with-eip4844 (nethermind)

Changes

Adjust tx type to geth

Types of changes

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

This reverts commit 9eb30e0.

LukaszRozmej

Make sure the changes will be only for eth_simulate and not other endpoints

src/Nethermind/Nethermind.Facade/Eth/RpcTransaction/BlobTransactionForRpc.cs

LukaszRozmej · 2025-10-29T20:02:09Z

src/Nethermind/Nethermind.Facade/Eth/RpcTransaction/TransactionForRpc.cs

+            return (TransactionForRpc?)JsonSerializer.Deserialize(ref reader, concreteTxType, options);
+        }
+
+        private Type DeriveTxType(JsonObject untyped, JsonSerializerOptions options)


Suggested change

private Type DeriveTxType(JsonObject untyped, JsonSerializerOptions options)

private Type DeriveTxType(JsonObject untyped)

LukaszRozmej · 2025-10-29T20:06:47Z

src/Nethermind/Nethermind.Facade/Eth/RpcTransaction/TransactionForRpc.cs

+            return _txTypes
+                .FirstOrDefault(p => p.TxType == defaultTxType || p.DiscriminatorProperties.Any(untyped.ContainsKey),
+                    _txTypes[^1]).Type;


I don't get it...

Won't this often default to defaultTxType and it is based on order?

I would expect something like below:

Suggested change

return _txTypes

.FirstOrDefault(p => p.TxType == defaultTxType || p.DiscriminatorProperties.Any(untyped.ContainsKey),

_txTypes[^1]).Type;

return _txTypes.FirstOrDefault(p => p.DiscriminatorProperties.Any(untyped.ContainsKey))?.Type ?? defaultTxType;

LukaszRozmej · 2025-10-29T22:33:32Z

src/Nethermind/Nethermind.Facade/Eth/RpcTransaction/TransactionForRpc.cs

+            if (untyped.ContainsKey(gasPriceFieldKey))
+            {
+                return typeof(LegacyTransactionForRpc);
+            }


If Legacy is first item in _txTypes we can probably drop this if?

I think it's last. So we should return Legacy if gasPrice is set. Even if we have blobHashes

Ok we have:

// Adding in reverse order so newer tx types are in priority

Interesting, maybe we should do that ignoring index?

So:

int indexOfPreviousTxType = _txTypes.FindIndex(t => t.TxType != TxType.Legacy && t.TxType < typeInfo.TxType); ?

I don't understand what's typeInfo.TxType. I think now it's very straight-forward

* Reduce peer logging (#9591) * Cancel timeout in NewPayload fast-path (#9597) * Refactor JsonRpcService.ExecuteAsync (#9600) * Refactor JsonRpcService.ExecuteAsync * Feedback * Add reflection cache * Optimize DeserializeParameters * Optimize enumeration * Can be made static * Remove unused using * Use regular Dictionary * formatting * Optimise CREATE(2) remove redundant account creation (#9585) * delay creation of executor account * store nonce zero for new executor * remove account creation --------- Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> * Replace Task.Delay/Thread.Sleep with NUnit .After() in tests (#9576) * Initial plan * Convert Task.Delay and Thread.Sleep to After pattern in tests Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Fix async method signatures after removing await Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Add NSubstituteExtensions and refactor to use ReceivedBool helper Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * fix * Try avoiding exceptions * increase Timeout * change * fix * whitespace * make Big_test Explicit * fix negative paths * revert one test * Revert inappropriate After() conversions for blocking tests Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> * Remove redundant diagnostic messages from ReceivedCallsMatching Co-authored-by: flcl42 <630501+flcl42@users.noreply.github.com> * Display diagnostic message only once per test when ReceivedCallsMatching fails Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Remove diagnostic logging from ReceivedCallsMatching to avoid issues Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * remove stale annotation --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com> Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> Co-authored-by: flcl42 <630501+flcl42@users.noreply.github.com> * Arbitrum - Refund on contract creation failure (#9610) * extension for arbitrum on refund at contract deployment failure * Fix eth_sumulate defaults (#9529) * Fix * Refactor * Test * Fix taiko * Revert "Test" This reverts commit 9eb30e0. * Fix * Fix gas * Test * Fix validate flag * Fix taiko * Fix gasEstimation tests * Add more flags * Fix tests * Change flags * Try fix tests * Fix tests * Proper nonce validation * More fixes * Revert * Fix tests * Fix types * Add two tests * Fix known-failing-tests * Default type * known failing tests * Fix suggestions * Fix tests * Extra peers reporting for supported chains runs (#9612) * Fix StatelessExecution tool build (#9613) * Fix/pow forward header provider cache ownership (#9614) * Fix: return copy from cache to avoid use-after-dispose * add a test * Update EthereumTests (#9225) * Update tests to fix missing test jsons * Use newer tests; add more moern legacy tests; fix transition tests; fix slnx * Rename projects; remove Blockchain tests as it was moved to legacy fully * Fix a csproj * Add tests * Add tests * Move transaition tests to legacy too; simplify namespaces * Fix workflow * Fix syntax * Fix transaction tests * Add empty coinbase * Add coinbase even if transaction is declined * Fix evm slnx compilation * Can be failing cause? * Return blockhash tests * Rollback coinbase set for blockchain tests * Less tests as old one have broken format * Rm TODO, as SuicideStorage tests are covered by state tests * Fix * Update to 5.0.0 * Remove redundant length==1 special-case in SliceWithZeroPadding (#9566) * Catch index exceptions in RlpDecoders and pack it in RlpException (#9603) * Catch index exceptions in RlpDecoders and pack it in RlpException * Catch index exceptions in RlpDecoders and pack it in RlpException (#9604) * Initial plan * Update RLP decoders to inherit from base classes with exception handling Updated all RLP decoder classes to inherit from RlpValueDecoder<T> or RlpStreamDecoder<T> instead of directly implementing interfaces. This ensures all decoders automatically get IndexOutOfRangeException and ArgumentOutOfRangeException handling, wrapping them in RlpException. Changed decoders: - WithdrawalDecoder, KeccakDecoder, LogEntryDecoder - AuthorizationTupleDecoder, AccessListDecoder - AccountDecoder, HeaderDecoder, BlockDecoder, BlockBodyDecoder - ChainLevelDecoder, BlockInfoDecoder - ReceiptMessageDecoder, ReceiptStorageDecoder, CompactReceiptStorageDecoder - ReceiptArrayStorageDecoder All decode methods renamed to DecodeInternal and marked as protected override. All Encode/GetLength methods marked as override where needed. Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Update remaining RLP decoders across all modules Updated RLP decoders in Consensus.Clique, Network, Consensus.AuRa, Taiko, and Xdc modules to inherit from base classes: - Consensus.Clique: SnapshotDecoder - Network: NetworkNodeDecoder - Consensus.AuRa: ValidatorInfoDecoder, PendingValidatorsDecoder - Taiko: L1OriginDecoder - Xdc: ExtraConsensusDataDecoder, QuorumCertificateDecoder, SnapshotDecoder, TimeoutCertificateDecoder, TimeoutDecoder, VoteDecoder, XdcBlockInfoDecoder All decoders now inherit from RlpValueDecoder<T> or RlpStreamDecoder<T> to get automatic exception handling for IndexOutOfRangeException and ArgumentOutOfRangeException. Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Mark all RLP decoder classes as sealed Added sealed modifier to all 27 RLP decoder classes to prevent further inheritance: - Core decoders (15): WithdrawalDecoder, KeccakDecoder, LogEntryDecoder, AccountDecoder, HeaderDecoder, BlockDecoder, BlockBodyDecoder, ChainLevelDecoder, BlockInfoDecoder, ReceiptMessageDecoder, ReceiptStorageDecoder, CompactReceiptStorageDecoder, ReceiptArrayStorageDecoder, AuthorizationTupleDecoder, AccessListDecoder - Consensus decoders (3): SnapshotDecoder (Clique), ValidatorInfoDecoder (AuRa), PendingValidatorsDecoder (AuRa) - Network decoder (1): NetworkNodeDecoder - Taiko decoder (1): L1OriginDecoder - Xdc decoders (7): ExtraConsensusDataDecoder, QuorumCertificateDecoder, SnapshotDecoder, TimeoutCertificateDecoder, TimeoutDecoder, VoteDecoder, XdcBlockInfoDecoder These classes are not inherited from and marking them as sealed provides better performance and clearer design intent. Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Update remaining RLP decoders: BasicStreamDecoder, TxDecoder, and OptimismReceiptMessageDecoder Updated the last remaining decoder implementations to inherit from base classes: - BasicStreamDecoder.cs: Updated 6 primitive type decoders (ByteStreamDecoder, ShortStreamDecoder, UShortStreamDecoder, IntStreamDecoder, UIntStreamDecoder, ULongStreamDecoder) to inherit from RlpStreamDecoder<T> and marked as sealed - TxDecoder.cs: Updated TxDecoder<T> base class to inherit from RlpValueDecoder<T> (not sealed as it has subclasses: TxDecoder, SystemTxDecoder, GeneratedTxDecoder) - OptimismReceiptMessageDecoder.cs: Updated OptimismReceiptMessageDecoder to inherit from RlpStreamDecoder<TxReceipt> (not sealed as it has subclass OptimismReceiptTrieDecoder) All decoders now have automatic exception handling for IndexOutOfRangeException and ArgumentOutOfRangeException. Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Fix TxDecoder: Add back Rlp Encode method for test compatibility Added the `Rlp Encode(T item, RlpBehaviors)` method that was removed during refactoring. This method is used by tests and provides a convenient way to encode transactions to Rlp without manually creating a RlpStream. The method internally uses the override Encode method to ensure exception handling is still applied. Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * fix build --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Add more logging in MultiSyncModeSelector (#9616) * Add more logging * fix for seq * feat: Add configurable EIP-2935 ring buffer size (#9611) * Blockchain Engine Tests support (#9394) * initial commit * fix normal blockchain tests * tidy * restore disposes * comment out BALs * fix var declaration * don't set basefeepergas if null * use network from genesis in blockchain test * update blockchain test base * add tracer to blockchain tests runner * tidy * tidy * add genesis processing timeout * check for null head block * try undo some changes * detect failure to process genesis * check removal is error * add back checks for genesis spec * only add noenginerequeststracker in tests * comment sealed block check * try remove timeout * only configure merge for engine tests * fix merge module init * add back timeout and remove sealer * await new payloads * use reflection for engine rpc method calling --------- Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> * use zero address when from address not specified in rpc calls (#9578) * use zero address for null values * small test * fix proof rpc * fix test and add more changes * Allow serving snap requests for more than 128 blocks (#9602) * Initial plan * Add SnapServingMaxDepth configuration and update LastNStateRootTracker Co-authored-by: tanishqjasoria <11698398+tanishqjasoria@users.noreply.github.com> * Add clarifying comments for configuration changes Co-authored-by: tanishqjasoria <11698398+tanishqjasoria@users.noreply.github.com> * Get reorgDepth from config instead of hardcoding in test Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tanishqjasoria <11698398+tanishqjasoria@users.noreply.github.com> Co-authored-by: Tanishq Jasoria <jasoriatanishq@gmail.com> Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> * Remove console log from FileTestsSource (#9622) Removed console log for loading test file. * Correct docs value for Blocks.BlockProductionMaxTxKilobytes (#9620) * Update OP Superchain chains (#9629) Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com> * Auto-update fast sync settings (#9628) Co-authored-by: rubo <rubo@users.noreply.github.com> * feat: write AckMessage directly to IByteBuffer without temp array (#9623) * Optimize Ripemd (#9627) --------- Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> Co-authored-by: Marc <Marchhill@users.noreply.github.com> Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: LukaszRozmej <12445221+LukaszRozmej@users.noreply.github.com> Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> Co-authored-by: flcl42 <630501+flcl42@users.noreply.github.com> Co-authored-by: Stavros Vlachakis <89769224+svlachakis@users.noreply.github.com> Co-authored-by: Nikita Mescheryakov <root@nikitam.io> Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com> Co-authored-by: radik878 <radikpadik76@gmail.com> Co-authored-by: Alexey Osipov <me@flcl.me> Co-authored-by: GarmashAlex <garmasholeksii@gmail.com> Co-authored-by: Daniil Ankushin <ankushin.daniil42@gmail.com> Co-authored-by: Tanishq Jasoria <jasoriatanishq@gmail.com> Co-authored-by: tanishqjasoria <11698398+tanishqjasoria@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: VolodymyrBg <aqdrgg19@gmail.com>

deffrian added 4 commits October 21, 2025 20:44

Fix

9d042d7

Refactor

e47ae97

Test

9eb30e0

Fix taiko

0298967

deffrian changed the title ~~Charge gas even if no validation & fix defaults~~ [WIP]Charge gas even if no validation & fix defaults Oct 21, 2025

deffrian added 16 commits October 22, 2025 18:05

Revert "Test"

2ad47be

This reverts commit 9eb30e0.

Fix

c9a1c0e

Fix gas

fd184c2

Test

5ac4885

Fix validate flag

28b312e

Fix taiko

de3bec1

Fix gasEstimation tests

87848f5

Add more flags

c35fd66

Fix tests

3880ccb

Change flags

183533f

Try fix tests

8a8a68f

Fix tests

82fed87

Proper nonce validation

7c16b39

More fixes

8359e29

Revert

cebcfe7

Fix tests

81e2809

deffrian changed the title ~~[WIP]Charge gas even if no validation & fix defaults~~ Fix eth_sumulate defaults Oct 23, 2025

deffrian added 3 commits October 24, 2025 02:19

Fix types

41e052f

Merge branch 'master' into fix/simulate-defaults

4440723

Add two tests

d09a5a2

deffrian marked this pull request as ready for review October 23, 2025 23:35

deffrian requested review from LukaszRozmej, benaadams, rubo and smartprogrammer93 as code owners October 23, 2025 23:35

deffrian requested a review from a team October 23, 2025 23:35

LukaszRozmej requested changes Oct 24, 2025

View reviewed changes

src/Nethermind/Nethermind.Facade/Eth/RpcTransaction/BlobTransactionForRpc.cs Show resolved Hide resolved

deffrian added 3 commits October 26, 2025 20:58

Merge branch 'master' into fix/simulate-defaults

b52404b

Fix known-failing-tests

3400d7d

Merge branch 'master' into fix/simulate-defaults

a6b8d17

LukaszRozmej approved these changes Oct 28, 2025

View reviewed changes

Default type

0405ef0

smartprogrammer93 approved these changes Oct 29, 2025

View reviewed changes

known failing tests

300b187

LukaszRozmej reviewed Oct 29, 2025

View reviewed changes

Fix suggestions

6afef1b

LukaszRozmej reviewed Oct 29, 2025

View reviewed changes

LukaszRozmej approved these changes Oct 29, 2025

View reviewed changes

Fix tests

333f81c

deffrian merged commit 489700f into master Oct 30, 2025
141 of 146 checks passed

deffrian deleted the fix/simulate-defaults branch October 30, 2025 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix eth_sumulate defaults#9529

Fix eth_sumulate defaults#9529
deffrian merged 30 commits intomasterfrom
fix/simulate-defaults

deffrian commented Oct 21, 2025 •

edited

Loading

Uh oh!

LukaszRozmej left a comment

Uh oh!

Uh oh!

LukaszRozmej Oct 29, 2025

Uh oh!

LukaszRozmej Oct 29, 2025 •

edited

Loading

Uh oh!

LukaszRozmej Oct 29, 2025

Uh oh!

deffrian Oct 29, 2025

Uh oh!

LukaszRozmej Oct 29, 2025

Uh oh!

deffrian Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	private Type DeriveTxType(JsonObject untyped, JsonSerializerOptions options)
	private Type DeriveTxType(JsonObject untyped)

Conversation

deffrian commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Types of changes

What types of changes does your code introduce?

Uh oh!

LukaszRozmej left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LukaszRozmej Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

deffrian Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

deffrian Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

deffrian commented Oct 21, 2025 •

edited

Loading

LukaszRozmej Oct 29, 2025 •

edited

Loading