Cleaner GRANDPA RPC API for proving finality

client/finality-grandpa/rpc/src/finality.rs

@@ -26,13 +26,9 @@ pub struct EncodedFinalityProofs(pub sp_core::Bytes);
 
 /// Local trait mainly to allow mocking in tests.
 pub trait RpcFinalityProofProvider<Block: BlockT> {
-	/// Return finality proofs for the given authorities set id, if it is provided, otherwise the
-	/// current one will be used.
 	fn rpc_prove_finality(
 		&self,
-		begin: Block::Hash,
-		end: Block::Hash,
-		authorities_set_id: u64,
+		block: NumberFor<Block>,
 	) -> Result<Option<EncodedFinalityProofs>, sp_blockchain::Error>;
 }
 
@@ -44,11 +40,9 @@ where
 {
 	fn rpc_prove_finality(
 		&self,
-		begin: Block::Hash,
-		end: Block::Hash,
-		authorities_set_id: u64,
+		block: NumberFor<Block>,
 	) -> Result<Option<EncodedFinalityProofs>, sp_blockchain::Error> {
-		self.prove_finality(begin, end, authorities_set_id)
+		self.prove_finality(block)
 			.map(|x| x.map(|y| EncodedFinalityProofs(y.into())))
 	}
 }

client/finality-grandpa/rpc/src/lib.rs

@@ -37,7 +37,7 @@ mod notification;
 mod report;
 
 use sc_finality_grandpa::GrandpaJustificationStream;
-use sp_runtime::traits::Block as BlockT;
+use sp_runtime::traits::{Block as BlockT, NumberFor};
 
 use finality::{EncodedFinalityProofs, RpcFinalityProofProvider};
 use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates};
@@ -48,7 +48,7 @@ type FutureResult<T> =
 
 /// Provides RPC methods for interacting with GRANDPA.
 #[rpc]
-pub trait GrandpaApi<Notification, Hash> {
+pub trait GrandpaApi<Notification, Hash, N> {
 	/// RPC Metadata
 	type Metadata;
 
@@ -82,14 +82,12 @@ pub trait GrandpaApi<Notification, Hash> {
 		id: SubscriptionId
 	) -> jsonrpc_core::Result<bool>;
 
-	/// Prove finality for the range (begin; end] hash. Returns None if there are no finalized blocks
-	/// unknown in the range. If no authorities set is provided, the current one will be attempted.
+	/// Prove finality for the given block number.
+	/// WIP: expand this
 	#[rpc(name = "grandpa_proveFinality")]
 	fn prove_finality(
 		&self,
-		begin: Hash,
-		end: Hash,
-		authorities_set_id: Option<u64>,
+		block: N,
 	) -> FutureResult<Option<EncodedFinalityProofs>>;
 }
 
@@ -127,7 +125,8 @@ impl<AuthoritySet, VoterState, Block: BlockT, ProofProvider>
 	}
 }
 
-impl<AuthoritySet, VoterState, Block, ProofProvider> GrandpaApi<JustificationNotification, Block::Hash>
+impl<AuthoritySet, VoterState, Block, ProofProvider>
+	GrandpaApi<JustificationNotification, Block::Hash, NumberFor<Block>>
 	for GrandpaRpcHandler<AuthoritySet, VoterState, Block, ProofProvider>
 where
 	VoterState: ReportVoterState + Send + Sync + 'static,
@@ -171,16 +170,11 @@ where
 
 	fn prove_finality(
 		&self,
-		begin: Block::Hash,
-		end: Block::Hash,
-		authorities_set_id: Option<u64>,
+		block: NumberFor<Block>,
 	) -> FutureResult<Option<EncodedFinalityProofs>> {
-		// If we are not provided a set_id, try with the current one.
-		let authorities_set_id = authorities_set_id
-			.unwrap_or_else(|| self.authority_set.get().0);
 		let result = self
 			.finality_proof_provider
-			.rpc_prove_finality(begin, end, authorities_set_id);
+			.rpc_prove_finality(block);
 		let future = async move { result }.boxed();
 		Box::new(
 			future
@@ -262,9 +256,7 @@ mod tests {
 	impl<Block: BlockT> RpcFinalityProofProvider<Block> for TestFinalityProofProvider {
 		fn rpc_prove_finality(
 			&self,
-			_begin: Block::Hash,
-			_end: Block::Hash,
-			_authoritites_set_id: u64,
+			_block: NumberFor<Block>
 		) -> Result<Option<EncodedFinalityProofs>, sp_blockchain::Error> {
 			Ok(Some(EncodedFinalityProofs(self.finality_proofs.encode().into())))
 		}
@@ -531,11 +523,8 @@ mod tests {
 			finality_proofs.clone(),
 		);
 
-		let request = "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[\
-			\"0x0000000000000000000000000000000000000000000000000000000000000000\",\
-			\"0x0000000000000000000000000000000000000000000000000000000000000001\",\
-			42\
-		],\"id\":1}";
+		let request =
+			"{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[42],\"id\":1}";
 
 		let meta = sc_rpc::Metadata::default();
 		let resp = io.handle_request_sync(request, meta);

client/finality-grandpa/src/authority_set_changes.rs

@@ -0,0 +1,49 @@
+// Copyright 2020 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate 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.
+
+// Substrate 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 Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+use parity_scale_codec::{Decode, Encode};
+use std::{cmp::Ord, sync::Arc};
+
+// Tracks authority set changes. We store the block numbers for the last block of each authority
+// set.
+#[derive(Debug, Encode, Decode)]
+pub(crate) struct AuthoritySetChanges<N> {
+    authority_set_changes: Vec<N>,
+}
+
+impl<N: Ord + Copy> AuthoritySetChanges<N> {
+    pub(crate) fn empty() -> Self {
+        Self {
+            authority_set_changes: Vec::new(),
+        }
+    }
+
+    pub(crate) fn append(&mut self, number: N) {
+        self.authority_set_changes.push(number)
+    }
+
+    pub(crate) fn get_set_id(&self, number: N) -> (u64, N) {
+        let set_id = self
+            .authority_set_changes
+            .binary_search(&number)
+            .unwrap_or_else(|idx| idx);
+        let last_block_for_set_id = self.authority_set_changes[set_id];
+        // WIP: avoid cast?
+        (set_id as u64, last_block_for_set_id)
+    }
+}
+
+pub(crate) type SharedAuthoritySetChanges<N> = Arc<parking_lot::Mutex<AuthoritySetChanges<N>>>;

client/finality-grandpa/src/aux_schema.rs

@@ -28,6 +28,7 @@ use log::{info, warn};
 use sp_finality_grandpa::{AuthorityList, SetId, RoundNumber};
 
 use crate::authorities::{AuthoritySet, SharedAuthoritySet, PendingChange, DelayKind};
+use crate::authority_set_changes::{AuthoritySetChanges, SharedAuthoritySetChanges};
 use crate::consensus_changes::{SharedConsensusChanges, ConsensusChanges};
 use crate::environment::{
 	CompletedRound, CompletedRounds, CurrentRounds, HasVoted, SharedVoterSetState, VoterSetState,
@@ -39,6 +40,7 @@ const SET_STATE_KEY: &[u8] = b"grandpa_completed_round";
 const CONCLUDED_ROUNDS: &[u8] = b"grandpa_concluded_rounds";
 const AUTHORITY_SET_KEY: &[u8] = b"grandpa_voters";
 const CONSENSUS_CHANGES_KEY: &[u8] = b"grandpa_consensus_changes";
+const AUTHORITY_SET_CHANGES_KEY: &[u8] = b"grandpa_authority_set_changes";
 
 const CURRENT_VERSION: u32 = 2;
 
@@ -122,6 +124,7 @@ pub(crate) fn load_decode<B: AuxStore, T: Decode>(backend: &B, key: &[u8]) -> Cl
 /// Persistent data kept between runs.
 pub(crate) struct PersistentData<Block: BlockT> {
 	pub(crate) authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
+	pub(crate) authority_set_changes: SharedAuthoritySetChanges<NumberFor<Block>>,
 	pub(crate) consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
 	pub(crate) set_state: SharedVoterSetState<Block>,
 }
@@ -274,6 +277,8 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 	let version: Option<u32> = load_decode(backend, VERSION_KEY)?;
 	let consensus_changes = load_decode(backend, CONSENSUS_CHANGES_KEY)?
 		.unwrap_or_else(ConsensusChanges::<Block::Hash, NumberFor<Block>>::empty);
+	let authority_set_changes = load_decode(backend, AUTHORITY_SET_CHANGES_KEY)?
+		.unwrap_or_else(AuthoritySetChanges::<NumberFor<Block>>::empty);
 
 	let make_genesis_round = move || RoundState::genesis((genesis_hash, genesis_number));
 
@@ -282,6 +287,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 			if let Some((new_set, set_state)) = migrate_from_version0::<Block, _, _>(backend, &make_genesis_round)? {
 				return Ok(PersistentData {
 					authority_set: new_set.into(),
+					authority_set_changes: Arc::new(authority_set_changes.into()),
 					consensus_changes: Arc::new(consensus_changes.into()),
 					set_state: set_state.into(),
 				});
@@ -291,6 +297,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 			if let Some((new_set, set_state)) = migrate_from_version1::<Block, _, _>(backend, &make_genesis_round)? {
 				return Ok(PersistentData {
 					authority_set: new_set.into(),
+					authority_set_changes: Arc::new(authority_set_changes.into()),
 					consensus_changes: Arc::new(consensus_changes.into()),
 					set_state: set_state.into(),
 				});
@@ -321,6 +328,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 
 				return Ok(PersistentData {
 					authority_set: set.into(),
+					authority_set_changes: Arc::new(authority_set_changes.into()),
 					consensus_changes: Arc::new(consensus_changes.into()),
 					set_state: set_state.into(),
 				});
@@ -358,6 +366,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
 
 	Ok(PersistentData {
 		authority_set: genesis_set.into(),
+		authority_set_changes: Arc::new(authority_set_changes.into()),
 		set_state: genesis_state.into(),
 		consensus_changes: Arc::new(consensus_changes.into()),
 	})
@@ -421,6 +430,20 @@ pub(crate) fn write_concluded_round<Block: BlockT, B: AuxStore>(
 	backend.insert_aux(&[(&key[..], round_data.encode().as_slice())], &[])
 }
 
+pub(crate) fn update_authority_set_changes<N, F, R>(
+	authority_set_changes: &AuthoritySetChanges<N>,
+	write_aux: F,
+) -> R
+where
+	N: Encode,
+	F: FnOnce(&[(&'static [u8], &[u8])]) -> R,
+{
+	write_aux(&[(
+		AUTHORITY_SET_CHANGES_KEY,
+		authority_set_changes.encode().as_slice()
+	)])
+}
+
 /// Update the consensus changes.
 pub(crate) fn update_consensus_changes<H, N, F, R>(
 	set: &ConsensusChanges<H, N>,
@@ -433,6 +456,12 @@ pub(crate) fn update_consensus_changes<H, N, F, R>(
 	write_aux(&[(CONSENSUS_CHANGES_KEY, set.encode().as_slice())])
 }
 
+pub(crate) fn load_authority_set_changes<B: AuxStore, Block: BlockT>(backend: &B)
+	-> ClientResult<Option<AuthoritySetChanges<NumberFor<Block>>>>
+{
+	load_decode(backend, AUTHORITY_SET_CHANGES_KEY)
+}
+
 #[cfg(test)]
 pub(crate) fn load_authorities<B: AuxStore, H: Decode, N: Decode>(backend: &B)
 	-> Option<AuthoritySet<H, N>> {

client/finality-grandpa/src/environment.rs

@@ -1,5 +1,4 @@
 // This file is part of Substrate.
-
 // Copyright (C) 2018-2020 Parity Technologies (UK) Ltd.
 // SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
 
@@ -49,6 +48,7 @@ use crate::{
 use sp_consensus::SelectChain;
 
 use crate::authorities::{AuthoritySet, SharedAuthoritySet};
+use crate::authority_set_changes::SharedAuthoritySetChanges;
 use crate::communication::Network as NetworkT;
 use crate::consensus_changes::SharedConsensusChanges;
 use crate::notification::GrandpaJustificationSender;
@@ -417,6 +417,7 @@ pub(crate) struct Environment<Backend, Block: BlockT, C, N: NetworkT<Block>, SC,
 	pub(crate) config: Config,
 	pub(crate) authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
 	pub(crate) consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
+	pub(crate) authority_set_changes: SharedAuthoritySetChanges<NumberFor<Block>>,
 	pub(crate) network: crate::communication::NetworkBridge<Block, N>,
 	pub(crate) set_id: SetId,
 	pub(crate) voter_set_state: SharedVoterSetState<Block>,
@@ -1071,6 +1072,7 @@ where
 		finalize_block(
 			self.client.clone(),
 			&self.authority_set,
+			&self.authority_set_changes,
 			&self.consensus_changes,
 			Some(self.config.justification_period.into()),
 			hash,
@@ -1136,6 +1138,7 @@ impl<Block: BlockT> From<GrandpaJustification<Block>> for JustificationOrCommit<
 pub(crate) fn finalize_block<BE, Block, Client>(
 	client: Arc<Client>,
 	authority_set: &SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
+	authority_set_changes: &SharedAuthoritySetChanges<NumberFor<Block>>,
 	consensus_changes: &SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
 	justification_period: Option<NumberFor<Block>>,
 	hash: Block::Hash,
@@ -1175,6 +1178,8 @@ where
 	// reverting in case of failure
 	let mut old_consensus_changes = None;
 
+	let mut authority_set_changes = authority_set_changes.lock();
+
 	let mut consensus_changes = consensus_changes.lock();
 	let canon_at_height = |canon_number| {
 		// "true" because the block is finalized
@@ -1292,6 +1297,23 @@ where
 			// the authority set has changed.
 			let (new_id, set_ref) = authority_set.current();
 
+			// Persist the number of the last block of the session
+			if number > NumberFor::<Block>::zero() {
+				let parent_number = number - NumberFor::<Block>::one();
+				authority_set_changes.append(parent_number);
+				let write_result = crate::aux_schema::update_authority_set_changes(
+					&*authority_set_changes,
+					|insert| apply_aux(import_op, insert, &[]),
+				);
+				if let Err(e) = write_result {
+					warn!(
+						target: "afg",
+						"Failed to write updated authority set changes to disk: {}",
+						e
+					);
+				}
+			}
+
 			if set_ref.len() > 16 {
 				afg_log!(initial_sync,
 					"👴 Applying GRANDPA set change to new set with {} authorities",

client/finality-grandpa/src/finality_proof.rs

@@ -39,7 +39,9 @@
 use std::sync::Arc;
 use log::{trace, warn};
 
-use sp_blockchain::{Backend as BlockchainBackend, Error as ClientError, Result as ClientResult};
+use sp_blockchain::{
+	Backend as BlockchainBackend, Error as ClientError, HeaderBackend, Result as ClientResult,
+};
 use sc_client_api::{
 	backend::Backend, StorageProof,
 	light::{FetchChecker, RemoteReadRequest},
@@ -188,20 +190,29 @@ impl<B, Block> FinalityProofProvider<B, Block>
 		NumberFor<Block>: BlockNumberOps,
 		B: Backend<Block> + Send + Sync + 'static,
 {
-	/// Prove finality for the range (begin; end] hash. Returns None if there are no finalized blocks
-	/// unknown in the range.
+	/// Prove finality for the given block number.
+	/// WIP: expand this description
 	pub fn prove_finality(
 		&self,
-		begin: Block::Hash,
-		end: Block::Hash,
-		authorities_set_id: u64,
+		block: NumberFor<Block>,
 	) -> Result<Option<Vec<u8>>, ClientError> {
+		let blocks_where_set_changes =
+			match crate::aux_schema::load_authority_set_changes::<_, Block>(&*self.backend)? {
+				Some(blocks) => blocks,
+				None => return Ok(None),
+			};
+		let (set_id, last_block_for_set) = blocks_where_set_changes.get_set_id(block);
+
+		// Convert from block numbers to hashes
+		let last_block_for_set = self.backend.blockchain().hash(last_block_for_set).unwrap().unwrap();
+		let block = self.backend.blockchain().hash(block).unwrap().unwrap();
+
 		prove_finality::<_, _, GrandpaJustification<Block>>(
 			&*self.backend.blockchain(),
 			&*self.authority_provider,
-			authorities_set_id,
-			begin,
-			end,
+			set_id,
+			block,
+			last_block_for_set,
 		)
 	}
 }