ENG-190: Partial Monza #34
Conversation
… l-monninger/celestia-testing
|
|
||
| impl MempoolTransactionOperations for RocksdbMempool { | ||
|
|
||
| async fn has_mempool_transaction(&self, transaction_id: Id) -> Result<bool, Error> { |
There was a problem hiding this comment.
There's a warning about async methods in this trait:
warning: use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified
--> protocol-units/mempool/util/src/lib.rs:11:5
|
11 | async fn has_mempool_transaction(&self, transaction_id :Id) -> Result<bool, anyhow::Error>;
| ^^^^^
|
= note: you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future`
= note: `#[warn(async_fn_in_trait)]` on by default
help: you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`, but these cannot be relaxed without a breaking API change
|
11 - async fn has_mempool_transaction(&self, transaction_id :Id) -> Result<bool, anyhow::Error>;
11 + fn has_mempool_transaction(&self, transaction_id :Id) -> impl std::future::Future<Output = Result<bool, anyhow::Error>> + Send;
Think twice about whether we want it to be async or desugar as per the warning? There is also a proc macro to auto-generate a Send version of the trait. See the details in Rust blog
There was a problem hiding this comment.
Yes I noticed these .. I thought maybe its a good idea for a maintenance ticket. The #[trait_variant::make()] looks good, or we can desugar manually if we prefer.
There was a problem hiding this comment.
Created a ticket: https://movement.atlassian.net/browse/ENG-245
There was a problem hiding this comment.
IDC whichever way you guys want to go. I don't care about desugared or not. I've only been using #[tonic::async_trait] when I want to Box<dyn >> because 1.75.0's async traits don't allow this.
And, @0xmovses, I don't thing this needs to be a ticket.
There was a problem hiding this comment.
Yeah I don’t mind the warnings for now. Thought it could be a nice starter issue for a new team member.
There was a problem hiding this comment.
But at least it's clear we want async here at all, isn't it?
The async_trait proc macro puts you up for boxing, while the new in-language async traits (or desugared impl Future in return types of trait methods) leave the room for static future types.
|
This should help unbreak the build: movementlabsxyz/aptos-core#8 |
|
One more time, with testing: movementlabsxyz/aptos-core#9 |
| async fn get_api( | ||
| &self, | ||
| mode : &FinalityMode, | ||
| ) -> Result<Apis, anyhow::Error>; |
There was a problem hiding this comment.
It looks like aptos-api is going to be needed and movementlabsxyz/aptos-core#5 cuts out too much. Correct me if this is not so; I will look into restoring a subset of it.
Summary