[WIP] Big RLP decoder refactoring - unifying code

[LukaszRozmej]

Dotnet 9 allows us to do more with ref structs:

1. They can implement interfaces
2. They can be generic parameters to methods

We can leverage that to simplify our RLP decoders code and remove duplication.

Changes

• Rename ValueDecoderContext to RlpValueStream
• Create IRlpStream interface
• Migrate AccessListDecoder, OptimismReceiptStorageDecoder, AuthorizationTupleDecoder

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

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Remarks

Potentially: Unify IRlpXXX interfaces

Decoders to refactor:

Search target
Nethermind.Serialization.Rlp.IRlpValueDecoder
Found usages (19 usages found)
AccountDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.AccountDecoder
BlockBodyDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.BlockBodyDecoder
BlockDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.BlockDecoder
BlockInfoDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.BlockInfoDecoder
ChainLevelDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.ChainLevelDecoder
CompactReceiptStorageDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.CompactReceiptStorageDecoder
HeaderDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.HeaderDecoder
KeccakDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.KeccakDecoder
LogEntryDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.LogEntryDecoder
ReceiptStorageDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.ReceiptStorageDecoder
TxDecoder.cs (4 usages found)
Nethermind.Serialization.Rlp.TxDecoder
Nethermind.Serialization.Rlp.SystemTxDecoder
Nethermind.Serialization.Rlp.GeneratedTxDecoder
Nethermind.Serialization.Rlp.TxDecoder
WithdrawalDecoder.cs (1 usage found)
Nethermind.Serialization.Rlp.WithdrawalDecoder
<Nethermind.TxPool> (1 usage found)
LightTxDecoder.cs (1 usage found)
Nethermind.TxPool.LightTxDecoder

[emlautarom1]

A few random thoughts that come to mind with this proposal:

• Does it make sense to have both a class and a ref struct to deal with RlpStreams ? Where do we even need the class implementation?
• How many implementations do we need for the IRlpStream interface? Do we gain anything with it? From the current API the only implementation that I think of is something that wraps over a ReadOnlySpan (or similar).
• Do we want to continue investing in certain APIs like Check, or should we revisit a different API like the one proposed Refactor TxDecoder #7334 (comment)
• The IRlpStream interface has several per-type specific methods like DecodeByte, DecodeInt, etc. Could we come up with a solution that supports extension without requiring modification?

Maybe this would be a good topic to discuss on the next Tech Chat. Personally I have some available time since we fixed the most urgent issues so I might play around with the new .NET 9 features to see what we could do (no promises though).

[LukaszRozmej]

A few random thoughts that come to mind with this proposal:

• Does it make sense to have both a class and a ref struct to deal with RlpStreams ? Where do we even need the class implementation?
• How many implementations do we need for the IRlpStream interface? Do we gain anything with it? From the current API the only implementation that I think of is something that wraps over a ReadOnlySpan (or similar).
• Do we want to continue investing in certain APIs like Check, or should we revisit a different API like the one proposed Refactor TxDecoder #7334 (comment)
• The IRlpStream interface has several per-type specific methods like DecodeByte, DecodeInt, etc. Could we come up with a solution that supports extension without requiring modification?

Maybe this would be a good topic to discuss on the next Tech Chat. Personally I have some available time since we fixed the most urgent issues so I might play around with the new .NET 9 features to see what we could do (no promises though).

ref struct was mostly constructed to work with Span but I also see it was modified to work with Memory, so could potentially be used generally. We would have potential problem with it if we ever want to use it in async context though.

We probably won't ever have more implementations than those 2.

Not sure if we could have some Decode method or extension method. If yes that would be great.