tests compile after changes

substrate/Cargo.lock

@@ -581,7 +581,6 @@ dependencies = [
 [[package]]
 name = "finality-grandpa"
 version = "0.3.0"
-source = "git+https://github.com/paritytech/finality-grandpa#f1ad8d7ca020e5db0e51e32cd62e8cd3c578d121"
 dependencies = [
  "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3034,7 +3033,7 @@ dependencies = [
 name = "substrate-finality-grandpa"
 version = "0.1.0"
 dependencies = [
- "finality-grandpa 0.3.0 (git+https://github.com/paritytech/finality-grandpa)",
+ "finality-grandpa 0.3.0",
  "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-codec 2.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4202,7 +4201,6 @@ dependencies = [
 "checksum failure_derive 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "946d0e98a50d9831f5d589038d2ca7f8f455b1c21028c0db0e84116a12696426"
 "checksum fake-simd 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed"
 "checksum fdlimit 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b1ee15a7050e5580b3712877157068ea713b245b080ff302ae2ca973cfcd9baa"
-"checksum finality-grandpa 0.3.0 (git+https://github.com/paritytech/finality-grandpa)" = "<none>"
 "checksum fixed-hash 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0d5ec8112f00ea8a483e04748a85522184418fd1cf02890b626d8fc28683f7de"
 "checksum fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2fad85553e09a6f881f739c29f0b00b0f01357c743266d478b68951ce23285f3"
 "checksum foreign-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1"

substrate/core/finality-grandpa/Cargo.toml

@@ -18,7 +18,7 @@ substrate-fg-primitives = { path = "primitives" }
 
 [dependencies.finality-grandpa]
 #version = "0.3.0"
-git = "https://github.com/paritytech/finality-grandpa"
+path = "../../../finality-afg"
 features = ["derive-codec"]
 
 [dev-dependencies]

substrate/core/finality-grandpa/primitives/src/lib.rs

@@ -33,7 +33,7 @@ use sr_primitives::traits::{Block as BlockT, DigestFor, NumberFor};
 
 /// A scheduled change of authority set.
 #[cfg_attr(feature = "std", derive(Debug, PartialEq))]
-#[derive(Encode, Decode)]
+#[derive(Clone, Encode, Decode)]
 pub struct ScheduledChange<N> {
 	/// The new authorities after the change, along with their respective weights.
 	pub next_authorities: Vec<(AuthorityId, u64)>,

substrate/core/finality-grandpa/src/lib.rs

@@ -699,20 +699,58 @@ impl<B, E, Block: BlockT, N> voter::Environment<Block::Hash, NumberFor<Block>> f
 	}
 }
 
+/// Client side of the GRANDPA APIs declared in fg-primitives.
+pub trait ApiClient<Block: BlockT> {
+	/// Get the genesis authorities for GRANDPA.
+	fn genesis_authorities(&self) -> Result<Vec<(AuthorityId, u64)>, ClientError>;
+
+	/// Check a header's digest for a scheduled change.
+	fn scheduled_change(&self, header: &Block::Header)
+		-> Result<Option<ScheduledChange<NumberFor<Block>>>, ClientError>;
+}
+
+impl<B, E, Block: BlockT> ApiClient<Block> for Arc<Client<B, E, Block>> where
+	B: Backend<Block, Blake2Hasher> + 'static,
+	E: CallExecutor<Block, Blake2Hasher> + 'static + Clone,
+	DigestFor<Block>: Encode,
+{
+	fn genesis_authorities(&self) -> Result<Vec<(AuthorityId, u64)>, ClientError> {
+		use runtime_primitives::traits::Zero;
+
+		self.call_api_at(
+			&BlockId::Number(NumberFor::<Block>::zero()),
+			fg_primitives::AUTHORITIES_CALL,
+			&()
+		)
+	}
+
+	fn scheduled_change(&self, header: &Block::Header)
+		-> Result<Option<ScheduledChange<NumberFor<Block>>>, ClientError>
+	{
+		self.call_api_at(
+			&BlockId::hash(header.parent_hash().clone()),
+			::fg_primitives::PENDING_CHANGE_CALL,
+			header.digest(),
+		)
+	}
+}
+
 /// A block-import handler for GRANDPA.
 ///
 /// This scans each imported block for signals of changing authority set.
 /// When using GRANDPA, the block import worker should be using this block import
 /// object.
-pub struct GrandpaBlockImport<B, E, Block: BlockT> {
+pub struct GrandpaBlockImport<B, E, Block: BlockT, Api> {
 	inner: Arc<Client<B, E, Block>>,
 	authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
+	api_client: Api,
 }
 
-impl<B, E, Block: BlockT> BlockImport<Block> for GrandpaBlockImport<B, E, Block> where
+impl<B, E, Block: BlockT, Api> BlockImport<Block> for GrandpaBlockImport<B, E, Block, Api> where
 	B: Backend<Block, Blake2Hasher> + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + 'static + Clone,
 	DigestFor<Block>: Encode,
+	Api: ApiClient<Block>,
 {
 	type Error = ClientError;
 
@@ -721,11 +759,7 @@ impl<B, E, Block: BlockT> BlockImport<Block> for GrandpaBlockImport<B, E, Block>
 	{
 		use authorities::PendingChange;
 
-		let maybe_change: Option<ScheduledChange<NumberFor<Block>>> = self.inner.call_api_at(
+		let maybe_change = self.api_client.scheduled_change(&block.header)?;
-			&BlockId::hash(block.header.parent_hash().clone()),
-			::fg_primitives::PENDING_CHANGE_CALL,
-			block.header.digest()
-		)?;
 
 		// when we update the authorities, we need to hold the lock
 		// until the block is written to prevent a race if we need to restore
@@ -768,14 +802,13 @@ pub struct LinkHalf<B, E, Block: BlockT> {
 
 /// Make block importer and link half necessary to tie the background voter
 /// to it.
-pub fn block_import<B, E, Block: BlockT>(client: Arc<Client<B, E, Block>>)
+pub fn block_import<B, E, Block: BlockT, Api>(client: Arc<Client<B, E, Block>>, api_client: Api)
-	-> Result<(GrandpaBlockImport<B, E, Block>, LinkHalf<B, E, Block>), ClientError>
+	-> Result<(GrandpaBlockImport<B, E, Block, Api>, LinkHalf<B, E, Block>), ClientError>
 	where
 	B: Backend<Block, Blake2Hasher> + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + 'static,
+	Api: ApiClient<Block>,
 {
-	use runtime_primitives::traits::Zero;
-
 	let authority_set = match client.backend().get_aux(AUTHORITY_SET_KEY)? {
 		None => {
 			info!(target: "afg", "Loading GRANDPA authorities \
@@ -784,8 +817,7 @@ pub fn block_import<B, E, Block: BlockT>(client: Arc<Client<B, E, Block>>)
 			// no authority set on disk: fetch authorities from genesis state.
 			// if genesis state is not available, we may be a light client, but these
 			// are unsupported for following GRANDPA directly.
-			let genesis_authorities: Vec<(AuthorityId, u64)> = client
+			let genesis_authorities: Vec<(AuthorityId, u64)> = api_client.genesis_authorities()?;
-				.call_api_at(&BlockId::Number(NumberFor::<Block>::zero()), fg_primitives::AUTHORITIES_CALL, &())?;
 
 			let authority_set = SharedAuthoritySet::genesis(genesis_authorities);
 			let encoded = authority_set.inner().read().encode();
@@ -801,7 +833,7 @@ pub fn block_import<B, E, Block: BlockT>(client: Arc<Client<B, E, Block>>)
 	};
 
 	Ok((
-		GrandpaBlockImport { inner: client.clone(), authority_set: authority_set.clone() },
+		GrandpaBlockImport { inner: client.clone(), authority_set: authority_set.clone(), api_client },
 		LinkHalf { client, authority_set },
 	))
 }

substrate/core/finality-grandpa/src/tests.rs

@@ -6,16 +6,34 @@ use parking_lot::Mutex;
 use tokio::runtime::current_thread;
 use keyring::Keyring;
 use client::BlockchainEvents;
-use test_client;
+use test_client::{self, runtime::BlockNumber};
 
 type PeerData = Mutex<Option<LinkHalf<test_client::Backend, test_client::Executor, Block>>>;
 type GrandpaPeer = Peer<PassThroughVerifier, PeerData>;
 
 struct GrandpaTestNet {
 	peers: Vec<Arc<GrandpaPeer>>,
+	test_config: TestApi,
 	started: bool
 }
 
+impl GrandpaTestNet {
+	fn new(test_config: TestApi, n_peers: usize) -> Self {
+		let mut net = GrandpaTestNet {
+			peers: Vec::with_capacity(n_peers),
+			started: false,
+			test_config,
+		};
+		let config = Self::default_config();
+
+		for _ in 0..n_peers {
+			net.add_peer(&config);
+		}
+
+		net
+	}
+}
+
 impl TestNetFactory for GrandpaTestNet {
 	type Verifier = PassThroughVerifier;
 	type PeerData = PeerData;
@@ -24,6 +42,7 @@ impl TestNetFactory for GrandpaTestNet {
 	fn from_config(_config: &ProtocolConfig) -> Self {
 		GrandpaTestNet {
 			peers: Vec::new(),
+			test_config: Default::default(),
 			started: false
 		}
 	}
@@ -37,7 +56,7 @@ impl TestNetFactory for GrandpaTestNet {
 	fn make_block_import(&self, client: Arc<PeersClient>)
 		-> (Arc<BlockImport<Block,Error=ClientError> + Send + Sync>, PeerData)
 	{
-		let (import, link) = block_import(client).expect("Could not create block import for fresh peer.");
+		let (import, link) = block_import(client, self.test_config.clone()).expect("Could not create block import for fresh peer.");
 		(Arc::new(import), Mutex::new(Some(link)))
 	}
 
@@ -113,43 +132,74 @@ impl Network for MessageRouting {
 	}
 }
 
+#[derive(Default, Clone)]
+struct TestApi {
+	genesis_authorities: Vec<(AuthorityId, u64)>,
+	scheduled_changes: HashMap<BlockNumber, ScheduledChange<BlockNumber>>,
+}
+
+impl TestApi {
+	fn new(genesis_authorities: Vec<(AuthorityId, u64)>) -> Self {
+		TestApi {
+			genesis_authorities,
+			scheduled_changes: HashMap::new(),
+		}
+	}
+}
+
+impl ApiClient<Block> for TestApi {
+	fn genesis_authorities(&self) -> Result<Vec<(AuthorityId, u64)>, ClientError> {
+		Ok(self.genesis_authorities.clone())
+	}
+
+	fn scheduled_change(&self, header: &<Block as BlockT>::Header)
+		-> Result<Option<ScheduledChange<NumberFor<Block>>>, ClientError>
+	{
+		// we take only scheduled changes at given block number where there are no
+		// extrinsics.
+		Ok(self.scheduled_changes.get(header.number()).map(|c| c.clone()))
+	}
+}
+
 const TEST_GOSSIP_DURATION: Duration = Duration::from_millis(500);
 const TEST_ROUTING_INTERVAL: Duration = Duration::from_millis(50);
 
 #[test]
-fn finalize_20_unanimous_3_peers() {
+fn finalize_3_voters_no_observers() {
-	let mut net = GrandpaTestNet::new(3);
+	let peers = &[Keyring::Alice, Keyring::Bob, Keyring::Charlie];
+	let voters: Vec<_> = peers.iter()
+		.map(|key| AuthorityId(key.to_raw_public()))
+		.map(|id| (id, 1))
+		.collect();
+
+	let mut net = GrandpaTestNet::new(TestApi::new(voters), 3);
 	net.peer(0).push_blocks(20, false);
 	net.sync();
 
-	let net = Arc::new(Mutex::new(net));
+	for i in 0..3 {
-	let peers = &[
+		assert_eq!(net.peer(i).client().info().unwrap().chain.best_number, 20,
-		(0, Keyring::Alice),
+			"Peer #{} failed to sync", i);
-		(1, Keyring::Bob),
+	}
-		(2, Keyring::Charlie),
-	];
 
-	let voters: Vec<_> = peers.iter()
+	let net = Arc::new(Mutex::new(net));
-		.map(|&(_, ref key)| AuthorityId(key.to_raw_public()))
-		.collect();
 
 	let mut finality_notifications = Vec::new();
-
 	let mut runtime = current_thread::Runtime::new().unwrap();
-	for (peer_id, key) in peers {
+
+	for (peer_id, key) in peers.iter().enumerate() {
 		let (client, link) = {
 			let mut net = net.lock();
 			// temporary needed for some reason
-			let link = net.peers[*peer_id].data.lock().take().expect("link initialized at startup; qed");
+			let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed");
 			(
-				net.peers[*peer_id].client().clone(),
+				net.peers[peer_id].client().clone(),
 				link,
 			)
 		};
 		finality_notifications.push(
 			client.finality_notification_stream()
 				.take_while(|n| Ok(n.header.number() < &20))
-				.for_each(move |_| Ok(()))
+				.for_each(|_| Ok(()))
 		);
 		let voter = run_grandpa(
 			Config {
@@ -157,7 +207,7 @@ fn finalize_20_unanimous_3_peers() {
 				local_key: Some(Arc::new(key.clone().into())),
 			},
 			link,
-			MessageRouting::new(net.clone(), *peer_id),
+			MessageRouting::new(net.clone(), peer_id),
 		).expect("all in order with client and network");
 
 		runtime.spawn(voter);
@@ -177,31 +227,27 @@ fn finalize_20_unanimous_3_peers() {
 }
 
 #[test]
-fn observer_can_finalize() {
+fn finalize_3_voters_1_observer() {
-	let mut net = GrandpaTestNet::new(4);
+	let peers = &[Keyring::Alice, Keyring::Bob, Keyring::Charlie];
+	let voters: Vec<_> = peers.iter()
+		.map(|key| AuthorityId(key.to_raw_public()))
+		.map(|id| (id, 1))
+		.collect();
+
+	let mut net = GrandpaTestNet::new(TestApi::new(voters), 4);
 	net.peer(0).push_blocks(20, false);
 	net.sync();
 
 	let net = Arc::new(Mutex::new(net));
-	let peers = &[
-		(0, Keyring::Alice),
-		(1, Keyring::Bob),
-		(2, Keyring::Charlie),
-	];
-
-	let voters: HashMap<_, _> = peers.iter()
-		.map(|&(_, ref key)| (AuthorityId(key.to_raw_public()), 1))
-		.collect();
-
 	let mut finality_notifications = Vec::new();
 
 	let mut runtime = current_thread::Runtime::new().unwrap();
 	let all_peers = peers.iter()
 		.cloned()
-		.map(|(id, key)| (id, Some(Arc::new(key.into()))))
+		.map(|key| Some(Arc::new(key.into())))
-		.chain(::std::iter::once((3, None)));
+		.chain(::std::iter::once(None));
 
-	for (peer_id, local_key) in all_peers {
+	for (peer_id, local_key) in all_peers.enumerate() {
 		let (client, link) = {
 			let mut net = net.lock();
 			let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed");

substrate/core/network/src/import_queue.rs

@@ -268,6 +268,11 @@ pub trait Link<B: BlockT>: Send {
 	fn restart(&self) { }
 }
 
+/// A link implementation that does nothing.
+pub struct NoopLink;
+
+impl<B: BlockT> Link<B> for NoopLink { }
+
 /// A link implementation that connects to the network.
 pub struct NetworkLink<B: BlockT, E: ExecuteInContext<B>> {
 	/// The chain-sync handle
@@ -340,9 +345,9 @@ enum BlockImportError {
 }
 
 /// Import a bunch of blocks.
-fn import_many_blocks<'a, B: BlockT, L: Link<B>, V: Verifier<B>>(
+fn import_many_blocks<'a, B: BlockT, V: Verifier<B>>(
 	import_handle: &BlockImport<B, Error=ClientError>,
-	link: &L,
+	link: &Link<B>,
 	qdata: Option<&AsyncImportQueueData<B>>,
 	blocks: (BlockOrigin, Vec<BlockData<B>>),
 	verifier: Arc<V>
@@ -460,7 +465,7 @@ fn import_single_block<B: BlockT, V: Verifier<B>>(
 }
 
 /// Process single block import result.
-fn process_import_result<'a, B: BlockT>(
+fn process_import_result<B: BlockT>(
 	link: &Link<B>,
 	result: Result<BlockImportResult<B::Hash, <<B as BlockT>::Header as HeaderT>::Number>, BlockImportError>
 ) -> usize
@@ -568,15 +573,30 @@ pub struct SyncImportQueue<B: BlockT, V: Verifier<B>> {
 }
 
 #[cfg(any(test, feature = "test-helpers"))]
-impl<B: BlockT, V: Verifier<B>> SyncImportQueue<B, V> {
+impl<B: 'static + BlockT, V: 'static + Verifier<B>> SyncImportQueue<B, V> {
 	/// Create a new SyncImportQueue wrapping the given Verifier and block import
 	/// handle.
 	pub fn new(verifier: Arc<V>, block_import: SharedBlockImport<B>) -> Self {
-		SyncImportQueue {
+		let queue = SyncImportQueue {
 			verifier,
 			link: ImportCB::new(),
 			block_import,
-		}
+		};
+
+		let v = queue.verifier.clone();
+		let import_handle = queue.block_import.clone();
+		queue.link.set(Box::new(move |origin, new_blocks| {
+			let verifier = v.clone();
+			import_many_blocks(
+				&*import_handle,
+				&NoopLink,
+				None,
+				(origin, new_blocks),
+				verifier,
+			)
+		}));
+
+		queue
 	}
 }
 

substrate/core/network/src/test/mod.rs

@@ -258,7 +258,7 @@ impl<V: 'static + Verifier<Block>, D> Peer<V, D> {
 					body: Some(block.extrinsics),
 					receipt: None,
 					message_queue: None,
-					justification: None,
+					justification: Some(Vec::new()),
 				},
 			}]);
 		}