Chain Selection: Follow-ups

node/core/approval-voting/src/approval_db/v1/mod.rs

@@ -423,17 +423,21 @@ pub(crate) fn add_block_entry(
 
 /// Forcibly approve all candidates included at up to the given relay-chain height in the indicated
 /// chain.
+///
+/// Returns a list of block hashes that were not approved and are now.
 pub fn force_approve(
 	store: &dyn KeyValueDB,
 	db_config: Config,
 	chain_head: Hash,
 	up_to: BlockNumber,
-) -> Result<()> {
+) -> Result<Vec<Hash>> {
 	enum State {
 		WalkTo,
 		Approving,
 	}
 
+	let mut approved_hashes = Vec::new();
+
 	let mut cur_hash = chain_head;
 	let mut state = State::WalkTo;
 
@@ -452,13 +456,20 @@ pub fn force_approve(
 		match state {
 			State::WalkTo => {},
 			State::Approving => {
-				entry.approved_bitfield.iter_mut().for_each(|mut b| *b = true);
-				tx.put_block_entry(entry);
+				let is_approved = entry.approved_bitfield.count_ones()
+					== entry.approved_bitfield.len();
+
+				if !is_approved {
+					entry.approved_bitfield.iter_mut().for_each(|mut b| *b = true);
+					approved_hashes.push(entry.block_hash);
+					tx.put_block_entry(entry);
+				}
 			}
 		}
 	}
 
-	tx.write(store)
+	tx.write(store)?;
+	Ok(approved_hashes)
 }
 
 /// Return all blocks which have entries in the DB, ascending, by height.

node/core/approval-voting/src/approval_db/v1/tests.rs

@@ -534,7 +534,7 @@ fn force_approve_works() {
 		).unwrap();
 	}
 
-	force_approve(&store, TEST_CONFIG,  block_hash_d, 2).unwrap();
+	let approved_hashes = force_approve(&store, TEST_CONFIG,  block_hash_d, 2).unwrap();
 
 	assert!(load_block_entry(
 		&store,
@@ -556,6 +556,10 @@ fn force_approve_works() {
 		&TEST_CONFIG,
 		&block_hash_d,
 	).unwrap().unwrap().approved_bitfield.not_any());
+	assert_eq!(
+		approved_hashes,
+		vec![block_hash_b, block_hash_a],
+	);
 }
 
 #[test]

node/core/approval-voting/src/import.rs

@@ -31,6 +31,7 @@
 use polkadot_node_subsystem::{
 	messages::{
 		RuntimeApiMessage, RuntimeApiRequest, ChainApiMessage, ApprovalDistributionMessage,
+		ChainSelectionMessage,
 	},
 	SubsystemContext, SubsystemError, SubsystemResult,
 };
@@ -462,10 +463,16 @@ pub(crate) async fn handle_new_head(
 						result.len(),
 					);
 				}
+
 				result
 			}
 		};
 
+		// If all bits are already set, then send an approve message.
+		if approved_bitfield.count_ones() == approved_bitfield.len() {
+			ctx.send_message(ChainSelectionMessage::Approved(block_hash).into()).await;
+		}
+
 		let block_entry = approval_db::v1::BlockEntry {
 			block_hash,
 			parent_hash: block_header.parent_hash,
@@ -487,8 +494,18 @@ pub(crate) async fn handle_new_head(
 				"Enacting force-approve",
 			);
 
-			approval_db::v1::force_approve(db_writer, db_config, block_hash, up_to)
+			let approved_hashes = approval_db::v1::force_approve(
+				db_writer,
+				db_config,
+				block_hash,
+				up_to,
+			)
 				.map_err(|e| SubsystemError::with_origin("approval-voting", e))?;
+
+			// Notify chain-selection of all approved hashes.
+			for hash in approved_hashes {
+				ctx.send_message(ChainSelectionMessage::Approved(hash).into()).await;
+			}
 		}
 
 		tracing::trace!(

node/core/approval-voting/src/lib.rs

@@ -26,7 +26,7 @@ use polkadot_node_subsystem::{
 		AssignmentCheckError, AssignmentCheckResult, ApprovalCheckError, ApprovalCheckResult,
 		ApprovalVotingMessage, RuntimeApiMessage, RuntimeApiRequest, ChainApiMessage,
 		ApprovalDistributionMessage, CandidateValidationMessage,
-		AvailabilityRecoveryMessage,
+		AvailabilityRecoveryMessage, ChainSelectionMessage,
 	},
 	errors::RecoveryError,
 	Subsystem, SubsystemContext, SubsystemError, SubsystemResult, SpawnedSubsystem,
@@ -717,6 +717,7 @@ enum Action {
 		candidate: CandidateReceipt,
 		backing_group: GroupIndex,
 	},
+	NoteApprovedInChainSelection(Hash),
 	IssueApproval(CandidateHash, ApprovalVoteRequest),
 	BecomeActive,
 	Conclude,
@@ -962,6 +963,9 @@ async fn handle_actions(
 					Some(_) => {},
 				}
 			}
+			Action::NoteApprovedInChainSelection(block_hash) => {
+				ctx.send_message(ChainSelectionMessage::Approved(block_hash).into()).await;
+			}
 			Action::BecomeActive => {
 				*mode = Mode::Active;
 
@@ -1805,6 +1809,7 @@ fn import_checked_approval(
 
 			if is_block_approved && !was_block_approved {
 				metrics.on_block_approved(status.tranche_now as _);
+				actions.push(Action::NoteApprovedInChainSelection(block_hash));
 			}
 
 			actions.push(Action::WriteBlockEntry(block_entry));

node/core/approval-voting/src/tests.rs

@@ -850,14 +850,22 @@ fn import_checked_approval_updates_entries_and_schedules() {
 
 		assert_matches!(
 			actions.get(0).unwrap(),
+			Action::NoteApprovedInChainSelection(h) => {
+				assert_eq!(h, &block_hash);
+			}
+		);
+
+
+		assert_matches!(
+			actions.get(1).unwrap(),
 			Action::WriteBlockEntry(b_entry) => {
 				assert_eq!(b_entry.block_hash(), block_hash);
 				assert!(b_entry.is_fully_approved());
 				assert!(b_entry.is_candidate_approved(&candidate_hash));
 			}
 		);
 		assert_matches!(
-			actions.get_mut(1).unwrap(),
+			actions.get_mut(2).unwrap(),
 			Action::WriteCandidateEntry(c_hash, ref mut c_entry) => {
 				assert_eq!(c_hash, &candidate_hash);
 				assert!(c_entry.approval_entry(&block_hash).unwrap().is_approved());
@@ -1391,9 +1399,16 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() {
 		ApprovalSource::Remote(validator_index_b),
 	);
 
-	assert_eq!(actions.len(), 2);
+	assert_eq!(actions.len(), 3);
 	assert_matches!(
 		actions.get(0).unwrap(),
+		Action::NoteApprovedInChainSelection(h) => {
+			assert_eq!(h, &block_hash);
+		}
+	);
+
+	assert_matches!(
+		actions.get(1).unwrap(),
 		Action::WriteBlockEntry(b_entry) => {
 			assert_eq!(b_entry.block_hash(), block_hash);
 			assert!(b_entry.is_fully_approved());
@@ -1403,7 +1418,7 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() {
 	);
 
 	assert_matches!(
-		actions.get(1).unwrap(),
+		actions.get(2).unwrap(),
 		Action::WriteCandidateEntry(c_h, c_entry) => {
 			assert_eq!(c_h, &candidate_hash_2);
 			assert!(c_entry.approval_entry(&block_hash).unwrap().is_approved());

node/core/chain-selection/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2018"
 
 [dependencies]
 futures = "0.3.15"
+futures-timer = "3"
 tracing = "0.1.26"
 polkadot-primitives = { path = "../../../primitives" }
 polkadot-node-primitives = { path = "../../primitives" }
@@ -21,3 +22,4 @@ polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" }
 sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
 parking_lot = "0.11"
 assert_matches = "1"
+kvdb-memorydb = "0.10.0"

node/core/chain-selection/src/db_backend/mod.rs

@@ -0,0 +1,19 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+//! A database [`Backend`][crate::backend::Backend] for the chain selection subsystem.
+
+pub(super) mod v1;