Skip to content

Catch index exceptions in RlpDecoders and pack it in RlpException#9603

Merged
LukaszRozmej merged 3 commits intomasterfrom
fix/rlp-packs-exceptions
Oct 31, 2025
Merged

Catch index exceptions in RlpDecoders and pack it in RlpException#9603
LukaszRozmej merged 3 commits intomasterfrom
fix/rlp-packs-exceptions

Conversation

@LukaszRozmej
Copy link
Copy Markdown
Member

@LukaszRozmej LukaszRozmej commented Oct 30, 2025

Changes

  • Creates base RlpDecoder classes.
  • Makes them catch IndexOutOfRandeException and ArgumentOutOfRangeException and pack it in RlpException
  • Makes it easier to catch them

Types of changes

What types of changes does your code introduce?

  • Refactoring

Testing

Requires testing

  • Yes

If yes, did you write tests?

  • No

Notes on testing

Should be covered by existing tests

Documentation

Requires documentation update

  • No

Requires explanation in Release Notes

  • No

@LukaszRozmej
Copy link
Copy Markdown
Member Author

@copilot can you fix other Rlp decoders?
This needs changes like I did in ReceiptMessageDecoder69 inheriting from RlpValueDecoder or RlpStreamDecoder

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 30, 2025

@LukaszRozmej I've opened a new pull request, #9604, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits October 31, 2025 00:04
)

* 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>
Comment on lines +44 to +89
public abstract class RlpStreamDecoder<T> : IRlpStreamDecoder<T>
{
public abstract int GetLength(T item, RlpBehaviors rlpBehaviors = RlpBehaviors.None);

public T Decode(RlpStream rlpStream, RlpBehaviors rlpBehaviors = RlpBehaviors.None)
{
try
{
return DecodeInternal(rlpStream, rlpBehaviors);
}
catch (Exception e) when (e is IndexOutOfRangeException or ArgumentOutOfRangeException)
{
ThrowRlpException(e);
return default;
}
}

[DoesNotReturn]
[StackTraceHidden]
protected static void ThrowRlpException(Exception exception)
{
throw new RlpException($"Cannot decode stream of {nameof(T)}", exception);
}

protected abstract T DecodeInternal(RlpStream rlpStream, RlpBehaviors rlpBehaviors = RlpBehaviors.None);

public abstract void Encode(RlpStream stream, T item, RlpBehaviors rlpBehaviors = RlpBehaviors.None);
}

public abstract class RlpValueDecoder<T> : RlpStreamDecoder<T>, IRlpValueDecoder<T>
{
public T Decode(ref Rlp.ValueDecoderContext decoderContext, RlpBehaviors rlpBehaviors = RlpBehaviors.None)
{
try
{
return DecodeInternal(ref decoderContext, rlpBehaviors);
}
catch (Exception e) when (e is IndexOutOfRangeException or ArgumentOutOfRangeException)
{
ThrowRlpException(e);
return default;
}
}

protected abstract T DecodeInternal(ref Rlp.ValueDecoderContext decoderContext, RlpBehaviors rlpBehaviors = RlpBehaviors.None);
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main changes here

Copy link
Copy Markdown
Contributor

@alexb5dh alexb5dh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only don't like that this makes it impossible for a single class to implement multiple decoders.

Copy link
Copy Markdown
Contributor

@smartprogrammer93 smartprogrammer93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i assume the sealing of the classes is done by AI and not intentional. can be changed later if needed

@LukaszRozmej
Copy link
Copy Markdown
Member Author

Only don't like that this makes it impossible for a single class to implement multiple decoders.

It is still possible, but you need to implement the catch stuff yourself

@LukaszRozmej
Copy link
Copy Markdown
Member Author

i assume the sealing of the classes is done by AI and not intentional. can be changed later if needed

It is intentional for performance

@LukaszRozmej LukaszRozmej merged commit 8ee14cf into master Oct 31, 2025
81 checks passed
@LukaszRozmej LukaszRozmej deleted the fix/rlp-packs-exceptions branch October 31, 2025 09:37
LukaszRozmej added a commit that referenced this pull request Nov 3, 2025
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants