Add versioning to request response protocols #2362

acerone85 · 2024-10-15T20:34:57Z

Linked Issues/PRs

Related issue: #1311

Context

The P2P service uses the 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
New behavior is reflected in tests
The specification 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
Sway compiler
Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
Someone else?

…_service

…l in the PeerManager table

…the request response protocol to use

…-requests

This reverts commit f3b7db3b0f4aa92f2606c87ed6f65f4798734889.

…ses to unsuccessful requests

AurelienFT · 2024-10-20T15:02:33Z

crates/services/p2p/src/request_response/messages.rs

@@ -32,14 +34,77 @@ pub enum RequestMessage {
    TxPoolFullTransactions(Vec<TxId>),
 }

+// TODO: Do we want explicit status codes or an Error type?


I think it's not a big problem to have more detailed error (with specific block id or thing like that) as soon as we add a limit on the size of the error.

MitchTurner

WIP. I'm gonna give this another pass soon.

MitchTurner · 2024-10-22T09:27:44Z

crates/services/p2p/src/request_response/messages.rs

 #[derive(Debug, Clone, Serialize, Deserialize)]
-pub enum ResponseMessage {
+pub enum LegacyResponseMessage {


I'm trying to decide if it would make more sense to name this V1ResponseMessage? The term Legacy will become ambiguous once we add a third protocol."

Same question applies to ResponseMessage.

Or you could name it something descriptive like the protocol consts, e.g. REQUEST_RESPONSE_WITH_ERROR_CODES_PROTOCOL_ID

MitchTurner · 2024-10-22T09:36:06Z

crates/services/p2p/src/codecs/postcard.rs

@@ -160,30 +176,200 @@ impl GossipsubCodec for PostcardCodec {
    }
 }

+// TODO: Remove this NetworkCodec


Have you created an issue? And I think the TODO should be on the trait itself, rather than one of the impls.

Yes, issue moved to the trait definition and referenced in b710d8d

MitchTurner · 2024-10-22T09:57:47Z

crates/services/p2p/src/codecs/postcard.rs

+    #[tokio::test]
+    async fn serialzation_roundtrip_using_v2() {
+        let sealed_block_headers = vec![SealedBlockHeader::default()];
+        let response = ResponseMessage::SealedHeaders(Ok(sealed_block_headers.clone()));
+        let mut codec = PostcardCodec::new(1024);
+        let mut buf = Vec::with_capacity(1024);
+        codec
+            .write_response(&PostcardProtocol::V2, &mut buf, response)
+            .await
+            .expect("Valid Vec<SealedBlockHeader> should be serialized using v1");
+
+        let deserialized = codec
+            .read_response(&PostcardProtocol::V2, &mut buf.as_slice())
+            .await
+            .expect("Valid Vec<SealedBlockHeader> should be deserialized using v1");
+
+        match deserialized {
+            ResponseMessage::SealedHeaders(Ok(sealed_block_headers_response)) => {
+                assert_eq!(sealed_block_headers, sealed_block_headers_response);
+            }
+            other => {
+                panic!("Deserialized to {other:?}, expected {sealed_block_headers:?}")
+            }
+        }
+    }


Nit: I prefer all UTs include given/when/then blocks and start the name with the SUT.

Suggested change

#[tokio::test]

async fn serialzation_roundtrip_using_v2() {

let sealed_block_headers = vec![SealedBlockHeader::default()];

let response = ResponseMessage::SealedHeaders(Ok(sealed_block_headers.clone()));

let mut codec = PostcardCodec::new(1024);

let mut buf = Vec::with_capacity(1024);

codec

.write_response(&PostcardProtocol::V2, &mut buf, response)

.await

.expect("Valid Vec<SealedBlockHeader> should be serialized using v1");

let deserialized = codec

.read_response(&PostcardProtocol::V2, &mut buf.as_slice())

.await

.expect("Valid Vec<SealedBlockHeader> should be deserialized using v1");

match deserialized {

ResponseMessage::SealedHeaders(Ok(sealed_block_headers_response)) => {

assert_eq!(sealed_block_headers, sealed_block_headers_response);

}

other => {

panic!("Deserialized to {other:?}, expected {sealed_block_headers:?}")

}

}

}

#[tokio::test]

async fn codec__v1_serialzation_roundtrip() {

/// given

let sealed_block_headers = vec![SealedBlockHeader::default()];

let response = ResponseMessage::SealedHeaders(Ok(sealed_block_headers.clone()));

let mut codec = PostcardCodec::new(1024);

let mut buf = Vec::with_capacity(1024);

/// when

codec

.write_response(&PostcardProtocol::V2, &mut buf, response)

.await

.expect("Valid Vec<SealedBlockHeader> should be serialized using v1");

let deserialized = codec

.read_response(&PostcardProtocol::V2, &mut buf.as_slice())

.await

.expect("Valid Vec<SealedBlockHeader> should be deserialized using v1");

/// then

match deserialized {

ResponseMessage::SealedHeaders(Ok(sealed_block_headers_response)) => {

assert_eq!(sealed_block_headers, sealed_block_headers_response);

}

other => {

panic!("Deserialized to {other:?}, expected {sealed_block_headers:?}")

}

}

}

This is a round-trip test, which I consider an exception to the rule that each test should be testing one thing (it's just so succinct)

Given/When/Then sections added in cec899c

MitchTurner · 2024-10-22T10:02:04Z

crates/services/p2p/src/codecs/postcard.rs

+            // We cannot access the codec trait from an old node here, 
+            // so we deserialize directly using the `LegacyResponseMessage` type.
+            deserialize::<LegacyResponseMessage>(&buf).expect("Deserialization as LegacyResponseMessage should succeed");
+        match deserialized_as_legacy {


I'd prefer a matches! macro here (and probably on all the tests) because when I'm reading a test I'm always looking for an explicit expected condition, not an implicit one like here.

nice :) I have refactored the tests to use matches! in 4280781

MitchTurner · 2024-10-22T10:06:49Z

crates/services/p2p/src/codecs/postcard.rs

+    #[tokio::test]
+    async fn backward_compatibility_v1_read_error_response() {
+        let response = ResponseMessage::SealedHeaders(Err(
+            ResponseMessageErrorCode::ProtocolV1EmptyResponse,
+        ));
+        let mut codec = PostcardCodec::new(1024);
+        let mut buf = Vec::with_capacity(1024);
+        codec
+            .write_response(&PostcardProtocol::V1, &mut buf, response.clone())
+            .await
+            .expect("Valid Vec<SealedBlockHeader> is serialized using v1");
+
+        let deserialized_as_legacy =
+            // We cannot access the codec trait from an old node here, 
+            // so we deserialize directly using the `LegacyResponseMessage` type.
+            deserialize::<LegacyResponseMessage>(&buf).expect("Deserialization as LegacyResponseMessage should succeed");
+        match deserialized_as_legacy {
+            LegacyResponseMessage::SealedHeaders(None) => {}
+            other => {
+                panic!("Deserialized to {other:?}, expected {response:?}")
+            }
+        }
+    }


Another example of how to structure the tests:

Suggested change

#[tokio::test]

async fn backward_compatibility_v1_read_error_response() {

let response = ResponseMessage::SealedHeaders(Err(

ResponseMessageErrorCode::ProtocolV1EmptyResponse,

));

let mut codec = PostcardCodec::new(1024);

let mut buf = Vec::with_capacity(1024);

codec

.write_response(&PostcardProtocol::V1, &mut buf, response.clone())

.await

.expect("Valid Vec<SealedBlockHeader> is serialized using v1");

let deserialized_as_legacy =

// We cannot access the codec trait from an old node here,

// so we deserialize directly using the `LegacyResponseMessage` type.

deserialize::<LegacyResponseMessage>(&buf).expect("Deserialization as LegacyResponseMessage should succeed");

match deserialized_as_legacy {

LegacyResponseMessage::SealedHeaders(None) => {}

other => {

panic!("Deserialized to {other:?}, expected {response:?}")

}

}

}

#[tokio::test]

async fn codec__write_response_is_backwards_compatible_with_v1() {

// given

let response = ResponseMessage::SealedHeaders(Err(

ResponseMessageErrorCode::ProtocolV1EmptyResponse,

));

let mut codec = PostcardCodec::new(1024);

let mut buf = Vec::with_capacity(1024);

// when

codec

.write_response(&PostcardProtocol::V1, &mut buf, response.clone())

.await

.expect("Valid Vec<SealedBlockHeader> is serialized using v1");

let deserialized_as_legacy =

// We cannot access the codec trait from an old node here,

// so we deserialize directly using the `LegacyResponseMessage` type.

deserialize::<LegacyResponseMessage>(&buf).expect("Deserialization as LegacyResponseMessage should succeed");

// then

match deserialized_as_legacy {

LegacyResponseMessage::SealedHeaders(None) => {}

other => {

panic!("Deserialized to {other:?}, expected {response:?}")

}

}

}

I have changed the names of the tests to follow your suggestions in 6276af5

…-requests

xgreenx · 2024-10-22T17:50:09Z

crates/services/p2p/src/codecs/postcard.rs

+pub enum PostcardProtocol {
+    #[default]
+    V1,
+    V2,
+}


Why V1 is default if we prefer V2?

Thanks, this is a leftover for when I was doing protocol selection manually. In fact, we don't even need a Default version.
I have removed it in the follow-up MR: #2377 (commit ea4842c)

xgreenx · 2024-10-22T17:52:06Z

crates/services/p2p/src/p2p_service.rs

-                            ResponseMessage::Transactions(v) => {
-                                c.send((peer, Ok(v))).is_ok()
+                            V2ResponseMessage::Transactions(v) => {
+                                c.send((peer, Ok(v.ok()))).is_ok()


It would be nice to have the same TODO here

xgreenx · 2024-10-22T17:52:10Z

crates/services/p2p/src/p2p_service.rs

-                            ResponseMessage::TxPoolAllTransactionsIds(v) => {
-                                c.send((peer, Ok(v))).is_ok()
+                            V2ResponseMessage::TxPoolAllTransactionsIds(v) => {
+                                c.send((peer, Ok(v.ok()))).is_ok()


It would be nice to have the same TODO here

xgreenx · 2024-10-22T17:52:14Z

crates/services/p2p/src/p2p_service.rs

-                            ResponseMessage::TxPoolFullTransactions(v) => {
-                                c.send((peer, Ok(v))).is_ok()
+                            V2ResponseMessage::TxPoolFullTransactions(v) => {
+                                c.send((peer, Ok(v.ok()))).is_ok()


It would be nice to have the same TODO here

These are addressed in the follow-up MR (#2377), but I agree that in the future I should duplicate the TODO comment in all relevant places

## Linked Issues/PRs  Closes #1311 ## Description  This PR follows #2362, which introduces a new version of the protocol `/fuel/req_response/0.0.2` which allows to return Errors in the response. This PR makes the following changes: - [X] the different errors that can be returned in a response are implemented. - [X] The type of `ResponseSender` used by a peer to propagate received responses upstream is changed to allow the rrors to be propagated - [X] Error Responses received by a peer following a request are displayed with as log messages with `warning` level. ## 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?

@acerone85

## Version v0.41.0 ### Added - [2547](#2547): Replace the old Graphql gas price provider adapter with the ArcGasPriceEstimate. - [2445](#2445): Added GQL endpoint for querying asset details. - [2442](#2442): Add uninitialized task for V1 gas price service - [2154](#2154): Added `Unknown` variant to `ConsensusParameters` graphql queries - [2154](#2154): Added `Unknown` variant to `Block` graphql queries - [2154](#2154): Added `TransactionType` type in `fuel-client` - [2321](#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 microseconds (`txpool_select_transactions_time_microseconds`) - The time it took to insert a transaction in the txpool in microseconds (`transaction_insertion_time_in_thread_pool_microseconds`) - [2385](#2385): Added new histogram buckets for some of the TxPool metrics, optimize the way they are collected. - [2347](#2364): Add activity concept in order to protect against infinitely increasing DA gas price scenarios - [2362](#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. - [2386](#2386): Add a flag to define the maximum number of file descriptors that RocksDB can use. By default it's half of the OS limit. - [2376](#2376): Add a way to fetch transactions in P2P without specifying a peer. - [2361](#2361): Add caches to the sync service to not reask for data it already fetched from the network. - [2327](#2327): Add more services tests and more checks of the pool. Also add an high level documentation for users of the pool and contributors. - [2416](#2416): Define the `GasPriceServiceV1` task. - [2447](#2447): Use new `expiration` policy in the transaction pool. Add a mechanism to prune the transactions when they expired. - [1922](#1922): Added support for posting blocks to the shared sequencer. - [2033](#2033): Remove `Option<BlockHeight>` in favor of `BlockHeightQuery` where applicable. - [2490](#2490): Added pagination support for the `balances` GraphQL query, available only when 'balances indexation' is enabled. - [2439](#2439): Add gas costs for the two new zk opcodes `ecop` and `eadd` and the benches that allow to calibrate them. - [2472](#2472): Added the `amountU128` field to the `Balance` GraphQL schema, providing the total balance as a `U128`. The existing `amount` field clamps any balance exceeding `U64` to `u64::MAX`. - [2526](#2526): Add possibility to not have any cache set for RocksDB. Add an option to either load the RocksDB columns families on creation of the database or when the column is used. - [2532](#2532): Getters for inner rocksdb database handles. - [2524](#2524): Adds a new lock type which is optimized for certain workloads to the txpool and p2p services. - [2535](#2535): Expose `backup` and `restore` APIs on the `CombinedDatabase` struct to create portable backups and restore from them. - [2550](#2550): Add statistics and more limits infos about txpool on the node_info endpoint ### Fixed - [2560](#2560): Fix flaky test by increasing timeout - [2558](#2558): Rename `cost` and `reward` to remove `excess` wording - [2469](#2469): Improved the logic for syncing the gas price database with on_chain database - [2365](#2365): Fixed the error during dry run in the case of race condition. - [2366](#2366): The `importer_gas_price_for_block` metric is properly collected. - [2369](#2369): The `transaction_insertion_time_in_thread_pool_milliseconds` metric is properly collected. - [2413](#2413): block production immediately errors if unable to lock the mutex. - [2389](#2389): Fix construction of reverse iterator in RocksDB. - [2479](#2479): Fix an error on the last iteration of the read and write sequential opcodes on contract storage. - [2478](#2478): Fix proof created by `message_receipts_proof` function by ignoring the receipts from failed transactions to match `message_outbox_root`. - [2485](#2485): Hardcode the timestamp of the genesis block and version of `tai64` to avoid breaking changes for us. - [2511](#2511): Fix backward compatibility of V0Metadata in gas price db. ### Changed - [2469](#2469): Updated adapter for querying costs from DA Block committer API - [2469](#2469): Use the gas price from the latest block to estimate future gas prices - [2501](#2501): Use gas price from block for estimating future gas prices - [2468](#2468): Abstract unrecorded blocks concept for V1 algorithm, create new storage impl. Introduce `TransactionableStorage` trait to allow atomic changes to the storage. - [2295](#2295): `CombinedDb::from_config` now respects `state_rewind_policy` with tmp RocksDB. - [2378](#2378): Use cached hash of the topic instead of calculating it on each publishing gossip message. - [2438](#2438): Refactored service to use new implementation of `StorageRead::read` that takes an offset in input. - [2429](#2429): Introduce custom enum for representing result of running service tasks - [2377](#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. - [2233](#2233): Introduce a new column `modification_history_v2` for storing the modification history in the historical rocksDB. Keys in this column are stored in big endian order. Changed the behaviour of the historical rocksDB to write changes for new block heights to the new column, and to perform lookup of values from the `modification_history_v2` table first, and then from the `modification_history` table, performing a migration upon access if necessary. - [2383](#2383): The `balance` and `balances` GraphQL query handlers now use index to provide the response in a more performant way. As the index is not created retroactively, the client must be initialized with an empty database and synced from the genesis block to utilize it. Otherwise, the legacy way of retrieving data will be used. - [2463](#2463): The `coinsToSpend` GraphQL query handler now uses index to provide the response in a more performant way. As the index is not created retroactively, the client must be initialized with an empty database and synced from the genesis block to utilize it. Otherwise, the legacy way of retrieving data will be used. - [2556](#2556): Ensure that the `last_recorded_height` is set for the DA gas price source. #### Breaking - [2469](#2469): Move from `GasPriceServicev0` to `GasPriceServiceV1`. Include new config values. - [2438](#2438): The `fuel-core-client` can only work with new version of the `fuel-core`. The `0.40` and all older versions are not supported. - [2438](#2438): Updated `fuel-vm` to `0.59.1` release. Check [release notes](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.59.0) for more details. - [2389](#2258): Updated the `messageProof` GraphQL schema to return a non-nullable `MessageProof`. - [2154](#2154): Transaction graphql endpoints use `TransactionType` instead of `fuel_tx::Transaction`. - [2446](#2446): Use graphiql instead of graphql-playground due to known vulnerability and stale development. - [2379](#2379): Change `kv_store::Value` to be `Arc<[u8]>` instead of `Arc<Vec<u8>>`. - [2490](#2490): Updated GraphQL complexity calculation for `balances` query to account for pagination (`first`/`last`) and nested field complexity (`child_complexity`). Queries with large pagination values or deeply nested fields may have higher complexity costs. - [2463](#2463): 'CoinsQueryError::MaxCoinsReached` variant has been removed. The `InsufficientCoins` variant has been renamed to `InsufficientCoinsForTheMax` and it now contains the additional `max` field - [2463](#2463): The number of excluded ids in the `coinsToSpend` GraphQL query is now limited to the maximum number of inputs allowed in transaction. - [2463](#2463): The `coinsToSpend` GraphQL query may now return different coins, depending whether the indexation is enabled or not. However, regardless of the differences, the returned coins will accurately reflect the current state of the database within the context of the query. - [2526](#2526): By default the cache of RocksDB is now disabled instead of being `1024 * 1024 * 1024`. ## What's Changed * Add metrics to TxPool by @acerone85 in #2321 * Fix collection of gas price metric by @rafal-ch in #2366 * Add documentation to run a ignition node in readme by @AurelienFT in #2363 * Fix collection of tx pool insertion time metric by @rafal-ch in #2369 * Add versioning to request response protocols by @acerone85 in #2362 * Return reason of why proof cant be generated by @rafal-ch in #2258 * p2p: use precalculated topic hash by @yaziciahmet in #2378 * Remove ignore RUSTSEC-2024-0336 by @AurelienFT in #2384 * Deal with negative feed back loop in DA gas price by @MitchTurner in #2364 * Add new flag for maximum file descriptors in rocksdb. by @AurelienFT in #2386 * Add codeowners for gas price algorithm crate by @rafal-ch in #2404 * Weekly `cargo update` by @github-actions in #2373 * chore(gas_price_service): initialize v1 metadata by @rymnc in #2288 * chore(gas_price_service_v0): remove unused trait impl by @rymnc in #2410 * Update tai64 to fix the wrong time offset by @AurelienFT in #2409 * fix(block_producer): immediately return error if lock cannot be acquired during production by @rymnc in #2413 * Add a way to fetch transactions in P2P without specifying a peer by @AurelienFT in #2376 * Add a new code owner for tx pool by @AurelienFT in #2417 * Satisfy clippy in `gas-price-analysis` by @rafal-ch in #2418 * Txpool metrics update by @rafal-ch in #2385 * Improve TxPool tests and documentation by @AurelienFT in #2327 * feat(gas_price_service_v1): define RunnableTask for GasPriceServiceV1 by @rymnc in #2416 * Return reason of why proof cant be generated (api change) by @rafal-ch in #2389 * Fuel/Request_Response v0.0.2: More meaningful error messages by @acerone85 in #2377 * Fix reverse iterator in RocksDB by @AurelienFT in #2398 * Add test node herself in reserved nodes. by @AurelienFT in #2390 * Weekly `cargo update` by @github-actions in #2424 * Weekly `cargo update` by @github-actions in #2440 * Resolve some falky tests and improve CI times by @AurelienFT in #2401 * feat: handle `Unknown` transactions, blocks and consensus parameters by @hal3e in #2154 * fix(p2p): cache responses to serve without roundtrip to db by @rymnc in #2352 * Replace task `run()` return result with custom enum by @MitchTurner in #2429 * Fix codeowners by @AurelienFT in #2444 * fix(graphql_playground): use graphiql instead by @rymnc in #2446 * Weekly `cargo update` by @github-actions in #2453 * refactor: remove `Option<BlockHeight>` and use new enum where applicable by @matt-user in #2033 * Fixed the error during dry run by @xgreenx in #2365 * Add decompression traits and a test case by @Dentosal in #2295 * Versioned Storage for Modifications History by @acerone85 in #2233 * Allow DA recorded blocks to come out-of-order by @MitchTurner in #2415 * feat: Change `kv_store::Value` to be Arc<[u8]> instead of Arc<Vec<u8>> by @netrome in #2411 * Optimize balance-related queries with a cache by @rafal-ch in #2383 * fix: Add missing features to `fuel-core-tests` by @netrome in #2467 * Keep data in fails cases in sync service by @AurelienFT in #2361 * Weekly `cargo update` by @github-actions in #2470 * Revert balances amount to `U64` and introduce new `amountU128` getter by @rafal-ch in #2472 * Create uninitialized task for v1 gas price service by @MitchTurner in #2442 * Port the 0.40.2 fix of TAI on master by @AurelienFT in #2485 * Ignore RUSTSEC-2024-0421 by @AurelienFT in #2489 * Ignore receipts from failed transactions in `message_receipts_proof` by @AurelienFT in #2478 * Add unrecorded blocks abstraction to gas price algo by @MitchTurner in #2468 * Fix last iteration in sequential opcode by @AurelienFT in #2479 * fix(gas_price_service_v0): bring back removed fields, causing UB when trying to access by @rymnc in #2511 * Refactor fuel-core to use version of StorageRead::read with offset (Full update to 0.59.1) by @acerone85 in #2438 * Sync the version of the `fuel-core` with minor hot fixes by @xgreenx in #2516 * fix(docs): typo preventing ci checks from passing by @rymnc in #2525 * Integration test for balances and (non)retryable messages by @rafal-ch in #2505 * Add document for launching Ignition node from source and Local network from source by @AurelienFT in #2502 * Make the rocksdb cache optional in config and add policy for column opening by @AurelienFT in #2526 * Weekly `cargo update` by @github-actions in #2530 * chore(rocksdb): getter for inner database handle by @rymnc in #2532 * Use gas prices from actual blocks to calculate estimate gas prices by @MitchTurner in #2501 * chore(codeowners): gas price service codeowners by @rymnc in #2534 * Add zk opcodes by @AurelienFT in #2439 * Gas price simulation data retriever by @acerone85 in #2533 * Shared sequencer integration by @Dentosal in #1922 * Use expiration policy by @AurelienFT in #2447 * Fixed TPS benchmark to work with latest changes by @xgreenx in #2515 * Use indexation cache to satisfy "coins to spend" queries by @rafal-ch in #2463 * feat(txpool|p2p): use seqlock instead of small copy-able RwLocks by @rymnc in #2524 * Create new index for tracking Asset metadata by @maschad in #2445 * feat(rocksdb): remove getters for internal rocksdb handles, expose `backup` instead by @rymnc in #2535 * Integrate with V1 algo for tests by @MitchTurner in #2469 * Lock-free `latest_l2_height` in gas price service by @rafal-ch in #2546 * chore(gas_price_service_v1): strictly ensure last_recorded_height is set, to avoid initial poll of da source by @rymnc in #2556 * Replace old Graphql Gas Price adapter with new latest gas price struct by @MitchTurner in #2547 * Rename cost and rewards without 'excess' by @MitchTurner in #2558 * Add current pool gas to the node info endpoint by @AurelienFT in #2550 * Pagination queries for `balances` endpoint by @rafal-ch in #2490 * 2559 Increase timeout for test by @MitchTurner in #2560 * Add test expiration policy in executor by @AurelienFT in #2563 ## New Contributors * @yaziciahmet made their first contribution in #2378 **Full Changelog**: v0.40.0...v0.41.0

acerone85 added 2 commits October 15, 2024 21:05

Add a module for versioning the request response protocols in the p2p…

ab26a0b

…_service

Record the latest compatible version for the request response protoco…

c9ba074

…l in the PeerManager table

acerone85 requested review from xgreenx, Dentosal and MitchTurner as code owners October 15, 2024 20:34

acerone85 linked an issue Oct 15, 2024 that may be closed by this pull request

Return better error messages from P2P requests #1311

Closed

P2PService uses peer manager information to determine the version of …

b3d3219

…the request response protocol to use

acerone85 changed the base branch from release/v0.40.0 to master October 15, 2024 22:28

acerone85 added 6 commits October 16, 2024 07:29

Remove argument from Protocol::V1 and minor improvements to tests

8817d88

Merge branch 'master' into 1311-return-better-error-messages-from-p2p…

2ad29e4

…-requests

Merge branch 'master' into 1311-return-better-error-messages-from-p2p…

aedda11

…-requests

Add changelog entry

1855350

Fix compilation error

1fa98a4

Placate Clippy

6cebf0e

acerone85 changed the title ~~WIP - Add versioning to request response protocols~~ Add versioning to request response protocols Oct 16, 2024

acerone85 added 3 commits October 16, 2024 11:28

Version PostcardCodec

939939d

Revert "Revert changes"

e8c563c

This reverts commit f3b7db3b0f4aa92f2606c87ed6f65f4798734889.

P2P Service: Add /fuel/req_res/0.0.2 that sends error codes in respon…

f50a4d3

…ses to unsuccessful requests

acerone85 self-assigned this Oct 18, 2024

AurelienFT previously approved these changes Oct 20, 2024

View reviewed changes

acerone85 added 2 commits October 21, 2024 11:10

Prefer V2 over V1 when peers support both versions of the protocol

77841f0

Changelog

7160034

acerone85 dismissed AurelienFT’s stale review via 7160034 October 21, 2024 10:20

PostcardProtocol default implementation is now derived

b2b5b3c

acerone85 mentioned this pull request Oct 21, 2024

Integration test for nodes running on different versions of /fuel/req_res protocol #2374

Open

3 tasks

acerone85 added 2 commits October 21, 2024 19:25

Add tests to check serialization and deserialization of responses

93b4d06

Formatting

06e4177

MitchTurner reviewed Oct 22, 2024

View reviewed changes

acerone85 added 2 commits October 22, 2024 11:30

Merge branch 'master' into 1311-return-better-error-messages-from-p2p…

0729a87

…-requests

Remove TODO comment around ResponseMessageErrorCode definition

273bb34

acerone85 added 9 commits October 22, 2024 11:43

Rename LegacyResponseMessage into V1ResponseMessage

0865da6

Rename ResponseMessage into V2ResponseMessage

52290e7

Rename Request/Response protocol version constant variables

50f92ae

Refine TODO to remove NetworkCodec trait and reference github issue

b710d8d

Refine todos for error codes and add github issue when missing

8534f84

Add Given, When, Then sections to tests

cec899c

Remove trailing space in TODO comment

319ec8a

Use matches! macro in tests

4280781

Give more meaningful names to tests

6276af5

acerone85 requested a review from MitchTurner October 22, 2024 11:24

MitchTurner approved these changes Oct 22, 2024

View reviewed changes

AurelienFT approved these changes Oct 22, 2024

View reviewed changes

acerone85 merged commit bb6a42c into master Oct 22, 2024
39 checks passed

acerone85 deleted the 1311-return-better-error-messages-from-p2p-requests branch October 22, 2024 12:46

acerone85 mentioned this pull request Oct 22, 2024

Fuel/Request_Response v0.0.2: More meaningful error messages #2377

Merged

12 tasks

xgreenx reviewed Oct 22, 2024

View reviewed changes

xgreenx mentioned this pull request Jan 15, 2025

Release v0.41.0 #2565

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add versioning to request response protocols #2362

Add versioning to request response protocols #2362

acerone85 commented Oct 15, 2024 •

edited

Loading

AurelienFT Oct 20, 2024

MitchTurner left a comment

MitchTurner Oct 22, 2024

MitchTurner Oct 22, 2024

acerone85 Oct 22, 2024

MitchTurner Oct 22, 2024

acerone85 Oct 22, 2024

MitchTurner Oct 22, 2024

acerone85 Oct 22, 2024

MitchTurner Oct 22, 2024

acerone85 Oct 22, 2024

xgreenx Oct 22, 2024

acerone85 Oct 23, 2024

xgreenx Oct 22, 2024

xgreenx Oct 22, 2024

xgreenx Oct 22, 2024

acerone85 Oct 23, 2024

Add versioning to request response protocols #2362

Add versioning to request response protocols #2362

Conversation

acerone85 commented Oct 15, 2024 • edited Loading

Linked Issues/PRs

Context

Description

TODO (here or in different PRs)

Checklist

Before requesting review

After merging, notify other teams

Choose a reason for hiding this comment

MitchTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acerone85 commented Oct 15, 2024 •

edited

Loading