Skip to content

Commit f3159db

Browse files
acerone85xgreenx
andauthored
Remove p2pservice networkcodec trait (#2388)
## Linked Issues/PRs <!-- List of related issues/PRs --> Closes #2368 ## Description <!-- List of detailed changes --> In the P2P service, we couple together GossipSub and RequestResponse codecs in a `NetworkCodec` trait. In practice, this coupling is not necessary as the two codecs use different Messages and different logics for encoding/deconding messages. The only thing that they share in common is using Postcard as the data format to serialize and deserialize data, but this is not made clear in the network trait. This PR is a rework of Codecs in the libp2p service, which deletes the `NetworkCodec` trait and therefore the coupling between `GossipSubCodec` and `RequestResponseCodec`. In particular: - [ ] A new DataFormat trait is defined for serializing deserializing any SerDe datatype, and the helper functions for postcard serialization and deserialization` are moved to a struct `Postcard` that implements the `DataFormat` trait, - [ ] A new `BoundedCodec` struct that implements the `RequestResponse::Codec` trait for any `DataFormat` supersedes the old `PostcardCodec`. - [ ] A new `UnboundedCodec` struct that implements the `GossipsubCodec trait for any `DataFormat` supersedes the old `PostcardCodec`, - [ ] - [ ] Parts specific to the Request/Response protocol have been moved to a new `request_response::protocols` module`, - [ ] The FuelBehaviour and LibP2P service have been refactored to use different codecs for `Gossipsub` and `RequestResponse`. TODO: - [ ] Better Documentation - [x] Better names for modules - [ ] Maybe move DataFormats to a different part of the codebase? ## Checklist - [ ] Breaking changes are clearly marked as such in the PR description and changelog - [ ] New behavior is reflected in tests - [ ] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [ ] I have reviewed the code myself - [ ] I have created follow-up issues caused by this PR and linked them here ### After merging, notify other teams [Add or remove entries as needed] - [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/) - [ ] [Sway compiler](https://github.com/FuelLabs/sway/) - [ ] [Platform documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+) (for out-of-organization contributors, the person merging the PR will do this) - [ ] Someone else? --------- Co-authored-by: Green Baneling <[email protected]>
1 parent 8f447b5 commit f3159db

File tree

12 files changed

+399
-256
lines changed

12 files changed

+399
-256
lines changed

.changes/changed/2388.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Rework the P2P service codecs to avoid unnecessary coupling between components. The refactoring makes it explicit that the Gossipsub and RequestResponse codecs only share encoding/decoding functionalities from the Postcard codec. It also makes handling Gossipsub and RequestResponse messages completely independent of each other.

crates/fuel-core/src/p2p_test_helpers.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ use fuel_core_chain_config::{
2323
StateConfig,
2424
};
2525
use fuel_core_p2p::{
26-
codecs::postcard::PostcardCodec,
26+
codecs::{
27+
gossipsub::GossipsubMessageHandler,
28+
request_response::RequestResponseMessageHandler,
29+
},
2730
network_service::FuelP2PService,
2831
p2p_service::FuelP2PEvent,
2932
request_response::messages::{
@@ -172,10 +175,18 @@ impl Bootstrap {
172175
/// Spawn a bootstrap node.
173176
pub async fn new(node_config: &Config) -> anyhow::Result<Self> {
174177
let bootstrap_config = extract_p2p_config(node_config).await;
175-
let codec = PostcardCodec::new(bootstrap_config.max_block_size);
178+
let request_response_codec =
179+
RequestResponseMessageHandler::new(bootstrap_config.max_block_size);
180+
let gossipsub_codec = GossipsubMessageHandler::new();
176181
let (sender, _) =
177182
broadcast::channel(bootstrap_config.reserved_nodes.len().saturating_add(1));
178-
let mut bootstrap = FuelP2PService::new(sender, bootstrap_config, codec).await?;
183+
let mut bootstrap = FuelP2PService::new(
184+
sender,
185+
bootstrap_config,
186+
gossipsub_codec,
187+
request_response_codec,
188+
)
189+
.await?;
179190
bootstrap.start().await?;
180191

181192
let listeners = bootstrap.multiaddrs();

crates/services/p2p/src/behavior.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::{
22
codecs::{
33
postcard::PostcardCodec,
4-
NetworkCodec,
4+
request_response::RequestResponseMessageHandler,
5+
RequestResponseProtocols,
56
},
67
config::Config,
78
discovery,
@@ -67,7 +68,8 @@ pub struct FuelBehaviour {
6768
discovery: discovery::Behaviour,
6869

6970
/// RequestResponse protocol
70-
request_response: request_response::Behaviour<PostcardCodec>,
71+
request_response:
72+
request_response::Behaviour<RequestResponseMessageHandler<PostcardCodec>>,
7173

7274
/// The Behaviour to manage connection limits.
7375
connection_limits: connection_limits::Behaviour,
@@ -76,8 +78,8 @@ pub struct FuelBehaviour {
7678
impl FuelBehaviour {
7779
pub(crate) fn new(
7880
p2p_config: &Config,
79-
codec: PostcardCodec,
80-
) -> anyhow::Result<Self, anyhow::Error> {
81+
request_response_codec: RequestResponseMessageHandler<PostcardCodec>,
82+
) -> anyhow::Result<Self> {
8183
let local_public_key = p2p_config.keypair.public();
8284
let local_peer_id = PeerId::from_public_key(&local_public_key);
8385

@@ -131,7 +133,7 @@ impl FuelBehaviour {
131133
.with_max_established(Some(MAX_ESTABLISHED_CONNECTIONS)),
132134
);
133135

134-
let req_res_protocol = codec
136+
let req_res_protocol = request_response_codec
135137
.get_req_res_protocols()
136138
.map(|protocol| (protocol, ProtocolSupport::Full));
137139

@@ -140,7 +142,7 @@ impl FuelBehaviour {
140142
.with_max_concurrent_streams(p2p_config.max_concurrent_streams);
141143

142144
let request_response = request_response::Behaviour::with_codec(
143-
codec.clone(),
145+
request_response_codec.clone(),
144146
req_res_protocol,
145147
req_res_config,
146148
);

crates/services/p2p/src/codecs.rs

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,48 @@
1+
pub mod gossipsub;
12
pub mod postcard;
3+
pub mod request_response;
24

3-
use crate::{
4-
gossipsub::messages::{
5-
GossipTopicTag,
6-
GossipsubBroadcastRequest,
7-
GossipsubMessage,
8-
},
9-
request_response::messages::{
10-
RequestMessage,
11-
V2ResponseMessage,
12-
},
5+
use crate::gossipsub::messages::GossipTopicTag;
6+
use libp2p::request_response as libp2p_request_response;
7+
8+
use std::{
9+
borrow::Cow,
10+
io,
1311
};
14-
use libp2p::request_response;
15-
use std::io;
12+
13+
pub trait Encoder: Send {
14+
/// Returns the serialized object as a slice.
15+
fn into_bytes(self) -> Vec<u8>;
16+
}
17+
18+
/// The trait encodes the type to the bytes and passes it to the `Encoder`,
19+
/// which stores it and provides a reference to it. That allows gives more
20+
/// flexibility and more performant encoding, allowing the use of slices and arrays
21+
/// instead of vectors in some cases. Since the [`Encoder`] returns `Cow<[u8]>`,
22+
/// it is always possible to take ownership of the serialized value.
23+
pub trait Encode<T: ?Sized> {
24+
type Error;
25+
/// The encoder type that stores serialized object.
26+
type Encoder<'a>: Encoder
27+
where
28+
T: 'a;
29+
30+
/// Encodes the object to the bytes and passes it to the `Encoder`.
31+
fn encode<'a>(&self, t: &'a T) -> Result<Self::Encoder<'a>, Self::Error>;
32+
}
33+
34+
/// The trait decodes the type from the bytes.
35+
pub trait Decode<T> {
36+
type Error;
37+
/// Decodes the type `T` from the bytes.
38+
fn decode(&self, bytes: &[u8]) -> Result<T, Self::Error>;
39+
}
40+
41+
impl<'a> Encoder for Cow<'a, [u8]> {
42+
fn into_bytes(self) -> Vec<u8> {
43+
self.into_owned()
44+
}
45+
}
1646

1747
/// Implement this in order to handle serialization & deserialization of Gossipsub messages
1848
pub trait GossipsubCodec {
@@ -28,22 +58,10 @@ pub trait GossipsubCodec {
2858
) -> Result<Self::ResponseMessage, io::Error>;
2959
}
3060

31-
// TODO: https://github.com/FuelLabs/fuel-core/issues/2368
32-
// Remove this trait
33-
/// Main Codec trait
34-
/// Needs to be implemented and provided to FuelBehaviour
35-
pub trait NetworkCodec:
36-
GossipsubCodec<
37-
RequestMessage = GossipsubBroadcastRequest,
38-
ResponseMessage = GossipsubMessage,
39-
> + request_response::Codec<Request = RequestMessage, Response = V2ResponseMessage>
40-
+ Clone
41-
+ Send
42-
+ 'static
43-
{
61+
pub trait RequestResponseProtocols: libp2p_request_response::Codec {
4462
/// Returns RequestResponse's Protocol
4563
/// Needed for initialization of RequestResponse Behaviour
4664
fn get_req_res_protocols(
4765
&self,
48-
) -> impl Iterator<Item = <Self as request_response::Codec>::Protocol>;
66+
) -> impl Iterator<Item = <Self as libp2p_request_response::Codec>::Protocol>;
4967
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
use std::io;
2+
3+
use fuel_core_types::fuel_tx::Transaction;
4+
5+
use crate::gossipsub::messages::{
6+
GossipTopicTag,
7+
GossipsubBroadcastRequest,
8+
GossipsubMessage,
9+
};
10+
11+
use super::{
12+
Decode,
13+
Encode,
14+
Encoder,
15+
GossipsubCodec,
16+
};
17+
18+
#[derive(Debug, Clone, Default)]
19+
pub struct GossipsubMessageHandler<Codec> {
20+
pub(crate) codec: Codec,
21+
}
22+
23+
impl<Codec> GossipsubCodec for GossipsubMessageHandler<Codec>
24+
where
25+
Codec:
26+
Encode<Transaction, Error = io::Error> + Decode<Transaction, Error = io::Error>,
27+
{
28+
type RequestMessage = GossipsubBroadcastRequest;
29+
type ResponseMessage = GossipsubMessage;
30+
31+
fn encode(&self, data: Self::RequestMessage) -> Result<Vec<u8>, io::Error> {
32+
match data {
33+
GossipsubBroadcastRequest::NewTx(tx) => {
34+
Ok(self.codec.encode(&tx)?.into_bytes())
35+
}
36+
}
37+
}
38+
39+
fn decode(
40+
&self,
41+
encoded_data: &[u8],
42+
gossipsub_tag: GossipTopicTag,
43+
) -> Result<Self::ResponseMessage, io::Error> {
44+
let decoded_response = match gossipsub_tag {
45+
GossipTopicTag::NewTx => {
46+
GossipsubMessage::NewTx(self.codec.decode(encoded_data)?)
47+
}
48+
};
49+
50+
Ok(decoded_response)
51+
}
52+
}

0 commit comments

Comments
 (0)