Refactoring of Optimism Plugin

[emlautarom1]

Is your feature request related to a problem? Please describe.

Currently, Nethermind supports Optimism through its plugin mechanism, though certain Optimism specific behaviors and constants are defined in Nethermind itself, resulting in severe coupling.

Describe the solution you'd like

Ideally, supporting for Optimism should not rely on having Optimism-specific classes/constants/behaviors. Note that this in turn requires Nethermind to adjust it's plugin mechanism to support injecting these components.

Describe alternatives you've considered

Do nothing and continue working with the current implementation. Continuing with the existing implementation implies no risk of introducing new bugs but will increase the long term maintenance cost when (not if) we introduce support for new chains (ex. Taiko).

Additional context

• Op-geth details: https://op-geth.optimism.io/
• Optimism Plugin preparation PR: Optimism plugin: Transactions refactoring #6221
• Optimism Plugin PR: Optimism plugin #6146
• Deposit TX type bug: https://github.com/NethermindEth/nethermind/pull/6349/files

[emlautarom1]

A first complication is the existence of Optimism-specific fields in the base Transaction type:

nethermind/src/Nethermind/Nethermind.Core/Transaction.cs

Lines 31 to 37 in 6beafee

	// Optimism deposit transaction fields
	// SourceHash uniquely identifies the source of the deposit
	public Hash256? SourceHash { get; set; }
	// Mint is minted on L2, locked on L1, nil if no minting.
	public UInt256 Mint { get; set; }
	// Field indicating if this transaction is exempt from the L2 gas limit.
	public bool IsOPSystemTransaction { get; set; }

I think that we can just push these fields to a subclass, but I haven't had the time to look into this yet. The main challenge of the current design is in TxDecoder

nethermind/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoder.cs

Line 36 in 6beafee

public class TxDecoder<T> :

TxDecoder, at a high level, decodes information about a Transaction from RLP, but in order to do so it needs to know about the TxType:

nethermind/src/Nethermind/Nethermind.Core/TxType.cs

Line 6 in 6beafee

public enum TxType : byte

Depending on the TxType different execution paths are taken. For example:

nethermind/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoder.cs

Lines 76 to 94 in 6beafee

	int transactionLength = rlpStream.ReadSequenceLength();
	int lastCheck = rlpStream.Position + transactionLength;
	switch (transaction.Type)
	{
	case TxType.Legacy:
	TxDecoder<T>.DecodeLegacyPayloadWithoutSig(transaction, rlpStream);
	break;
	case TxType.AccessList:
	DecodeAccessListPayloadWithoutSig(transaction, rlpStream, rlpBehaviors);
	break;
	case TxType.EIP1559:
	DecodeEip1559PayloadWithoutSig(transaction, rlpStream, rlpBehaviors);
	break;
	case TxType.Blob:
	DecodeShardBlobPayloadWithoutSig(transaction, rlpStream, rlpBehaviors);
	break;
	case TxType.DepositTx:
	TxDecoder<T>.DecodeDepositPayloadWithoutSig(transaction, rlpStream, rlpBehaviors);

As mentioned in the ticket, we want to support additional transaction types which are not defined in Nethermind itself, but can be defined in plugins. Thus, the current design of using an enum is inappropriate due to the lack of extensibility.

This requirements forces us to look for some kind of runtime dispatch and I think our options are as follows:

• Define a ITxType interface, and multiple classes that implement it. Plugins can define their own classes that implement this interface. Since it does not make sense to have multiple instances of a specific ITxType, all classes that implement the interface should be essentially singletons. A variant of this approach is to use an abstract class TxType instead of an interface.
• Define a TxType class that is essentially a record of functions, such that different transaction types can be implemented as global static variables that provide the appropriate behavior (this approach is not very OOP-like).

In any case, we need to be able to know at runtime all possible transaction types in order to properly decode transactions. For example, if we read a 0, we need to look-up which transaction type matches the code in order to fetch the appropriate transaction decoding behavior.

For this, I think our options are as follow:

1. Make user code register ITxType instances
2. Automatically discover ITxType implementators using reflection.

Then, we need to decide on how to pass this collection of instances, either by manually threading some kind of "registry" (actually, a parser from byte to ITxType), or by defining one globally.

There is precedent for both approaches in the codebase:

1. JsonConverter<T> instances are registered in EthereumJsonSerializer, which is then threaded as a dependency.
2. INethermindPlugin implementators are registered and initialized in PluginLoader and TypeDiscovery automatically through reflection and stored in static variables.

[emlautarom1]

After discussing it with the team, we'll take this opportunity to perform a bigger refactor by completely separating different transaction into their own types. As a rough sketch, we would like to construct a Transaction hierarchy and have each transaction provide it's own processing pipeline logic, from RLP decoding to JSON serialization.

Currently we have 5 transactions encoded in the system: Legacy, AccessList, EIP1559, Blob and Deposit being the later one Optimism-specific. By separating each transaction into it's own type we'll avoid the current conditional logic in multiple places (ex. in JSON encoding), and we'll support extension without involving modification of existing types.

Since new transactions can be added by plugins, we'll construct a "registry" during initialization where different transactions can be registered (NOTE: we believe manual registration is a better approach over automatic discovery through reflection). This registry will be globally accessible to all code that requires it, and it's up to discussion how we want to provide this access. On this note, different access mechanism will have an impact on how much we need to refactor the existing test suite.

Lastly, we want to ensure that only valid transactions will be processed by the client based on information provided by ForkActivation. Care will be required to ensure that for each transaction we're properly validating that its type is valid in each context.

Starting with separating the deposit transaction from Optimism will give us a better understanding of this approach, which then we can extend to all remaining transaction types.

cc. @deffrian @flcl42 @jmederosalvarado @LukaszRozmej