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

Remove p2pservice networkcodec trait #2388

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b6f7353
Request/Response: remove NetworkCodec trait
acerone85 Oct 23, 2024
529d1d6
Dinstinguish between Format being used and Codec for requests and res…
acerone85 Oct 23, 2024
a4465d5
Move RequestResponse protocol definitions to dedicated module
acerone85 Oct 23, 2024
7b858c5
Decouple GossibSub and RequestResponse codecs
acerone85 Oct 23, 2024
f2cd720
Merge branch 'master' into 2368-remove-p2pservice-networkcodec-trait
acerone85 Oct 28, 2024
1aa98dc
Simplify DataFormat trait
acerone85 Oct 28, 2024
79b507d
Use Encode and Decode trait in favour of DataFormat
acerone85 Oct 28, 2024
ead761a
Rename codecs to message handlers
acerone85 Oct 28, 2024
ed29376
Format
acerone85 Oct 28, 2024
138716b
Encode trait function takes &self as argument
acerone85 Oct 28, 2024
58ffe67
Minor improvements
acerone85 Oct 28, 2024
c564941
Decode trait function takes &self as argument
acerone85 Oct 28, 2024
c5aa362
DataFormat is now Codec
acerone85 Oct 28, 2024
89b022c
Typo
acerone85 Oct 28, 2024
45bbd91
CHANGELOG
acerone85 Oct 28, 2024
caf4f00
Add issue to TODO
acerone85 Oct 28, 2024
a55f695
Rename lifetime
acerone85 Oct 28, 2024
fc2d71d
Formatting
acerone85 Oct 29, 2024
136e6b4
Placate Clippy
acerone85 Oct 29, 2024
5bd9e21
Merge branch 'master' into 2368-remove-p2pservice-networkcodec-trait
acerone85 Nov 15, 2024
029fbff
Fix test compilation
acerone85 Nov 15, 2024
39e1ed9
Merge branch 'master' into 2368-remove-p2pservice-networkcodec-trait
acerone85 Nov 26, 2024
05c50fd
Reference todo issue
acerone85 Nov 26, 2024
e9f1404
Avoid dereference
acerone85 Nov 26, 2024
b94c451
Todo: max_response_size should be u64
acerone85 Nov 26, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2378](https://github.com/FuelLabs/fuel-core/pull/2378): Use cached hash of the topic instead of calculating it on each publishing gossip message.
- [2429](https://github.com/FuelLabs/fuel-core/pull/2429): Introduce custom enum for representing result of running service tasks
- [2377](https://github.com/FuelLabs/fuel-core/pull/2377): Add more errors that can be returned as responses when using protocol `/fuel/req_res/0.0.2`. The errors supported are `ProtocolV1EmptyResponse` (status code `0`) for converting empty responses sent via protocol `/fuel/req_res/0.0.1`, `RequestedRangeTooLarge`(status code `1`) if the client requests a range of objects such as sealed block headers or transactions too large, `Timeout` (status code `2`) if the remote peer takes too long to fulfill a request, or `SyncProcessorOutOfCapacity` if the remote peer is fulfilling too many requests concurrently.
- [2388](https://github.com/FuelLabs/fuel-core/pull/2388): 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.

#### Breaking
- [2389](https://github.com/FuelLabs/fuel-core/pull/2258): Updated the `messageProof` GraphQL schema to return a non-nullable `MessageProof`.
Expand Down
17 changes: 14 additions & 3 deletions crates/fuel-core/src/p2p_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ use fuel_core_chain_config::{
StateConfig,
};
use fuel_core_p2p::{
codecs::postcard::PostcardCodec,
codecs::{
gossipsub::GossipsubMessageHandler,
request_response::RequestResponseMessageHandler,
},
network_service::FuelP2PService,
p2p_service::FuelP2PEvent,
request_response::messages::{
Expand Down Expand Up @@ -142,10 +145,18 @@ impl Bootstrap {
/// Spawn a bootstrap node.
pub async fn new(node_config: &Config) -> anyhow::Result<Self> {
let bootstrap_config = extract_p2p_config(node_config).await;
let codec = PostcardCodec::new(bootstrap_config.max_block_size);
let request_response_codec =
RequestResponseMessageHandler::new(bootstrap_config.max_block_size);
let gossipsub_codec = GossipsubMessageHandler::new();
let (sender, _) =
broadcast::channel(bootstrap_config.reserved_nodes.len().saturating_add(1));
let mut bootstrap = FuelP2PService::new(sender, bootstrap_config, codec).await?;
let mut bootstrap = FuelP2PService::new(
sender,
bootstrap_config,
gossipsub_codec,
request_response_codec,
)
.await?;
bootstrap.start().await?;

let listeners = bootstrap.multiaddrs();
Expand Down
15 changes: 10 additions & 5 deletions crates/services/p2p/src/behavior.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{
codecs::{
postcard::PostcardCodec,
NetworkCodec,
request_response::RequestResponseMessageHandler,
RequestResponseProtocols,
},
config::Config,
discovery,
Expand Down Expand Up @@ -59,11 +60,15 @@ pub struct FuelBehaviour {
discovery: discovery::Behaviour,

/// RequestResponse protocol
request_response: request_response::Behaviour<PostcardCodec>,
request_response:
request_response::Behaviour<RequestResponseMessageHandler<PostcardCodec>>,
}

impl FuelBehaviour {
pub(crate) fn new(p2p_config: &Config, codec: PostcardCodec) -> anyhow::Result<Self> {
pub(crate) fn new(
p2p_config: &Config,
request_response_codec: RequestResponseMessageHandler<PostcardCodec>,
) -> anyhow::Result<Self> {
let local_public_key = p2p_config.keypair.public();
let local_peer_id = PeerId::from_public_key(&local_public_key);

Expand Down Expand Up @@ -110,7 +115,7 @@ impl FuelBehaviour {
BlockHeight::default(),
);

let req_res_protocol = codec
let req_res_protocol = request_response_codec
.get_req_res_protocols()
.map(|protocol| (protocol, ProtocolSupport::Full));

Expand All @@ -119,7 +124,7 @@ impl FuelBehaviour {
.with_max_concurrent_streams(p2p_config.max_concurrent_streams);

let request_response = request_response::Behaviour::with_codec(
codec.clone(),
request_response_codec.clone(),
req_res_protocol,
req_res_config,
);
Expand Down
82 changes: 56 additions & 26 deletions crates/services/p2p/src/codecs.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,60 @@
pub mod gossipsub;
pub mod postcard;
pub mod request_response;

use crate::{
gossipsub::messages::{
GossipTopicTag,
GossipsubBroadcastRequest,
GossipsubMessage,
},
request_response::messages::{
RequestMessage,
V2ResponseMessage,
},
use crate::gossipsub::messages::GossipTopicTag;
use libp2p::request_response as libp2p_request_response;

use std::{
borrow::Cow,
io,
};
use libp2p::request_response;
use std::io;

// TODO: https://github.com/FuelLabs/fuel-core/issues/2403
// This trait is largely a copy-paste from the storage crate.
// It would be best to have this trait in a separate crate that both storage and p2p can depend on.
pub trait Encoder: Send {
/// Returns the serialized object as a slice.
fn as_bytes(&self) -> Cow<[u8]>;
}

/// The trait encodes the type to the bytes and passes it to the `Encoder`,
/// which stores it and provides a reference to it. That allows gives more
/// flexibility and more performant encoding, allowing the use of slices and arrays
/// instead of vectors in some cases. Since the [`Encoder`] returns `Cow<[u8]>`,
/// it is always possible to take ownership of the serialized value.
// TODO: https://github.com/FuelLabs/fuel-core/issues/2403
// This trait is largely a copy-paste from the storage crate.
// It would be best to have this trait in a separate crate that both storage and p2p can depend on.
pub trait Encode<T: ?Sized> {
type Error;
/// The encoder type that stores serialized object.
type Encoder<'a>: Encoder
where
T: 'a;

/// Encodes the object to the bytes and passes it to the `Encoder`.
fn encode<'a>(&self, t: &'a T) -> Result<Self::Encoder<'a>, Self::Error>;
}

/// The trait decodes the type from the bytes.
// TODO: https://github.com/FuelLabs/fuel-core/issues/2403
// This trait is largely a copy-paste from the storage crate.
// It would be best to have this trait in a separate crate that both storage and p2p can depend on.
pub trait Decode<T> {
type Error;
/// Decodes the type `T` from the bytes.
fn decode(&self, bytes: &[u8]) -> Result<T, Self::Error>;
}

impl<'a> Encoder for Cow<'a, [u8]> {
fn as_bytes(&self) -> Cow<'_, [u8]> {
match self {
Cow::Borrowed(borrowed) => Cow::Borrowed(borrowed),
Cow::Owned(owned) => Cow::Borrowed(owned.as_ref()),
}
}
}

/// Implement this in order to handle serialization & deserialization of Gossipsub messages
pub trait GossipsubCodec {
Expand All @@ -28,22 +70,10 @@ pub trait GossipsubCodec {
) -> Result<Self::ResponseMessage, io::Error>;
}

// TODO: https://github.com/FuelLabs/fuel-core/issues/2368
// Remove this trait
/// Main Codec trait
/// Needs to be implemented and provided to FuelBehaviour
pub trait NetworkCodec:
GossipsubCodec<
RequestMessage = GossipsubBroadcastRequest,
ResponseMessage = GossipsubMessage,
> + request_response::Codec<Request = RequestMessage, Response = V2ResponseMessage>
+ Clone
+ Send
+ 'static
{
pub trait RequestResponseProtocols: libp2p_request_response::Codec {
/// Returns RequestResponse's Protocol
/// Needed for initialization of RequestResponse Behaviour
fn get_req_res_protocols(
&self,
) -> impl Iterator<Item = <Self as request_response::Codec>::Protocol>;
) -> impl Iterator<Item = <Self as libp2p_request_response::Codec>::Protocol>;
}
53 changes: 53 additions & 0 deletions crates/services/p2p/src/codecs/gossipsub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use std::io;

use fuel_core_types::fuel_tx::Transaction;

use crate::gossipsub::messages::{
GossipTopicTag,
GossipsubBroadcastRequest,
GossipsubMessage,
};

use super::{
Decode,
Encode,
Encoder,
GossipsubCodec,
};

#[derive(Debug, Clone, Default)]
pub struct GossipsubMessageHandler<Codec> {
pub(crate) codec: Codec,
}

impl<Codec> GossipsubCodec for GossipsubMessageHandler<Codec>
where
Codec: Encode<Transaction, Error = io::Error>
+ Decode<Transaction, Error = io::Error>
+ Send,
{
type RequestMessage = GossipsubBroadcastRequest;
type ResponseMessage = GossipsubMessage;

fn encode(&self, data: Self::RequestMessage) -> Result<Vec<u8>, io::Error> {
match data {
GossipsubBroadcastRequest::NewTx(tx) => {
Ok(self.codec.encode(&tx)?.as_bytes().into_owned())
}
}
}

fn decode(
&self,
encoded_data: &[u8],
gossipsub_tag: GossipTopicTag,
) -> Result<Self::ResponseMessage, io::Error> {
let decoded_response = match gossipsub_tag {
GossipTopicTag::NewTx => {
GossipsubMessage::NewTx(self.codec.decode(encoded_data)?)
}
};

Ok(decoded_response)
}
}
Loading
Loading