Mempool add APIs #489
Mempool add APIs #489
Conversation
| post, | ||
| path = "/mempool/add/tx", | ||
| responses( | ||
| (status = 200, description = "add transaction to the mempool"), |
There was a problem hiding this comment.
We need to be consistent in descriptions. I have seen usually we capitalize them. Tiny details.
| .route("/mempool/add/tx", routing::post(add_tx::<T>)) | ||
| .route("/mempool/add/cert", routing::post(add_cert)) |
There was a problem hiding this comment.
I am not sure add_tx and add_cert should be under mempool. I think we can consider mempool just for txs, add_cert should be under da. @zeegomo wdyt?
There was a problem hiding this comment.
The concept of a mempool is the same for both, I don't have strong opinions on this but there might be suggestions on the OpenAPI standard on how to handle it
| pub(crate) async fn add_tx<N, A, T>( | ||
| handle: &overwatch_rs::overwatch::handle::OverwatchHandle, | ||
| tx: T, | ||
| ) -> Result<(), super::DynError> | ||
| where | ||
| N: NetworkBackend, | ||
| A: NetworkAdapter<Backend = N, Item = T, Key = <T as Transaction>::Hash> | ||
| + Send | ||
| + Sync | ||
| + 'static, | ||
| A::Settings: Send + Sync, | ||
| T: Transaction | ||
| + Clone | ||
| + Debug | ||
| + Hash | ||
| + serde::Serialize | ||
| + serde::de::DeserializeOwned | ||
| + Send | ||
| + Sync | ||
| + 'static, | ||
| <T as Transaction>::Hash: std::cmp::Ord + Debug + Send + Sync + 'static, | ||
| { | ||
| let relay = handle | ||
| .relay::<MempoolService<A, MockPool<T, <T as Transaction>::Hash>, TxDiscriminant>>() | ||
| .connect() | ||
| .await?; | ||
| let (sender, receiver) = oneshot::channel(); | ||
|
|
||
| relay | ||
| .send(MempoolMsg::Add { | ||
| key: Transaction::hash(&tx), | ||
| item: tx, | ||
| reply_channel: sender, | ||
| }) | ||
| .await | ||
| .map_err(|(e, _)| e)?; | ||
|
|
||
| match receiver.await { | ||
| Ok(Ok(())) => Ok(()), | ||
| Ok(Err(())) => Err("mempool error".into()), | ||
| Err(e) => Err(e.into()), | ||
| } |
There was a problem hiding this comment.
We don't need two different functions for this, they're the same service with different generics
| .route("/mempool/add/tx", routing::post(add_tx::<T>)) | ||
| .route("/mempool/add/cert", routing::post(add_cert)) |
There was a problem hiding this comment.
The concept of a mempool is the same for both, I don't have strong opinions on this but there might be suggestions on the OpenAPI standard on how to handle it
| @@ -30,6 +31,8 @@ pub struct AxumBackendSettings { | |||
| pub carnot: OverwatchHandle, | |||
| pub network: OverwatchHandle, | |||
| pub storage: OverwatchHandle, | |||
| pub cert_mempool: OverwatchHandle, | |||
There was a problem hiding this comment.
why do we need to keep adding handles?
No description provided.