Refactor availability-recovery strategies (#1457)

Refactors availability-recovery strategies to allow for easily adding
new hotpaths and failover mechanisms.

The new interface allows for chaining multiple `RecoveryStrategy`-es
together, to cleanly express the relationship between them and share
state and code where neccessary/possible:

This was done in order to aid in implementing new hotpaths like
[systematic chunks
recovery](https://github.com/paritytech/polkadot-sdk/issues/598) and
[fetching from approval
checkers](https://github.com/paritytech/polkadot-sdk/issues/575).

Thanks to this design, intermediate state can be shared between the
strategies. For example, if the systematic chunks recovery retrieved
less than the needed amount of chunks, pass them over to the next
FetchChunks strategy, which will only need to recover the remaining
number of chunks.

Draft example of how a systematic chunk recovery strategy would look:
https://github.com/paritytech/polkadot-sdk/commit/667d870bdf1470525d66c13929d5eac7249dd995
(notice how easy it was to add and reuse code)

Note that this PR doesn't itself add any new strategy, it should fully
preserve backwards compatiblity in terms of functionality. Follow-up PRs
to add new strategies will come.
This commit is contained in:
Alin Dima
2023-09-20 15:56:43 +03:00
committed by GitHub
parent 771c3fbde7
commit 6f00edbc55
5 changed files with 953 additions and 754 deletions
@@ -21,15 +21,19 @@ use futures::{executor, future};
use futures_timer::Delay;
use parity_scale_codec::Encode;
use polkadot_node_network_protocol::request_response::{IncomingRequest, ReqProtocolNames};
use polkadot_node_network_protocol::request_response::{
self as req_res, IncomingRequest, Recipient, ReqProtocolNames, Requests,
};
use super::*;
use sc_network::config::RequestResponseConfig;
use sc_network::{config::RequestResponseConfig, IfDisconnected, OutboundFailure, RequestFailure};
use polkadot_erasure_coding::{branches, obtain_chunks_v1 as obtain_chunks};
use polkadot_node_primitives::{BlockData, PoV, Proof};
use polkadot_node_subsystem::messages::{AllMessages, RuntimeApiMessage, RuntimeApiRequest};
use polkadot_node_subsystem::messages::{
AllMessages, NetworkBridgeTxMessage, RuntimeApiMessage, RuntimeApiRequest,
};
use polkadot_node_subsystem_test_helpers::{
make_subsystem_context, mock::new_leaf, TestSubsystemContextHandle,
};
@@ -204,7 +208,7 @@ use sp_keyring::Sr25519Keyring;
enum Has {
No,
Yes,
NetworkError(sc_network::RequestFailure),
NetworkError(RequestFailure),
/// Make request not return at all, instead the sender is returned from the function.
///
/// Note, if you use `DoesNotReturn` you have to keep the returned senders alive, otherwise the
@@ -214,7 +218,7 @@ enum Has {
impl Has {
fn timeout() -> Self {
Has::NetworkError(sc_network::RequestFailure::Network(sc_network::OutboundFailure::Timeout))
Has::NetworkError(RequestFailure::Network(OutboundFailure::Timeout))
}
}
@@ -393,7 +397,7 @@ impl TestState {
candidate_hash: CandidateHash,
virtual_overseer: &mut VirtualOverseer,
who_has: impl Fn(usize) -> Has,
) -> Vec<oneshot::Sender<std::result::Result<Vec<u8>, sc_network::RequestFailure>>> {
) -> Vec<oneshot::Sender<std::result::Result<Vec<u8>, RequestFailure>>> {
let mut senders = Vec::new();
for _ in 0..self.validators.len() {
// Receive a request for a chunk.
@@ -1010,7 +1014,7 @@ fn recovers_from_only_chunks_if_pov_large() {
AvailabilityRecoveryMessage::RecoverAvailableData(
new_candidate.clone(),
test_state.session_index,
None,
Some(GroupIndex(0)),
tx,
),
)
@@ -1546,36 +1550,3 @@ fn invalid_local_chunk_is_ignored() {
(virtual_overseer, req_cfg)
});
}
#[test]
fn parallel_request_calculation_works_as_expected() {
let num_validators = 100;
let threshold = recovery_threshold(num_validators).unwrap();
let (erasure_task_tx, _erasure_task_rx) = futures::channel::mpsc::channel(16);
let mut phase = RequestChunksFromValidators::new(100, erasure_task_tx);
assert_eq!(phase.get_desired_request_count(threshold), threshold);
phase.error_count = 1;
phase.total_received_responses = 1;
// We saturate at threshold (34):
assert_eq!(phase.get_desired_request_count(threshold), threshold);
let dummy_chunk =
ErasureChunk { chunk: Vec::new(), index: ValidatorIndex(0), proof: Proof::dummy_proof() };
phase.insert_chunk(ValidatorIndex(0), dummy_chunk.clone());
phase.total_received_responses = 2;
// With given error rate - still saturating:
assert_eq!(phase.get_desired_request_count(threshold), threshold);
for i in 1..9 {
phase.insert_chunk(ValidatorIndex(i), dummy_chunk.clone());
}
phase.total_received_responses += 8;
// error rate: 1/10
// remaining chunks needed: threshold (34) - 9
// expected: 24 * (1+ 1/10) = (next greater integer) = 27
assert_eq!(phase.get_desired_request_count(threshold), 27);
phase.insert_chunk(ValidatorIndex(9), dummy_chunk.clone());
phase.error_count = 0;
// With error count zero - we should fetch exactly as needed:
assert_eq!(phase.get_desired_request_count(threshold), threshold - phase.chunk_count());
}