Relayer integration. ServiceBuilder. Context #450
Conversation
| @@ -1,6 +1,7 @@ | |||
| use crate::config::Config; | |||
| use crate::database::Database; | |||
| use anyhow::Result; | |||
| use fuel_core_interfaces::relayer::RelayerDb; | |||
There was a problem hiding this comment.
[nit] Unrelated to this PR, but what do you think about moving the RelayerDb trait to the relayer crate? Since fuel-core already depends on the relayer, it would largely just be changing the import paths. My thinking is that we don't expect other modules outside of the relayer to access the RelayerDb directly, they should only use the Sender interface. So we probably shouldn't share guarded / lower level implementation requirements of modules with each other via fuel-core-interfaces.
There was a problem hiding this comment.
The best way would be to create fuel-storage and extract database related stuff from fuel-core. In this case leaving RelayerDb in fuel-core-interface makes sense. I feel like those traits for db are going to be refactored in future to aggregate them and make better intention/clarification for usage, but for now, they seem good enough.
There was a problem hiding this comment.
i meant fuel-core-storage or fuel-database as we already have fuel-storage
| .db(Box::new(database.clone()) as Box<dyn RelayerDb>) | ||
| .import_block_event(block_importer.subscribe()) | ||
| .private_key( | ||
| hex::decode("c6bd905dcac2a0b1c43f574ab6933df14d7ceee0194902bce523ed054e8e798b") |
There was a problem hiding this comment.
Shouldn't this be part of the relayer config / not hardcoded?
There was a problem hiding this comment.
I missed removing this. I have one last thing to do with the integration of fuel-relayer and that would be to include config in cli. This will be resolved there: #447
There was a problem hiding this comment.
Very clean and nice PR! The use of context as a way to guard singleton resources in a threadsafe way seems like a decent approach. Although I wonder if the same simplicity could be accomplished by merging the context and relayer together?
Thanks for all the database tech debt cleanup as well.
Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
It is a good improvement, context can contain externals to connect to other modules, and there is a clear separation of external dependencies and it allows us to have I didn't think a lot about this, for now, it is still just an idea for the future, but having a separate context allows us to have a clearer idea about what external connections we can switch. As in inserting a sniffing channel that would log interaction/calls between for example |
rakita
requested review from
adlerjohn and
Voxelot
and removed request for
Voxelot
Integrate
fuel-relayerasfuel-coremodule and connect it to other module channels. Introducerelayer::SenderIntroduce
ServiceBuilderin relayer so that we can build service and service can be called withstart()without any inputs.Introduce notion of
Contextinside relayer so that we know what to share tofuel_relayer::runand what to expect to be returned back and kept in relayer server.