Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Remove async-trait #5812

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ libp2p-autonat = { version = "0.13.1", path = "protocols/autonat" }
libp2p-connection-limits = { version = "0.4.1", path = "misc/connection-limits" }
libp2p-core = { version = "0.42.1", path = "core" }
libp2p-dcutr = { version = "0.12.1", path = "protocols/dcutr" }
libp2p-dns = { version = "0.42.1", path = "transports/dns" }
libp2p-dns = { version = "0.43.0", path = "transports/dns" }
libp2p-floodsub = { version = "0.45.0", path = "protocols/floodsub" }
libp2p-gossipsub = { version = "0.48.0", path = "protocols/gossipsub" }
libp2p-identify = { version = "0.46.0", path = "protocols/identify" }
Expand Down
3 changes: 1 addition & 2 deletions protocols/autonat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ categories = ["network-programming", "asynchronous"]


[dependencies]
async-trait = { version = "0.1", optional = true }
asynchronous-codec = { workspace = true }
either = { version = "1.9.0", optional = true }
futures = { workspace = true }
Expand All @@ -43,7 +42,7 @@ libp2p-swarm = { workspace = true, features = ["macros"] }

[features]
default = ["v1", "v2"]
v1 = ["dep:libp2p-request-response", "dep:web-time", "dep:async-trait"]
v1 = ["dep:libp2p-request-response", "dep:web-time"]
v2 = ["dep:either", "dep:futures-bounded", "dep:thiserror", "dep:rand_core"]

# Passing arguments to the docsrs builder in order to properly document cfg's.
Expand Down
74 changes: 42 additions & 32 deletions protocols/autonat/src/v1/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.

use std::io;
use std::{future::Future, io};

use async_trait::async_trait;
use asynchronous_codec::{FramedRead, FramedWrite};
use futures::{
io::{AsyncRead, AsyncWrite},
Expand All @@ -39,72 +38,83 @@ pub const DEFAULT_PROTOCOL_NAME: StreamProtocol = StreamProtocol::new("/libp2p/a
#[derive(Clone)]
pub struct AutoNatCodec;

#[async_trait]
impl request_response::Codec for AutoNatCodec {
type Protocol = StreamProtocol;
type Request = DialRequest;
type Response = DialResponse;

async fn read_request<T>(&mut self, _: &StreamProtocol, io: &mut T) -> io::Result<Self::Request>
fn read_request<T>(
&mut self,
_: &StreamProtocol,
io: &mut T,
) -> impl Send + Future<Output = io::Result<Self::Request>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it might be better to have Send last

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, we could probably leave the async signature intact here while having the trait member return impl Future<Output = T> + Send without a compiler warning. Thoughts on that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I left a comment misunderstanding your note which I've now deleted.

async fn for the implementations should be possible 👍 TIL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, we could probably leave the async signature intact here while having the trait member return impl Future<Output = T> + Send without a compiler warning. Thoughts on that?

You mean async fn read_request(...) -> impl Future<Output = ...>? What would be the advantage of that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean the traits would use fn read_request(...) -> impl Future<Output = ...> while the impl of the trait would use async fn read_request(...) -> ....

Copy link
Member

@dariusc93 dariusc93 Feb 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eg this patch should suffice (regarding request-response)

diff --git a/protocols/autonat/src/v1/protocol.rs b/protocols/autonat/src/v1/protocol.rs
index 6aa0c9916..f17a029d3 100644
--- a/protocols/autonat/src/v1/protocol.rs
+++ b/protocols/autonat/src/v1/protocol.rs
@@ -20,7 +20,6 @@
 
 use std::io;
 
-use async_trait::async_trait;
 use asynchronous_codec::{FramedRead, FramedWrite};
 use futures::{
     io::{AsyncRead, AsyncWrite},
@@ -39,7 +38,6 @@ pub const DEFAULT_PROTOCOL_NAME: StreamProtocol = StreamProtocol::new("/libp2p/a
 #[derive(Clone)]
 pub struct AutoNatCodec;
 
-#[async_trait]
 impl request_response::Codec for AutoNatCodec {
     type Protocol = StreamProtocol;
     type Request = DialRequest;
diff --git a/protocols/rendezvous/src/codec.rs b/protocols/rendezvous/src/codec.rs
index 60f9f14f3..aec7c3c2c 100644
--- a/protocols/rendezvous/src/codec.rs
+++ b/protocols/rendezvous/src/codec.rs
@@ -20,7 +20,6 @@
 
 use std::{fmt, io};
 
-use async_trait::async_trait;
 use asynchronous_codec::{BytesMut, Decoder, Encoder, FramedRead, FramedWrite};
 use futures::{AsyncRead, AsyncWrite, SinkExt, StreamExt};
 use libp2p_core::{peer_record, signed_envelope, PeerRecord, SignedEnvelope};
@@ -241,7 +240,6 @@ impl Decoder for Codec {
 #[derive(Clone, Default)]
 pub struct Codec {}
 
-#[async_trait]
 impl libp2p_request_response::Codec for Codec {
     type Protocol = StreamProtocol;
     type Request = Message;
diff --git a/protocols/request-response/src/cbor.rs b/protocols/request-response/src/cbor.rs
index 9dea02ef6..3b6f82c6d 100644
--- a/protocols/request-response/src/cbor.rs
+++ b/protocols/request-response/src/cbor.rs
@@ -49,7 +49,6 @@ pub type Behaviour<Req, Resp> = crate::Behaviour<codec::Codec<Req, Resp>>;
 pub mod codec {
     use std::{collections::TryReserveError, convert::Infallible, io, marker::PhantomData};
 
-    use async_trait::async_trait;
     use cbor4ii::core::error::DecodeError;
     use futures::prelude::*;
     use libp2p_swarm::StreamProtocol;
@@ -97,7 +96,6 @@ pub mod codec {
         }
     }
 
-    #[async_trait]
     impl<Req, Resp> crate::Codec for Codec<Req, Resp>
     where
         Req: Send + Serialize + DeserializeOwned,
diff --git a/protocols/request-response/src/codec.rs b/protocols/request-response/src/codec.rs
index d396a75ad..ac1ab06bf 100644
--- a/protocols/request-response/src/codec.rs
+++ b/protocols/request-response/src/codec.rs
@@ -19,14 +19,11 @@
 // DEALINGS IN THE SOFTWARE.
 
 use std::io;
-
-use async_trait::async_trait;
 use futures::prelude::*;
 
 /// A `Codec` defines the request and response types
 /// for a request-response [`Behaviour`](crate::Behaviour) protocol or
 /// protocol family and how they are encoded / decoded on an I/O stream.
-#[async_trait]
 pub trait Codec {
     /// The type of protocol(s) or protocol versions being negotiated.
     type Protocol: AsRef<str> + Send + Clone;
@@ -37,43 +34,43 @@ pub trait Codec {
 
     /// Reads a request from the given I/O stream according to the
     /// negotiated protocol.
-    async fn read_request<T>(
+    fn read_request<T>(
         &mut self,
         protocol: &Self::Protocol,
         io: &mut T,
-    ) -> io::Result<Self::Request>
+    ) -> impl Future<Output = io::Result<Self::Request>> + Send
     where
         T: AsyncRead + Unpin + Send;
 
     /// Reads a response from the given I/O stream according to the
     /// negotiated protocol.
-    async fn read_response<T>(
+    fn read_response<T>(
         &mut self,
         protocol: &Self::Protocol,
         io: &mut T,
-    ) -> io::Result<Self::Response>
+    ) -> impl Future<Output = io::Result<Self::Response>> + Send
     where
         T: AsyncRead + Unpin + Send;
 
     /// Writes a request to the given I/O stream according to the
     /// negotiated protocol.
-    async fn write_request<T>(
+    fn write_request<T>(
         &mut self,
         protocol: &Self::Protocol,
         io: &mut T,
         req: Self::Request,
-    ) -> io::Result<()>
+    ) -> impl Future<Output = io::Result<()>> + Send
     where
         T: AsyncWrite + Unpin + Send;
 
     /// Writes a response to the given I/O stream according to the
     /// negotiated protocol.
-    async fn write_response<T>(
+    fn write_response<T>(
         &mut self,
         protocol: &Self::Protocol,
         io: &mut T,
         res: Self::Response,
-    ) -> io::Result<()>
+    ) -> impl Future<Output = io::Result<()>> + Send
     where
         T: AsyncWrite + Unpin + Send;
 }
diff --git a/protocols/request-response/src/json.rs b/protocols/request-response/src/json.rs
index 5832c2b35..9a6ae496f 100644
--- a/protocols/request-response/src/json.rs
+++ b/protocols/request-response/src/json.rs
@@ -96,7 +96,6 @@ pub mod codec {
         }
     }
 
-    #[async_trait]
     impl<Req, Resp> crate::Codec for Codec<Req, Resp>
     where
         Req: Send + Serialize + DeserializeOwned,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice! Didn't know that was possible. I thought the trait bounds always have to be the same between trait declaration and trait impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The async sugar does produce equivalent bounds internally (or compilation errors if it can't). It's solely a difference in how they're writen.

where
T: AsyncRead + Send + Unpin,
{
let message = FramedRead::new(io, codec())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;
let request = DialRequest::from_proto(message)?;

Ok(request)
async move {
let message = FramedRead::new(io, codec())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;
let request = DialRequest::from_proto(message)?;

Ok(request)
}
}

async fn read_response<T>(
fn read_response<T>(
&mut self,
_: &StreamProtocol,
io: &mut T,
) -> io::Result<Self::Response>
) -> impl Send + Future<Output = io::Result<Self::Response>>
where
T: AsyncRead + Send + Unpin,
{
let message = FramedRead::new(io, codec())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;
let response = DialResponse::from_proto(message)?;

Ok(response)
async move {
let message = FramedRead::new(io, codec())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;
let response = DialResponse::from_proto(message)?;

Ok(response)
}
}

async fn write_request<T>(
fn write_request<T>(
&mut self,
_: &StreamProtocol,
io: &mut T,
data: Self::Request,
) -> io::Result<()>
) -> impl Send + Future<Output = io::Result<()>>
where
T: AsyncWrite + Send + Unpin,
{
let mut framed = FramedWrite::new(io, codec());
framed.send(data.into_proto()).await?;
framed.close().await?;
async move {
let mut framed = FramedWrite::new(io, codec());
framed.send(data.into_proto()).await?;
framed.close().await?;

Ok(())
Ok(())
}
}

async fn write_response<T>(
fn write_response<T>(
&mut self,
_: &StreamProtocol,
io: &mut T,
data: Self::Response,
) -> io::Result<()>
) -> impl Send + Future<Output = io::Result<()>>
where
T: AsyncWrite + Send + Unpin,
{
let mut framed = FramedWrite::new(io, codec());
framed.send(data.into_proto()).await?;
framed.close().await?;
async move {
let mut framed = FramedWrite::new(io, codec());
framed.send(data.into_proto()).await?;
framed.close().await?;

Ok(())
Ok(())
}
}
}

Expand Down
1 change: 0 additions & 1 deletion protocols/rendezvous/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ categories = ["network-programming", "asynchronous"]

[dependencies]
asynchronous-codec = { workspace = true }
async-trait = "0.1"
bimap = "0.6.3"
futures = { workspace = true, features = ["std"] }
futures-timer = "3.0.3"
Expand Down
58 changes: 34 additions & 24 deletions protocols/rendezvous/src/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.

use std::{fmt, io};
use std::{fmt, future::Future, io};

use async_trait::async_trait;
use asynchronous_codec::{BytesMut, Decoder, Encoder, FramedRead, FramedWrite};
use futures::{AsyncRead, AsyncWrite, SinkExt, StreamExt};
use libp2p_core::{peer_record, signed_envelope, PeerRecord, SignedEnvelope};
Expand Down Expand Up @@ -241,66 +240,77 @@ impl Decoder for Codec {
#[derive(Clone, Default)]
pub struct Codec {}

#[async_trait]
impl libp2p_request_response::Codec for Codec {
type Protocol = StreamProtocol;
type Request = Message;
type Response = Message;

async fn read_request<T>(&mut self, _: &Self::Protocol, io: &mut T) -> io::Result<Self::Request>
fn read_request<T>(
&mut self,
_: &Self::Protocol,
io: &mut T,
) -> impl Send + Future<Output = io::Result<Self::Request>>
where
T: AsyncRead + Unpin + Send,
{
let message = FramedRead::new(io, self.clone())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;
async move {
let message = FramedRead::new(io, self.clone())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;

Ok(message)
Ok(message)
}
}

async fn read_response<T>(
fn read_response<T>(
&mut self,
_: &Self::Protocol,
io: &mut T,
) -> io::Result<Self::Response>
) -> impl Send + Future<Output = io::Result<Self::Response>>
where
T: AsyncRead + Unpin + Send,
{
let message = FramedRead::new(io, self.clone())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;
async move {
let message = FramedRead::new(io, self.clone())
.next()
.await
.ok_or(io::ErrorKind::UnexpectedEof)??;

Ok(message)
Ok(message)
}
}

async fn write_request<T>(
fn write_request<T>(
&mut self,
_: &Self::Protocol,
io: &mut T,
req: Self::Request,
) -> io::Result<()>
) -> impl Send + Future<Output = io::Result<()>>
where
T: AsyncWrite + Unpin + Send,
{
FramedWrite::new(io, self.clone()).send(req).await?;
async move {
FramedWrite::new(io, self.clone()).send(req).await?;

Ok(())
Ok(())
}
}

async fn write_response<T>(
fn write_response<T>(
&mut self,
_: &Self::Protocol,
io: &mut T,
res: Self::Response,
) -> io::Result<()>
) -> impl Send + Future<Output = io::Result<()>>
where
T: AsyncWrite + Unpin + Send,
{
FramedWrite::new(io, self.clone()).send(res).await?;
async move {
FramedWrite::new(io, self.clone()).send(res).await?;

Ok(())
Ok(())
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions protocols/request-response/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
- Allow configurable request and response sizes for `json` and `cbor` codec.
See [PR 5792](https://github.com/libp2p/rust-libp2p/pull/5792).

- Remove async-trait for RPIT. See [PR 5812](https:/github.com/libp2p/rust-libp2p/pull/5812).

## 0.27.1

- Deprecate `void` crate.
Expand Down
1 change: 0 additions & 1 deletion protocols/request-response/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ keywords = ["peer-to-peer", "libp2p", "networking"]
categories = ["network-programming", "asynchronous"]

[dependencies]
async-trait = "0.1"
cbor4ii = { version = "0.3.2", features = ["serde1", "use_std"], optional = true }
futures = { workspace = true }
libp2p-core = { workspace = true }
Expand Down
Loading
Loading