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

P2pService: Make max_response_size in PostcardCodec a u64 #2460

Open
wants to merge 2 commits into
base: 2368-remove-p2pservice-networkcodec-trait
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [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.
- [2460](https://github.com/FuelLabs/fuel-core/pull/2460): The type of the `max_response_size`for the postcard codec used in `RequestResponse` protocols has been changed from `usize` to `u64`.

#### Breaking
- [2389](https://github.com/FuelLabs/fuel-core/pull/2258): Updated the `messageProof` GraphQL schema to return a non-nullable `MessageProof`.
Expand Down
2 changes: 1 addition & 1 deletion crates/services/p2p/src/codecs/postcard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::{
pub struct PostcardCodec;

impl RequestResponseMessageHandler<PostcardCodec> {
pub fn new(max_block_size: usize) -> Self {
pub fn new(max_block_size: u64) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can use NonZeroU64, then we don't need check below=)

assert_ne!(
max_block_size, 0,
"RequestResponseMessageHandler does not support zero block size"
Expand Down
4 changes: 1 addition & 3 deletions crates/services/p2p/src/codecs/request_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ pub struct RequestResponseMessageHandler<Codec> {
/// Used for `max_size` parameter when reading Response Message
/// Necessary in order to avoid DoS attacks
/// Currently the size mostly depends on the max size of the Block
// TODO: https://github.com/FuelLabs/fuel-core/issues/2459.
// Make this a u64 instead of usize.
pub(crate) max_response_size: usize,
pub(crate) max_response_size: u64,
Copy link
Member

Choose a reason for hiding this comment

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

I think we can reduce the size here ~ u64::MAX is way larger than any data we'd be sending via the req-res protocol

}

/// Since Postcard does not support async reads or writes out of the box
Expand Down
2 changes: 1 addition & 1 deletion crates/services/p2p/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ where
broadcast.reserved_peers_broadcast.clone(),
config,
GossipsubMessageHandler::new(),
RequestResponseMessageHandler::new(max_block_size),
RequestResponseMessageHandler::new(max_block_size.try_into()?),
)
.await?;
p2p_service.update_block_height(last_height);
Expand Down
Loading