another day, another master merge

* master: (21 commits)
  Fix cycle dispute-coordinator <-> dispute-distribution  (#6489)
  Replace async-std with tokio in PVF subsystem (#6419)
  Guide changes 3 (#6520)
  Bump tokio from 1.22.0 to 1.24.1 (#6523)
  Fix flaky test in `dispute-coordinator` (#6524)
  Remove unused code (#6525)
  disputes pallet: Remove spam slots (#6345)
  Use `polkadot-node-metrics` where possible (#6484)
  Unlimited Proof size per block (#6085)
  [ci] Remove check-transaction-versions job (#6509)
  Update serde because substrate->trybuild needs a newer version (#6505)
  upgrade libp2p to 0.50.0 (#6500)
  av-store: write meta for unknown finalized blocks (#6452)
  Log PVF retries (#6504)
  Issue 6274: keeping all backing votes in provisioner vote set (#6494)
  Fix `polkadot-runtime-constants` std build (#6503)
  Add `try-runtime` to Rococo runtime (#6501)
  Co #13045: Selectable on-runtime-upgrade checks (#6498)
  Migrate Staking pallet to v13 (#6365)
  Deploy scheduler agenda cleanup migration (#6465)
  ...

Cargo.toml

@@ -27,7 +27,7 @@ tikv-jemallocator = "0.5.0"
 assert_cmd = "2.0.4"
 nix = "0.24.1"
 tempfile = "3.2.0"
-tokio = "1.22.0"
+tokio = "1.24.1"
 substrate-rpc-client = { git = "https://github.com/paritytech/substrate", branch = "master" }
 polkadot-core-primitives = { path = "core-primitives" }
 

node/client/src/lib.rs

@@ -29,7 +29,7 @@ use sp_api::{CallApiAt, Encode, NumberFor, ProvideRuntimeApi};
 use sp_blockchain::{HeaderBackend, HeaderMetadata};
 use sp_consensus::BlockStatus;
 use sp_runtime::{
-	generic::{BlockId, SignedBlock},
+	generic::SignedBlock,
 	traits::{BlakeTwo256, Block as BlockT},
 	Justifications,
 };
@@ -338,22 +338,25 @@ impl sc_client_api::BlockBackend<Block> for Client {
 		}
 	}
 
-	fn block(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<SignedBlock<Block>>> {
+	fn block(
+		&self,
+		hash: <Block as BlockT>::Hash,
+	) -> sp_blockchain::Result<Option<SignedBlock<Block>>> {
 		with_client! {
 			self,
 			client,
 			{
-				client.block(id)
+				client.block(hash)
 			}
 		}
 	}
 
-	fn block_status(&self, id: &BlockId<Block>) -> sp_blockchain::Result<BlockStatus> {
+	fn block_status(&self, hash: <Block as BlockT>::Hash) -> sp_blockchain::Result<BlockStatus> {
 		with_client! {
 			self,
 			client,
 			{
-				client.block_status(id)
+				client.block_status(hash)
 			}
 		}
 	}

node/core/av-store/src/lib.rs

@@ -570,6 +570,14 @@ async fn run_iteration<Context>(
 				FromOrchestra::Signal(OverseerSignal::BlockFinalized(hash, number)) => {
 					let _timer = subsystem.metrics.time_process_block_finalized();
 
+					if !subsystem.known_blocks.is_known(&hash) {
+						// If we haven't processed this block yet,
+						// make sure we write the metadata about the
+						// candidates backed in this finalized block.
+						// Otherwise, we won't be able to store our chunk
+						// for these candidates.
+						process_block_activated(ctx, subsystem, hash).await?;
+					}
 					subsystem.finalized_number = Some(number);
 					subsystem.known_blocks.prune_finalized(number);
 					process_block_finalized(

node/core/av-store/src/tests.rs

@@ -732,8 +732,49 @@ fn we_dont_miss_anything_if_import_notifications_are_missed() {
 	let test_state = TestState::default();
 
 	test_harness(test_state.clone(), store.clone(), |mut virtual_overseer| async move {
-		overseer_signal(&mut virtual_overseer, OverseerSignal::BlockFinalized(Hash::zero(), 1))
-			.await;
+		let block_hash = Hash::repeat_byte(1);
+		overseer_signal(&mut virtual_overseer, OverseerSignal::BlockFinalized(block_hash, 1)).await;
+
+		let header = Header {
+			parent_hash: Hash::repeat_byte(0),
+			number: 1,
+			state_root: Hash::zero(),
+			extrinsics_root: Hash::zero(),
+			digest: Default::default(),
+		};
+
+		assert_matches!(
+			overseer_recv(&mut virtual_overseer).await,
+			AllMessages::ChainApi(ChainApiMessage::BlockHeader(
+				relay_parent,
+				tx,
+			)) => {
+				assert_eq!(relay_parent, block_hash);
+				tx.send(Ok(Some(header))).unwrap();
+			}
+		);
+
+		assert_matches!(
+			overseer_recv(&mut virtual_overseer).await,
+			AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+				relay_parent,
+				RuntimeApiRequest::CandidateEvents(tx),
+			)) => {
+				assert_eq!(relay_parent, block_hash);
+				tx.send(Ok(Vec::new())).unwrap();
+			}
+		);
+
+		assert_matches!(
+			overseer_recv(&mut virtual_overseer).await,
+			AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+				relay_parent,
+				RuntimeApiRequest::Validators(tx),
+			)) => {
+				assert_eq!(relay_parent, Hash::zero());
+				tx.send(Ok(Vec::new())).unwrap();
+			}
+		);
 
 		let header = Header {
 			parent_hash: Hash::repeat_byte(3),

node/core/candidate-validation/Cargo.toml

@@ -16,8 +16,8 @@ parity-scale-codec = { version = "3.1.5", default-features = false, features = [
 polkadot-primitives = { path = "../../../primitives" }
 polkadot-parachain = { path = "../../../parachain" }
 polkadot-node-primitives = { path = "../../primitives" }
-polkadot-node-subsystem = {path = "../../subsystem" }
-polkadot-node-subsystem-util = { path = "../../subsystem-util" }
+polkadot-node-subsystem = { path = "../../subsystem" }
+polkadot-node-metrics = { path = "../../metrics" }
 
 [target.'cfg(not(any(target_os = "android", target_os = "unknown")))'.dependencies]
 polkadot-node-core-pvf = { path = "../pvf" }
@@ -27,5 +27,6 @@ sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master
 futures = { version = "0.3.21", features = ["thread-pool"] }
 assert_matches = "1.4.0"
 polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" }
+polkadot-node-subsystem-util = { path = "../../subsystem-util" }
 sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
 test-helpers = { package = "polkadot-primitives-test-helpers", path = "../../../primitives/test-helpers" }

node/core/candidate-validation/src/lib.rs

@@ -604,13 +604,16 @@ async fn validate_candidate_exhaustive(
 
 #[async_trait]
 trait ValidationBackend {
+	/// Tries executing a PVF a single time (no retries).
 	async fn validate_candidate(
 		&mut self,
 		pvf: Pvf,
 		timeout: Duration,
 		encoded_params: Vec<u8>,
 	) -> Result<WasmValidationResult, ValidationError>;
 
+	/// Tries executing a PVF. Will retry once if an error is encountered that may have been
+	/// transient.
 	async fn validate_candidate_with_retry(
 		&mut self,
 		raw_validation_code: Vec<u8>,
@@ -620,22 +623,30 @@ trait ValidationBackend {
 		// Construct the PVF a single time, since it is an expensive operation. Cloning it is cheap.
 		let pvf = Pvf::from_code(raw_validation_code);
 
-		let validation_result =
+		let mut validation_result =
 			self.validate_candidate(pvf.clone(), timeout, params.encode()).await;
 
 		// If we get an AmbiguousWorkerDeath error, retry once after a brief delay, on the
-		// assumption that the conditions that caused this error may have been transient.
+		// assumption that the conditions that caused this error may have been transient. Note that
+		// this error is only a result of execution itself and not of preparation.
 		if let Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::AmbiguousWorkerDeath)) =
 			validation_result
 		{
 			// Wait a brief delay before retrying.
 			futures_timer::Delay::new(PVF_EXECUTION_RETRY_DELAY).await;
+
+			gum::debug!(
+				target: LOG_TARGET,
+				?pvf,
+				"Re-trying failed candidate validation due to AmbiguousWorkerDeath."
+			);
+
 			// Encode the params again when re-trying. We expect the retry case to be relatively
 			// rare, and we want to avoid unconditionally cloning data.
-			self.validate_candidate(pvf, timeout, params.encode()).await
-		} else {
-			validation_result
+			validation_result = self.validate_candidate(pvf, timeout, params.encode()).await;
 		}
+
+		validation_result
 	}
 
 	async fn precheck_pvf(&mut self, pvf: Pvf) -> Result<Duration, PrepareError>;
@@ -666,12 +677,12 @@ impl ValidationBackend for ValidationHost {
 
 	async fn precheck_pvf(&mut self, pvf: Pvf) -> Result<Duration, PrepareError> {
 		let (tx, rx) = oneshot::channel();
-		if let Err(_) = self.precheck_pvf(pvf, tx).await {
+		if let Err(err) = self.precheck_pvf(pvf, tx).await {
 			// Return an IO error if there was an error communicating with the host.
-			return Err(PrepareError::IoErr)
+			return Err(PrepareError::IoErr(err))
 		}
 
-		let precheck_result = rx.await.or(Err(PrepareError::IoErr))?;
+		let precheck_result = rx.await.map_err(|err| PrepareError::IoErr(err.to_string()))?;
 
 		precheck_result
 	}

node/core/candidate-validation/src/metrics.rs

@@ -15,7 +15,7 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use super::{ValidationFailed, ValidationResult};
-use polkadot_node_subsystem_util::metrics::{self, prometheus};
+use polkadot_node_metrics::metrics::{self, prometheus};
 
 #[derive(Clone)]
 pub(crate) struct MetricsInner {

node/core/candidate-validation/src/tests.rs

@@ -1053,5 +1053,5 @@ fn precheck_properly_classifies_outcomes() {
 	inner(Err(PrepareError::Panic("baz".to_owned())), PreCheckOutcome::Invalid);
 
 	inner(Err(PrepareError::TimedOut), PreCheckOutcome::Failed);
-	inner(Err(PrepareError::IoErr), PreCheckOutcome::Failed);
+	inner(Err(PrepareError::IoErr("fizz".to_owned())), PreCheckOutcome::Failed);
 }

node/core/chain-api/Cargo.toml

@@ -9,8 +9,8 @@ futures = "0.3.21"
 gum = { package = "tracing-gum", path = "../../gum" }
 sp-blockchain = { git = "https://github.com/paritytech/substrate", branch = "master" }
 polkadot-primitives = { path = "../../../primitives" }
+polkadot-node-metrics = { path = "../../metrics" }
 polkadot-node-subsystem = {path = "../../subsystem" }
-polkadot-node-subsystem-util = { path = "../../subsystem-util" }
 sc-client-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
 sc-consensus-babe = { git = "https://github.com/paritytech/substrate", branch = "master" }
 

node/core/chain-api/src/metrics.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
-use polkadot_node_subsystem_util::metrics::{self, prometheus};
+use polkadot_node_metrics::metrics::{self, prometheus};
 
 #[derive(Clone)]
 pub(crate) struct MetricsInner {

node/core/dispute-coordinator/Cargo.toml

@@ -31,6 +31,7 @@ assert_matches = "1.4.0"
 test-helpers = { package = "polkadot-primitives-test-helpers", path = "../../../primitives/test-helpers" }
 futures-timer = "3.0.2"
 sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master" }
+sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }
 
 [features]
 # If not enabled, the dispute coordinator will do nothing.

node/core/dispute-coordinator/src/import.rs

@@ -87,60 +87,69 @@ impl<'a> CandidateEnvironment<'a> {
 
 /// Whether or not we already issued some statement about a candidate.
 pub enum OwnVoteState {
-	/// We already voted/issued a statement for the candidate.
-	Voted,
-	/// We already voted/issued a statement for the candidate and it was an approval vote.
+	/// Our votes, if any.
+	Voted(Vec<(ValidatorIndex, (DisputeStatement, ValidatorSignature))>),
+
+	/// We are not a parachain validator in the session.
 	///
-	/// Needs special treatment as we have to make sure to propagate it to peers, to guarantee the
-	/// dispute can conclude.
-	VotedApproval(Vec<(ValidatorIndex, ValidatorSignature)>),
-	/// We not yet voted for the dispute.
-	NoVote,
+	/// Hence we cannot vote.
+	CannotVote,
 }
 
 impl OwnVoteState {
 	fn new<'a>(votes: &CandidateVotes, env: &CandidateEnvironment<'a>) -> Self {
-		let mut our_valid_votes = env
-			.controlled_indices()
+		let controlled_indices = env.controlled_indices();
+		if controlled_indices.is_empty() {
+			return Self::CannotVote
+		}
+
+		let our_valid_votes = controlled_indices
 			.iter()
 			.filter_map(|i| votes.valid.raw().get_key_value(i))
-			.peekable();
-		let mut our_invalid_votes =
-			env.controlled_indices.iter().filter_map(|i| votes.invalid.get_key_value(i));
-		let has_valid_votes = our_valid_votes.peek().is_some();
-		let has_invalid_votes = our_invalid_votes.next().is_some();
-		let our_approval_votes: Vec<_> = our_valid_votes
-			.filter_map(|(index, (k, sig))| {
-				if let ValidDisputeStatementKind::ApprovalChecking = k {
-					Some((*index, sig.clone()))
-				} else {
-					None
-				}
-			})
-			.collect();
+			.map(|(index, (kind, sig))| (*index, (DisputeStatement::Valid(*kind), sig.clone())));
+		let our_invalid_votes = controlled_indices
+			.iter()
+			.filter_map(|i| votes.invalid.get_key_value(i))
+			.map(|(index, (kind, sig))| (*index, (DisputeStatement::Invalid(*kind), sig.clone())));
 
-		if !our_approval_votes.is_empty() {
-			return Self::VotedApproval(our_approval_votes)
-		}
-		if has_valid_votes || has_invalid_votes {
-			return Self::Voted
-		}
-		Self::NoVote
+		Self::Voted(our_valid_votes.chain(our_invalid_votes).collect())
 	}
 
-	/// Whether or not we issued a statement for the candidate already.
-	fn voted(&self) -> bool {
+	/// Is a vote from us missing but we are a validator able to vote?
+	fn vote_missing(&self) -> bool {
 		match self {
-			Self::Voted | Self::VotedApproval(_) => true,
-			Self::NoVote => false,
+			Self::Voted(votes) if votes.is_empty() => true,
+			Self::Voted(_) | Self::CannotVote => false,
 		}
 	}
 
 	/// Get own approval votes, if any.
-	fn approval_votes(&self) -> Option<&Vec<(ValidatorIndex, ValidatorSignature)>> {
+	///
+	/// Empty iterator means, no approval votes. `None` means, there will never be any (we cannot
+	/// vote).
+	fn approval_votes(
+		&self,
+	) -> Option<impl Iterator<Item = (ValidatorIndex, &ValidatorSignature)>> {
 		match self {
-			Self::VotedApproval(votes) => Some(&votes),
-			_ => None,
+			Self::Voted(votes) => Some(votes.iter().filter_map(|(index, (kind, sig))| {
+				if let DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking) = kind {
+					Some((*index, sig))
+				} else {
+					None
+				}
+			})),
+			Self::CannotVote => None,
+		}
+	}
+
+	/// Get our votes if there are any.
+	///
+	/// Empty iterator means, no votes. `None` means, there will never be any (we cannot
+	/// vote).
+	fn votes(&self) -> Option<&Vec<(ValidatorIndex, (DisputeStatement, ValidatorSignature))>> {
+		match self {
+			Self::Voted(votes) => Some(&votes),
+			Self::CannotVote => None,
 		}
 	}
 }
@@ -170,7 +179,7 @@ impl CandidateVoteState<CandidateVotes> {
 			valid: ValidCandidateVotes::new(),
 			invalid: BTreeMap::new(),
 		};
-		Self { votes, own_vote: OwnVoteState::NoVote, dispute_status: None }
+		Self { votes, own_vote: OwnVoteState::CannotVote, dispute_status: None }
 	}
 
 	/// Create a new `CandidateVoteState` from already existing votes.
@@ -327,16 +336,25 @@ impl<V> CandidateVoteState<V> {
 		self.dispute_status.map_or(false, |s| s.is_confirmed_concluded())
 	}
 
-	/// This machine already cast some vote in that dispute/for that candidate.
-	pub fn has_own_vote(&self) -> bool {
-		self.own_vote.voted()
+	/// Are we a validator in the session, but have not yet voted?
+	pub fn own_vote_missing(&self) -> bool {
+		self.own_vote.vote_missing()
 	}
 
 	/// Own approval votes if any:
-	pub fn own_approval_votes(&self) -> Option<&Vec<(ValidatorIndex, ValidatorSignature)>> {
+	pub fn own_approval_votes(
+		&self,
+	) -> Option<impl Iterator<Item = (ValidatorIndex, &ValidatorSignature)>> {
 		self.own_vote.approval_votes()
 	}
 
+	/// Get own votes if there are any.
+	pub fn own_votes(
+		&self,
+	) -> Option<&Vec<(ValidatorIndex, (DisputeStatement, ValidatorSignature))>> {
+		self.own_vote.votes()
+	}
+
 	/// Whether or not there is a dispute and it has already enough valid votes to conclude.
 	pub fn has_concluded_for(&self) -> bool {
 		self.dispute_status.map_or(false, |s| s.has_concluded_for())