fuel-core
fuel-core copied to clipboard
FuelLabs
Block Producer WIP
Initial base for the block producer service WIP
Leveraging ports & adapters architecture to unblock development from depending on other components & also to improve composability & testability.
The main principle behind the P&A design is to allow the component to specify all external dependencies via its' own traits which don't need to be depended on by others. Fuel Core can provide adapter impls at a later time when feasible, while allowing the block producer to be logically implemented and functionally tested in the meantime as a self-contained unit.
I'm also experimenting with the service design a bit, it seems like treating the service as a dummy wrapper for spawning the stateful block producer task makes the borrow checker happier and avoids the need for mutexes and other concurrency primitives.
Most of this PR is just additional logic for the block producer. I brought in the P&A concepts as I went along because I ran into blockers directly integrating with the relayer and txpool since they aren't done yet.
Then I realized the ports pattern could be used to start developing the other modules in parallel so that we don't have to serialize the development of these modules based on their order of dependencies. I'd consider it complementary to the event-driven arch. It's purely an additional abstraction which makes the core domain logic of the modules more pure, testable, and easier to follow. For example, the impl for the core domain logic of block production is already there and fits into a 30 LOC function.
If we go with this P&A approach in the other modules, we'll basically need to do two passes. The first pass is building the core domain logic and testing all the edge cases, and then the second pass would be simply implementing the adapters in the main fuel-core crate and injecting them into the appropriate modules.
I'll get more of this wired up and then make an official design proposal using this as an example.
Why i asked that directly for intention is that I was not sure if you looked at how it is now, and for example how fuel-relayer did this, as it does rely on database and fuel-importer and can be standalone tested: https://github.com/FuelLabs/fuel-core/blob/f549a3f315380202dec60893a14bd7c9990c27e0/fuel-relayer/src/service.rs#L26-L27
For relayer channels, I need to move them from new function but the idea is there.
same for txpool: https://github.com/FuelLabs/fuel-core/blob/887f9443d9a362cb150967c1953d82b68c515ea1/fuel-txpool/src/service.rs#L31
Abstraction is done over channel message enums.
Maybe there is a need for having new, init and start functions. new to create, init to bind channels, and start/stop to stop and start service.
As discussed with @Voxelot earlier today, this PR now contains the standalone block producer module with minimal implementation. It will not be used elsewhere in the code yet, but to keep PRs small enough to review, that integration will be done later.
Follow-up PRs will include:
- Integrating the block producer into Fuel node service
- Configurable block production trigger https://github.com/FuelLabs/fuel-core/issues/50
- A minimal block importer, see also https://github.com/FuelLabs/fuel-core/issues/465
- Proper e2e testing
It appears that we are reintroducing the concept of a prefilled test database in the form of DummyDb in fuel-core-interfaces/src/db/helpers.rs.
We removed all instances of DummyDb in unit tests and replaced it with an empty database that unit tests could instantiate and fill themselves.
Is there a reason we want to reintroduce the DummyDb struct and all its prefilled data?
Is there a reason we want to reintroduce the DummyDb struct and all its prefilled data?
No, the helpers file just slipped by me when merging from master. I'll remove it.
Just pushed a bunch of changes:
- No longer using the mpsc channels, using a simpler async trait approach for the block producer instead.
- Cleaned up the integ test to no longer require fuel-core by using a mockdb for the txpool
- No longer use a trailing da_height. This just slows down the deposit process and creates a bunch of extra corner cases when selecting transaction from the txpool that contain messages. Instead, the block importer should queue blocks until the local relayer finalized da_height catches up to the pending imported block's da_height. This way the block producer can always use the latest finalized da_height for new blocks since it'll be guaranteed to always be >= the previously imported block's da_height.
- unit tests!
Look good!=) It is hard to read the integration test, but maybe in the future, we will find how to make it more readable=D
I think once the followup tasks are implemented this test should look a lot simpler and not have to simulate as much of the node behavior.