From dda4a1365c75f7d89b2bf2418b1fd01b7ce1a589 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Wed, 27 Sep 2023 10:55:15 +0200 Subject: [PATCH] Add discriminant type to allow multiple mempool services (#441) Overwatch requires all services to have a different service id. Unfortunately, such service id can't depend on generic parameters, which means that we can't have two instances of the mempool service even if they are instantiated with different types. This commit circuments this limitation by adding another type parameter. --- nodes/nomos-node/src/bridges/mod.rs | 5 ++-- nodes/nomos-node/src/lib.rs | 3 ++- nomos-services/consensus/src/lib.rs | 3 ++- nomos-services/mempool/src/lib.rs | 38 ++++++++++++++++++++++++---- nomos-services/mempool/tests/mock.rs | 12 +++++---- 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/nodes/nomos-node/src/bridges/mod.rs b/nodes/nomos-node/src/bridges/mod.rs index fe9aefdc..b3299ba2 100644 --- a/nodes/nomos-node/src/bridges/mod.rs +++ b/nodes/nomos-node/src/bridges/mod.rs @@ -17,6 +17,7 @@ use nomos_http::http::{HttpMethod, HttpRequest, HttpResponse}; use nomos_mempool::backend::mockpool::MockPool; use nomos_mempool::network::adapters::libp2p::Libp2pAdapter; use nomos_mempool::network::NetworkAdapter; +use nomos_mempool::Transaction as TxDiscriminant; use nomos_mempool::{MempoolMetrics, MempoolMsg, MempoolService}; use nomos_network::backends::libp2p::Libp2p; use nomos_network::backends::NetworkBackend; @@ -51,7 +52,7 @@ pub fn mempool_metrics_bridge( handle: overwatch_rs::overwatch::handle::OverwatchHandle, ) -> HttpBridgeRunner { Box::new(Box::pin(async move { - get_handler!(handle, MempoolService::Hash>, MockPool::Hash>>, "metrics" => handle_mempool_metrics_req) + get_handler!(handle, MempoolService::Hash>, MockPool::Hash>, TxDiscriminant>, "metrics" => handle_mempool_metrics_req) })) } @@ -77,7 +78,7 @@ where Box::new(Box::pin(async move { let (mempool_channel, mut http_request_channel) = build_http_bridge::< - MempoolService::Hash>>, + MempoolService::Hash>, TxDiscriminant>, AxumBackend, _, >(handle.clone(), HttpMethod::POST, "addtx") diff --git a/nodes/nomos-node/src/lib.rs b/nodes/nomos-node/src/lib.rs index 3dff12c8..11142ca9 100644 --- a/nodes/nomos-node/src/lib.rs +++ b/nodes/nomos-node/src/lib.rs @@ -20,7 +20,7 @@ use nomos_http::bridge::HttpBridgeService; use nomos_http::http::HttpService; use nomos_log::Logger; use nomos_mempool::network::adapters::libp2p::Libp2pAdapter as MempoolLibp2pAdapter; - +use nomos_mempool::Transaction as TxDiscriminant; use nomos_mempool::{backend::mockpool::MockPool, MempoolService}; use nomos_network::backends::libp2p::Libp2p; @@ -56,6 +56,7 @@ type DataAvailability = DataAvailabilityService< type Mempool = MempoolService< MempoolLibp2pAdapter::Hash>, MockPool::Hash>, + TxDiscriminant, >; #[derive(Services)] diff --git a/nomos-services/consensus/src/lib.rs b/nomos-services/consensus/src/lib.rs index 838b8547..111062cb 100644 --- a/nomos-services/consensus/src/lib.rs +++ b/nomos-services/consensus/src/lib.rs @@ -42,6 +42,7 @@ use nomos_core::tx::{Transaction, TxSelect}; use nomos_core::vote::Tally; use nomos_mempool::{ backend::MemPool, network::NetworkAdapter as MempoolAdapter, MempoolMsg, MempoolService, + Transaction as TxDiscriminant, }; use nomos_network::NetworkService; use overwatch_rs::services::relay::{OutboundRelay, Relay, RelayMessage}; @@ -119,7 +120,7 @@ where // underlying networking backend. We need this so we can relay and check the types properly // when implementing ServiceCore for CarnotConsensus network_relay: Relay>, - mempool_relay: Relay>, + mempool_relay: Relay>, _overlay: std::marker::PhantomData, // this need to be substituted by some kind DA bo _blob_certificate: std::marker::PhantomData, diff --git a/nomos-services/mempool/src/lib.rs b/nomos-services/mempool/src/lib.rs index 728b4681..7fff021e 100644 --- a/nomos-services/mempool/src/lib.rs +++ b/nomos-services/mempool/src/lib.rs @@ -2,7 +2,10 @@ pub mod backend; pub mod network; // std -use std::fmt::{Debug, Error, Formatter}; +use std::{ + fmt::{Debug, Error, Formatter}, + marker::PhantomData, +}; // crates use futures::StreamExt; @@ -19,17 +22,23 @@ use overwatch_rs::services::{ ServiceCore, ServiceData, ServiceId, }; -pub struct MempoolService +pub struct MempoolService where N: NetworkAdapter, P: MemPool, P::Settings: Clone, P::Item: Debug + 'static, P::Key: Debug + 'static, + D: Discriminant, { service_state: ServiceStateHandle, network_relay: Relay>, pool: P, + // This is an hack because SERVICE_ID has to be univoque and associated const + // values can't depend on generic parameters. + // Unfortunately, this means that the mempools for certificates and transactions + // would have the same SERVICE_ID and break overwatch asumptions. + _d: PhantomData, } pub struct MempoolMetrics { @@ -93,15 +102,31 @@ where impl RelayMessage for MempoolMsg {} -impl ServiceData for MempoolService +pub struct Transaction; +pub struct Certificate; + +pub trait Discriminant { + const ID: &'static str; +} + +impl Discriminant for Transaction { + const ID: &'static str = "mempool-txs"; +} + +impl Discriminant for Certificate { + const ID: &'static str = "mempool-certs"; +} + +impl ServiceData for MempoolService where N: NetworkAdapter, P: MemPool, P::Settings: Clone, P::Item: Debug + 'static, P::Key: Debug + 'static, + D: Discriminant, { - const SERVICE_ID: ServiceId = "Mempool"; + const SERVICE_ID: ServiceId = D::ID; type Settings = Settings; type State = NoState; type StateOperator = NoOperator; @@ -109,7 +134,7 @@ where } #[async_trait::async_trait] -impl ServiceCore for MempoolService +impl ServiceCore for MempoolService where P: MemPool + Send + 'static, P::Settings: Clone + Send + Sync + 'static, @@ -117,6 +142,7 @@ where P::Item: Clone + Debug + Send + Sync + 'static, P::Key: Debug + Send + Sync + 'static, N: NetworkAdapter + Send + Sync + 'static, + D: Discriminant + Send, { fn init(service_state: ServiceStateHandle) -> Result { let network_relay = service_state.overwatch_handle.relay(); @@ -125,6 +151,7 @@ where service_state, network_relay, pool: P::new(settings.backend), + _d: PhantomData, }) } @@ -133,6 +160,7 @@ where mut service_state, network_relay, mut pool, + .. } = self; let network_relay: OutboundRelay<_> = network_relay diff --git a/nomos-services/mempool/tests/mock.rs b/nomos-services/mempool/tests/mock.rs index 329d0a3e..f1c94cfc 100644 --- a/nomos-services/mempool/tests/mock.rs +++ b/nomos-services/mempool/tests/mock.rs @@ -13,7 +13,7 @@ use overwatch_rs::{overwatch::OverwatchRunner, services::handle::ServiceHandle}; use nomos_mempool::{ backend::mockpool::MockPool, network::adapters::mock::{MockAdapter, MOCK_TX_CONTENT_TOPIC}, - MempoolMsg, MempoolService, Settings, + MempoolMsg, MempoolService, Settings, Transaction, }; #[derive(Services)] @@ -21,7 +21,7 @@ struct MockPoolNode { logging: ServiceHandle, network: ServiceHandle>, mockpool: ServiceHandle< - MempoolService, MockTxId>>, + MempoolService, MockTxId>, Transaction>, >, } @@ -73,9 +73,11 @@ fn test_mockmempool() { .unwrap(); let network = app.handle().relay::>(); - let mempool = app - .handle() - .relay::, MockTxId>>>(); + let mempool = app.handle().relay::, MockTxId>, + Transaction, + >>(); app.spawn(async move { let network_outbound = network.connect().await.unwrap();