Skip to content

Commit

Permalink
add fallback request for req-response protocols (#2771)
Browse files Browse the repository at this point in the history
Previously, it was only possible to retry the same request on a
different protocol name that had the exact same binary payloads.

Introduce a way of trying a different request on a different protocol if
the first one fails with Unsupported protocol.

This helps with adding new req-response versions in polkadot while
preserving compatibility with unupgraded nodes.

The way req-response protocols were bumped previously was that they were
bundled with some other notifications protocol upgrade, like for async
backing (but that is more complicated, especially if the feature does
not require any changes to a notifications protocol). Will be needed for
implementing polkadot-fellows/RFCs#47

TODO:
- [x]  add tests
- [x] add guidance docs in polkadot about req-response protocol
versioning
  • Loading branch information
alindima authored Jan 10, 2024
1 parent af2e30e commit f2a750e
Show file tree
Hide file tree
Showing 29 changed files with 802 additions and 304 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ mod tests {
use futures::{executor, future};

use parity_scale_codec::Encode;
use sc_network::ProtocolName;
use sp_core::testing::TaskExecutor;

use polkadot_node_primitives::BlockData;
Expand Down Expand Up @@ -231,7 +232,10 @@ mod tests {
Some(Requests::PoVFetchingV1(outgoing)) => {outgoing}
);
req.pending_response
.send(Ok(PoVFetchingResponse::PoV(pov.clone()).encode()))
.send(Ok((
PoVFetchingResponse::PoV(pov.clone()).encode(),
ProtocolName::from(""),
)))
.unwrap();
break
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use futures::{
Future, FutureExt, StreamExt,
};

use sc_network as network;
use sc_network::{self as network, ProtocolName};
use sp_keyring::Sr25519Keyring;

use polkadot_node_network_protocol::request_response::{v1, Recipient};
Expand Down Expand Up @@ -252,7 +252,7 @@ impl TestRun {
}
}
req.pending_response
.send(response.map(Encode::encode))
.send(response.map(|r| (r.encode(), ProtocolName::from(""))))
.expect("Sending response should succeed");
}
return (valid_responses == 0) && self.valid_chunks.is_empty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::{
time::Duration,
};

use network::ProtocolName;
use polkadot_node_subsystem_test_helpers::TestSubsystemContextHandle;
use polkadot_node_subsystem_util::TimeoutExt;

Expand Down Expand Up @@ -324,7 +325,11 @@ fn to_incoming_req(
let response = rx.await;
let payload = response.expect("Unexpected canceled request").result;
pending_response
.send(payload.map_err(|_| network::RequestFailure::Refused))
.send(
payload
.map_err(|_| network::RequestFailure::Refused)
.map(|r| (r, ProtocolName::from(""))),
)
.expect("Sending response is expected to work");
}
.boxed(),
Expand Down
Loading

0 comments on commit f2a750e

Please sign in to comment.