NethermindEth / juno

Starknet client implementation.
https://juno.nethermind.io
Apache License 2.0
404 stars 170 forks source link

Refactor `feeder` and `gateway` packages and extract common types #1015

Open IronGauntlets opened 1 year ago

IronGauntlets commented 1 year ago

The clients/feeder and clients/gateway packages belong to the Frameworks & Drivers layer of clean architecture, therefore any layer inside of Frameworks & Drivers can only use clients/feeder and clients/gateway through an interface. This is known as The Dependency Inversion Rule, the overarching principle of clean architecture.

Defining an interface to access an outer layer has the benefit of having multiple concrete implementations which are accessed through a single interface. This helps with minimising sweeping changes across the codebase when a breaking change is introduced. However, the Dependency Inversion rule can introduce unnecessary complexity early on in the project, as declaring an interface for a single concrete implementation doesn't always make sense.

At the beginning of the project, we declared StarknetData as an interface used to fetch Starknet's data which could be implemented by either the feeder gateway or the p2p layer (Interface Adapter layer).

To add different transactions we introduced a gateway package, accessed through a Gateway interface in the rpc package, however, we still use the feeder package directly in the starknet data package and rpc package which violates the dependency inversion rule. Therefore we need to introduce a Feeder interface which is used by both the rpc package and the StarknetData package.

feeder and gateway are under the same parent client package where each package creates its own client with the only difference of post and get methods. As @omerfirmak suggested we should merge these packages together to have a single client with both a post and get methods. This client is then composed into feeder and gateway structs (which are responsible for building the query URL for fetching Starknet data from the sequencer). Another benefit of merging these packages together is that they share the same types, therefore, merging them would reduce duplication.

Crossing layers in the clean architecture requires adapter functions: input from rpc to core types should have its own adapter and input from feeder and gateway also should have their own adapters to core types (and vice versa). However, due to the commonality between all of the adapters and to reduce duplication, we created feeder2core. These functions can be moved to the utils package with a more generic name.

Also, the feeder, gateway and rpc packages have very similar types (namely Transaction and TransactionReceipts nested types) all of whom can also be moved to the utils to reduce duplication.

In summary, this issue has the following parts:

rianhughes commented 1 year ago

I'd like to work on this

rianhughes commented 1 year ago

I've broken this PR into three sub-PRs:

joshklop commented 1 year ago

Love the idea of merging feeder and gateway.

Therefore we need to introduce a Feeder interface which is used by both the rpc package and the StarknetData package.

Why not add an AddTransaction method to the StarknetData interface and use StarknetData in rpc? cc @rianhughes @IronGauntlets