Skip to content

Commit

Permalink
Add versioning to request response protocols (#2362)
Browse files Browse the repository at this point in the history
## Linked Issues/PRs
<!-- List of related issues/PRs -->

Related issue: #1311 

## Context
The P2P service uses the
[request_response](https://docs.rs/libp2p-request-response/latest/libp2p_request_response/)
protocol to let peer request information (e.g. block headers, or
transactions in the tx pool) from other peers.
Currently, the P2P service provides a protocol `/fuel/req_res/0.0.1`,
which defines responses to have
an `Option` value. When a request from a peer is unsuccessful, we send a
`None` value as a response, wrapped around the relevant response type.

In practice, we want to be able to send more meaningful information in
responses when requests are unsuccessful, in the form of error codes.
This requires implementing a new version of the protocol
`/fuel/req_res/0.0.2` where Response messages are changed to contain
error codes.
Because we want to be backward compatible with peers that will only use
the version `0.0.1` of the protocol, we must account for requests sent
by, and responses sent to, those peers.

## Description
- Rename the `ResponseMessage` type in the P2P service to
`LegacyResponseMessage`
- Add a `ResponseMessageErrorCode` enum, which for now only contains a
`LegacyNodeEmptyResponse` enum.
- Implement a new `ResponseMessage` type that returns a `Result<_,
ResponseMessageErrorCode>` wrapped around enum variants
- Change P2P service to use the neq `ResponseMessage` type. Responses
previously returned as `None` are now returned as
`Err<LegacyNodeEmptyResponse>`.
- Change the `PostcardCodec` to serialize and deserialize according to
two different versions of the `/fuel/req_res` protocol:
`/fuel/req_res/0.0.1` and `/fuel/req_res/0.0.2`.
- Serialization and deserialization of responses using
`/fuel/req_res/0.0.1` are performed by first converting a
`ResponseMessage` into a `LegacyResponseMessage` and vicecersa, thus
preserving backward-compatibility
- Change the req_res behaviour used by the `P2P` service to use both
version of the req_res protocol.

### TODO (here or in different PRs)
- [ ] Investigate how protocols are exchanged during peer handshakes. We
don't want two nodes that both understand `/fuel/req_res/0.0.2` to
communicate using `/fuel/req_res/0.0.1`.
- [ ] Add more meaningful error codes.

## Checklist
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] 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
- [x] 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?
  • Loading branch information
acerone85 authored Oct 22, 2024
1 parent 4680a62 commit bb6a42c
Show file tree
Hide file tree
Showing 10 changed files with 377 additions and 77 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Added
- [2321](https://github.com/FuelLabs/fuel-core/pull/2321): New metrics for the txpool: "The size of transactions in the txpool" (`txpool_tx_size`), "The time spent by a transaction in the txpool in seconds" (`txpool_tx_time_in_txpool_seconds`), The number of transactions in the txpool (`txpool_number_of_transactions`), "The number of transactions pending verification before entering the txpool" (`txpool_number_of_transactions_pending_verification`), "The number of executable transactions in the txpool" (`txpool_number_of_executable_transactions`), "The time it took to select transactions for inclusion in a block in nanoseconds" (`txpool_select_transaction_time_nanoseconds`), The time it took to insert a transaction in the txpool in milliseconds (`txpool_insert_transaction_time_milliseconds`).
- [2362](https://github.com/FuelLabs/fuel-core/pull/2362): Added a new request_response protocol version `/fuel/req_res/0.0.2`. In comparison with `/fuel/req/0.0.1`, which returns an empty response when a request cannot be fulfilled, this version returns more meaningful error codes. Nodes still support the version `0.0.1` of the protocol to guarantee backward compatibility with fuel-core nodes. Empty responses received from nodes using the old protocol `/fuel/req/0.0.1` are automatically converted into an error `ProtocolV1EmptyResponse` with error code 0, which is also the only error code implemented. More specific error codes will be added in the future.

## [Version 0.40.0]

Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions crates/fuel-core/src/p2p_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use fuel_core_p2p::{
p2p_service::FuelP2PEvent,
request_response::messages::{
RequestMessage,
ResponseMessage,
V2ResponseMessage,
},
service::to_message_acceptance,
};
Expand Down Expand Up @@ -178,7 +178,7 @@ impl Bootstrap {
if request_message == RequestMessage::TxPoolAllTransactionsIds {
let _ = bootstrap.send_response_msg(
request_id,
ResponseMessage::TxPoolAllTransactionsIds(Some(vec![])),
V2ResponseMessage::TxPoolAllTransactionsIds(Ok(vec![])),
);
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/services/p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ rayon = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_with = { workspace = true }
sha2 = "0.10"
strum = { workspace = true }
strum_macros = { workspace = true }
thiserror = "1.0.47"
tokio = { workspace = true, features = ["sync"] }
tracing = { workspace = true }
Expand Down
15 changes: 8 additions & 7 deletions crates/services/p2p/src/behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
peer_report,
request_response::messages::{
RequestMessage,
ResponseMessage,
V2ResponseMessage,
},
};
use fuel_core_types::fuel_types::BlockHeight;
Expand Down Expand Up @@ -112,15 +112,16 @@ impl FuelBehaviour {
BlockHeight::default(),
);

let req_res_protocol =
core::iter::once((codec.get_req_res_protocol(), ProtocolSupport::Full));
let req_res_protocol = codec
.get_req_res_protocols()
.map(|protocol| (protocol, ProtocolSupport::Full));

let req_res_config = request_response::Config::default()
.with_request_timeout(p2p_config.set_request_timeout)
.with_max_concurrent_streams(p2p_config.max_concurrent_streams);

let request_response = request_response::Behaviour::with_codec(
codec,
codec.clone(),
req_res_protocol,
req_res_config,
);
Expand Down Expand Up @@ -165,9 +166,9 @@ impl FuelBehaviour {

pub fn send_response_msg(
&mut self,
channel: ResponseChannel<ResponseMessage>,
message: ResponseMessage,
) -> Result<(), ResponseMessage> {
channel: ResponseChannel<V2ResponseMessage>,
message: V2ResponseMessage,
) -> Result<(), V2ResponseMessage> {
self.request_response.send_response(channel, message)
}

Expand Down
10 changes: 7 additions & 3 deletions crates/services/p2p/src/codecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
},
request_response::messages::{
RequestMessage,
ResponseMessage,
V2ResponseMessage,
},
};
use libp2p::request_response;
Expand All @@ -28,18 +28,22 @@ 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 = ResponseMessage>
> + request_response::Codec<Request = RequestMessage, Response = V2ResponseMessage>
+ Clone
+ Send
+ 'static
{
/// Returns RequestResponse's Protocol
/// Needed for initialization of RequestResponse Behaviour
fn get_req_res_protocol(&self) -> <Self as request_response::Codec>::Protocol;
fn get_req_res_protocols(
&self,
) -> impl Iterator<Item = <Self as request_response::Codec>::Protocol>;
}
Loading

0 comments on commit bb6a42c

Please sign in to comment.