Implement Substrate Pallet Runtime APIs (#389)

* Implement public helpers for querying header info

* Update `best_header` when importing headers

* Add BestHeader to GenesisConfig

* Define extra types for Millau primitives

* Start implementing runtime APIs in Millau runtime

* Add helper for getting headers which require a justification

* Add runtime API for getting headers requiring a justification

* Reword `expect()` proof for valid authority sets

* Fix typo

* Clean up Hasher comment

* Add the Call Dispatch Pallet back to the Millau runtime

* Use types from Rialto in bridge pallet config

* Use the Rialto runtime APIS in the Millau runtime

* Include Millau bridge instance in Rialto runtime

* Add missing doc comment

* Use one storage function for setting and clearing `RequiresJustification`

* Remove TODO comments

bridges/bin/millau/runtime/Cargo.toml

@@ -15,9 +15,11 @@ serde = { version = "1.0.115", optional = true, features = ["derive"] }
 
 bp-message-lane = { path = "../../../primitives/message-lane", default-features = false }
 bp-millau = { path = "../../../primitives/millau", default-features = false }
+bp-rialto = { path = "../../../primitives/rialto", default-features = false }
 pallet-bridge-call-dispatch = { path = "../../../modules/call-dispatch", default-features = false }
 pallet-message-lane = { path = "../../../modules/message-lane", default-features = false }
 pallet-shift-session-manager = { path = "../../../modules/shift-session-manager", default-features = false }
+pallet-substrate-bridge = { path = "../../../modules/substrate", default-features = false }
 
 # Substrate Dependencies
 
@@ -53,6 +55,7 @@ default = ["std"]
 std = [
 	"bp-message-lane/std",
 	"bp-millau/std",
+	"bp-rialto/std",
 	"codec/std",
 	"frame-executive/std",
 	"frame-support/std",
@@ -66,6 +69,7 @@ std = [
 	"pallet-randomness-collective-flip/std",
 	"pallet-shift-session-manager/std",
 	"pallet-session/std",
+	"pallet-substrate-bridge/std",
 	"pallet-sudo/std",
 	"pallet-timestamp/std",
 	"pallet-transaction-payment/std",

bridges/bin/millau/runtime/src/lib.rs

@@ -54,6 +54,7 @@ pub use frame_support::{
 };
 
 pub use pallet_balances::Call as BalancesCall;
+pub use pallet_substrate_bridge::Call as BridgeSubstrateCall;
 pub use pallet_timestamp::Call as TimestampCall;
 
 #[cfg(any(feature = "std", test))]
@@ -216,7 +217,6 @@ impl frame_system::Trait for Runtime {
 impl pallet_aura::Trait for Runtime {
 	type AuthorityId = AuraId;
 }
-
 impl pallet_bridge_call_dispatch::Trait for Runtime {
 	type Event = Event;
 	type MessageId = (bp_message_lane::LaneId, bp_message_lane::MessageNonce);
@@ -308,6 +308,13 @@ impl pallet_session::Trait for Runtime {
 	type WeightInfo = ();
 }
 
+impl pallet_substrate_bridge::Trait for Runtime {
+	type BridgedHeader = bp_rialto::Header;
+	type BridgedBlockNumber = bp_rialto::BlockNumber;
+	type BridgedBlockHash = bp_rialto::Hash;
+	type BridgedBlockHasher = bp_rialto::Hasher;
+}
+
 impl pallet_shift_session_manager::Trait for Runtime {}
 
 construct_runtime!(
@@ -316,6 +323,7 @@ construct_runtime!(
 		NodeBlock = opaque::Block,
 		UncheckedExtrinsic = UncheckedExtrinsic
 	{
+		BridgeRialto: pallet_substrate_bridge::{Module, Call, Storage},
 		BridgeCallDispatch: pallet_bridge_call_dispatch::{Module, Event<T>},
 		System: frame_system::{Module, Call, Config, Storage, Event<T>},
 		RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Module, Call, Storage},
@@ -479,4 +487,34 @@ impl_runtime_apis! {
 			None
 		}
 	}
+
+	impl bp_rialto::RialtoHeaderApi<Block> for Runtime {
+		fn best_block() -> (bp_rialto::BlockNumber, bp_rialto::Hash) {
+			let header = BridgeRialto::best_header();
+			(header.number, header.hash())
+		}
+
+		fn finalized_block() -> (bp_rialto::BlockNumber, bp_rialto::Hash) {
+			let header = BridgeRialto::best_finalized();
+			(header.number, header.hash())
+		}
+
+		fn incomplete_headers() -> Vec<(bp_rialto::BlockNumber, bp_rialto::Hash)> {
+			// Since the pallet doesn't accept multiple scheduled changes right now
+			// we can only have one header requiring a justification at any time.
+			if let Some(header) = BridgeRialto::requires_justification() {
+				vec![(header.number, header.hash())]
+			} else {
+				vec![]
+			}
+		}
+
+		fn is_known_block(hash: bp_rialto::Hash) -> bool {
+			BridgeRialto::is_known_header(hash)
+		}
+
+		fn is_finalized_block(hash: bp_rialto::Hash) -> bool {
+			BridgeRialto::is_finalized_header(hash)
+		}
+	}
 }

bridges/bin/rialto/runtime/Cargo.toml

@@ -24,6 +24,7 @@ bp-rialto = { path = "../../../primitives/rialto", default-features = false }
 pallet-bridge-eth-poa = { path = "../../../modules/ethereum", default-features = false }
 pallet-bridge-call-dispatch = { path = "../../../modules/call-dispatch", default-features = false }
 pallet-bridge-currency-exchange = { path = "../../../modules/currency-exchange", default-features = false }
+pallet-substrate-bridge = { path = "../../../modules/substrate", default-features = false }
 pallet-message-lane = { path = "../../../modules/message-lane", default-features = false }
 pallet-shift-session-manager = { path = "../../../modules/shift-session-manager", default-features = false }
 
@@ -68,8 +69,8 @@ std = [
 	"bp-eth-poa/std",
 	"bp-header-chain/std",
 	"bp-message-lane/std",
-	"bp-millau/std",
 	"bp-rialto/std",
+	"bp-millau/std",
 	"codec/std",
 	"frame-benchmarking/std",
 	"frame-executive/std",
@@ -85,6 +86,7 @@ std = [
 	"pallet-message-lane/std",
 	"pallet-randomness-collective-flip/std",
 	"pallet-shift-session-manager/std",
+	"pallet-substrate-bridge/std",
 	"pallet-sudo/std",
 	"pallet-timestamp/std",
 	"pallet-transaction-payment/std",

bridges/bin/rialto/runtime/src/lib.rs

@@ -414,6 +414,13 @@ impl pallet_session::Trait for Runtime {
 	type WeightInfo = ();
 }
 
+impl pallet_substrate_bridge::Trait for Runtime {
+	type BridgedHeader = bp_millau::Header;
+	type BridgedBlockNumber = bp_millau::BlockNumber;
+	type BridgedBlockHash = bp_millau::Hash;
+	type BridgedBlockHasher = bp_millau::Hasher;
+}
+
 impl pallet_shift_session_manager::Trait for Runtime {}
 
 construct_runtime!(
@@ -426,6 +433,7 @@ construct_runtime!(
 		BridgeKovan: pallet_bridge_eth_poa::<Instance2>::{Module, Call, Config, Storage, ValidateUnsigned},
 		BridgeRialtoCurrencyExchange: pallet_bridge_currency_exchange::<Instance1>::{Module, Call},
 		BridgeKovanCurrencyExchange: pallet_bridge_currency_exchange::<Instance2>::{Module, Call},
+		BridgeMillau: pallet_substrate_bridge::{Module, Call, Storage},
 		BridgeCallDispatch: pallet_bridge_call_dispatch::{Module, Event<T>},
 		System: frame_system::{Module, Call, Config, Storage, Event<T>},
 		RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Module, Call, Storage},
@@ -562,6 +570,36 @@ impl_runtime_apis! {
 		}
 	}
 
+	impl bp_millau::MillauHeaderApi<Block> for Runtime {
+		fn best_block() -> (bp_millau::BlockNumber, bp_millau::Hash) {
+			let header = BridgeMillau::best_header();
+			(header.number, header.hash())
+		}
+
+		fn finalized_block() -> (bp_millau::BlockNumber, bp_millau::Hash) {
+			let header = BridgeMillau::best_finalized();
+			(header.number, header.hash())
+		}
+
+		fn incomplete_headers() -> Vec<(bp_millau::BlockNumber, bp_millau::Hash)> {
+			// Since the pallet doesn't accept multiple scheduled changes right now
+			// we can only have one header requiring a justification at any time.
+			if let Some(header) = BridgeMillau::requires_justification() {
+				vec![(header.number, header.hash())]
+			} else {
+				vec![]
+			}
+		}
+
+		fn is_known_block(hash: bp_millau::Hash) -> bool {
+			BridgeMillau::is_known_header(hash)
+		}
+
+		fn is_finalized_block(hash: bp_millau::Hash) -> bool {
+			BridgeMillau::is_finalized_header(hash)
+		}
+	}
+
 	impl bp_currency_exchange::RialtoCurrencyExchangeApi<Block, exchange::EthereumTransactionInclusionProof> for Runtime {
 		fn filter_transaction_proof(proof: exchange::EthereumTransactionInclusionProof) -> bool {
 			BridgeRialtoCurrencyExchange::filter_transaction_proof(&proof)
@@ -574,24 +612,6 @@ impl_runtime_apis! {
 		}
 	}
 
-	impl bp_millau::MillauHeaderApi<Block> for Runtime {
-		fn best_block() -> (bp_millau::BlockNumber, bp_millau::Hash) {
-			unimplemented!("https://github.com/paritytech/parity-bridges-common/issues/368")
-		}
-
-		fn finalized_block() -> (bp_millau::BlockNumber, bp_millau::Hash) {
-			unimplemented!("https://github.com/paritytech/parity-bridges-common/issues/368")
-		}
-
-		fn incomplete_headers() -> Vec<(bp_millau::BlockNumber, bp_millau::Hash)> {
-			unimplemented!("https://github.com/paritytech/parity-bridges-common/issues/368")
-		}
-
-		fn is_known_block(_hash: bp_millau::Hash) -> bool {
-			unimplemented!("https://github.com/paritytech/parity-bridges-common/issues/368")
-		}
-	}
-
 	impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime {
 		fn validate_transaction(
 			source: TransactionSource,

bridges/modules/substrate/src/lib.rs

@@ -109,8 +109,15 @@ pub trait Trait: frame_system::Trait {
 
 decl_storage! {
 	trait Store for Module<T: Trait> as SubstrateBridge {
+		/// Hash of the header at the highest known height.
+		BestHeader: T::BridgedBlockHash;
 		/// Hash of the best finalized header.
 		BestFinalized: T::BridgedBlockHash;
+		/// A header which enacts an authority set change and therefore
+		/// requires a Grandpa justification.
+		// Since we won't always have an authority set change scheduled we
+		// won't always have a header which needs a justification.
+		RequiresJustification: Option<T::BridgedBlockHash>;
 		/// Headers which have been imported into the pallet.
 		ImportedHeaders: map hasher(identity) T::BridgedBlockHash => Option<ImportedHeader<T::BridgedHeader>>;
 		/// The current Grandpa Authority set.
@@ -136,6 +143,7 @@ decl_storage! {
 				.clone()
 				.expect("An initial header is needed");
 
+			<BestHeader<T>>::put(initial_header.hash());
 			<BestFinalized<T>>::put(initial_header.hash());
 			<ImportedHeaders<T>>::insert(
 				initial_header.hash(),
@@ -223,6 +231,58 @@ decl_module! {
 	}
 }
 
+impl<T: Trait> Module<T> {
+	/// Get the highest header that the pallet knows of.
+	// In a future where we support forks this could be a Vec of headers
+	// since we may have multiple headers at the same height.
+	pub fn best_header() -> T::BridgedHeader {
+		PalletStorage::<T>::new().best_header().header
+	}
+
+	/// Get the best finalized header the pallet knows of.
+	///
+	/// Since this has been finalized correctly a user of the bridge
+	/// pallet should be confident that any transactions that were
+	/// included in this or any previous header will not be reverted.
+	pub fn best_finalized() -> T::BridgedHeader {
+		PalletStorage::<T>::new().best_finalized_header().header
+	}
+
+	/// Check if a particular header is known to the bridge pallet.
+	pub fn is_known_header(hash: T::BridgedBlockHash) -> bool {
+		PalletStorage::<T>::new().header_exists(hash)
+	}
+
+	/// Check if a particular header is finalized.
+	///
+	/// Will return false if the header is not known to the pallet.
+	// One thing worth noting here is that this approach won't work well
+	// once we track forks since there could be an older header on a
+	// different fork which isn't an ancestor of our best finalized header.
+	pub fn is_finalized_header(hash: T::BridgedBlockHash) -> bool {
+		let storage = PalletStorage::<T>::new();
+		if let Some(header) = storage.header_by_hash(hash) {
+			header.number() <= storage.best_finalized_header().number()
+		} else {
+			false
+		}
+	}
+
+	/// Return the latest header which enacts an authority set change
+	/// and still needs a finality proof.
+	///
+	/// Will return None if there are no headers which are missing finality proofs.
+	pub fn requires_justification() -> Option<T::BridgedHeader> {
+		let storage = PalletStorage::<T>::new();
+		let hash = storage.unfinalized_header()?;
+		let imported_header = storage.header_by_hash(hash).expect(
+			"We write a header to storage before marking it as unfinalized, therefore
+			this must always exist if we got an unfinalized header hash.",
+		);
+		Some(imported_header.header)
+	}
+}
+
 /// Expected interface for interacting with bridge pallet storage.
 // TODO: This should be split into its own less-Substrate-dependent crate
 pub trait BridgeStorage {
@@ -232,6 +292,12 @@ pub trait BridgeStorage {
 	/// Write a header to storage.
 	fn write_header(&mut self, header: &ImportedHeader<Self::Header>);
 
+	/// Get the header at the highest known height.
+	fn best_header(&self) -> ImportedHeader<Self::Header>;
+
+	/// Update the header at the highest height.
+	fn update_best_header(&mut self, hash: <Self::Header as HeaderT>::Hash);
+
 	/// Get the best finalized header the pallet knows of.
 	fn best_finalized_header(&self) -> ImportedHeader<Self::Header>;
 
@@ -241,6 +307,15 @@ pub trait BridgeStorage {
 	/// Check if a particular header is known to the pallet.
 	fn header_exists(&self, hash: <Self::Header as HeaderT>::Hash) -> bool;
 
+	/// Return a header which requires a justification. A header will require
+	/// a justification when it enacts an new authority set.
+	fn unfinalized_header(&self) -> Option<<Self::Header as HeaderT>::Hash>;
+
+	/// Mark a header as eventually requiring a justification.
+	///
+	/// If None is passed the storage item is cleared.
+	fn update_unfinalized_header(&mut self, hash: Option<<Self::Header as HeaderT>::Hash>);
+
 	/// Get a specific header by its hash.
 	///
 	/// Returns None if it is not known to the pallet.
@@ -284,6 +359,16 @@ impl<T: Trait> BridgeStorage for PalletStorage<T> {
 		<ImportedHeaders<T>>::insert(hash, header);
 	}
 
+	fn best_header(&self) -> ImportedHeader<Self::Header> {
+		let hash = <BestHeader<T>>::get();
+		self.header_by_hash(hash)
+			.expect("A header must have been written at genesis, therefore this must always exist")
+	}
+
+	fn update_best_header(&mut self, hash: T::BridgedBlockHash) {
+		<BestHeader<T>>::put(hash)
+	}
+
 	fn best_finalized_header(&self) -> ImportedHeader<T::BridgedHeader> {
 		let hash = <BestFinalized<T>>::get();
 		self.header_by_hash(hash)
@@ -302,6 +387,18 @@ impl<T: Trait> BridgeStorage for PalletStorage<T> {
 		<ImportedHeaders<T>>::get(hash)
 	}
 
+	fn unfinalized_header(&self) -> Option<T::BridgedBlockHash> {
+		<RequiresJustification<T>>::get()
+	}
+
+	fn update_unfinalized_header(&mut self, hash: Option<<Self::Header as HeaderT>::Hash>) {
+		if let Some(hash) = hash {
+			<RequiresJustification<T>>::put(hash);
+		} else {
+			<RequiresJustification<T>>::kill();
+		}
+	}
+
 	fn current_authority_set(&self) -> AuthoritySet {
 		CurrentAuthoritySet::get()
 	}

bridges/modules/substrate/src/verifier.rs

@@ -106,13 +106,14 @@ where
 	/// Will perform some basic checks to make sure that this header doesn't break any assumptions
 	/// such as being on a different finalized fork.
 	pub fn import_header(&mut self, header: H) -> Result<(), ImportError> {
+		let hash = header.hash();
 		let best_finalized = self.storage.best_finalized_header();
 
 		if header.number() <= best_finalized.number() {
 			return Err(ImportError::OldHeader);
 		}
 
-		if self.storage.header_exists(header.hash()) {
+		if self.storage.header_exists(hash) {
 			return Err(ImportError::HeaderAlreadyExists);
 		}
 
@@ -176,13 +177,20 @@ where
 			}
 		};
 
-		let is_finalized = false;
 		self.storage.write_header(&ImportedHeader {
 			header,
 			requires_justification,
-			is_finalized,
+			is_finalized: false,
 		});
 
+		if requires_justification {
+			self.storage.update_unfinalized_header(Some(hash));
+		}
+
+		// Since we're not dealing with forks at the moment we know that
+		// the header we just got will be the one at the best height
+		self.storage.update_best_header(hash);
+
 		Ok(())
 	}
 
@@ -205,8 +213,8 @@ where
 
 		let current_authority_set = self.storage.current_authority_set();
 		let voter_set = VoterSet::new(current_authority_set.authorities).expect(
-			"This only fails if we have an invalid list of authorities. Since we
-			got this from storage it should always be valid, otherwise we have a bug.",
+			"We verified the correctness of the authority list during header import,
+			before writing them to storage. This must always be valid.",
 		);
 		verify_justification::<H>(
 			(hash, *header.number()),
@@ -255,6 +263,9 @@ where
 			let _ = self.storage.enact_authority_set().expect(
 				"Headers must only be marked as `requires_justification` if there's a scheduled change in storage.",
 			);
+
+			// Clear the storage entry since we got a justification
+			self.storage.update_unfinalized_header(None);
 		}
 
 		for header in finalized_headers.iter_mut() {
@@ -442,9 +453,11 @@ mod tests {
 			};
 			assert_ok!(verifier.import_header(header.clone()));
 
-			let stored_header = storage.header_by_hash(header.hash());
-			assert!(stored_header.is_some());
-			assert_eq!(stored_header.unwrap().is_finalized, false);
+			let stored_header = storage
+				.header_by_hash(header.hash())
+				.expect("Should have been imported successfully");
+			assert_eq!(stored_header.is_finalized, false);
+			assert_eq!(stored_header, storage.best_header());
 		})
 	}
 
@@ -648,11 +661,14 @@ mod tests {
 			};
 
 			assert_ok!(verifier.import_header(header.clone()));
+			assert_eq!(storage.unfinalized_header(), Some(header.hash()));
+
 			assert_ok!(verifier.import_finality_proof(header.hash(), justification.into()));
 			assert_eq!(storage.best_finalized_header().header, header);
 
 			// Make sure that we have updated the set now that we've finalized our header
 			assert_eq!(storage.current_authority_set(), change.authority_set);
+			assert_eq!(storage.unfinalized_header(), None);
 		})
 	}
 

bridges/primitives/millau/Cargo.toml

@@ -11,6 +11,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 # Substrate Based Dependencies
 sp-api = { version = "2.0", default-features = false }
 sp-core = { version = "2.0", default-features = false }
+sp-runtime = { version = "2.0", default-features = false }
 sp-std = { version = "2.0", default-features = false }
 
 [features]
@@ -18,5 +19,6 @@ default = ["std"]
 std = [
 	"sp-api/std",
 	"sp-core/std",
+	"sp-runtime/std",
 	"sp-std/std",
 ]

bridges/primitives/millau/src/lib.rs

@@ -20,13 +20,21 @@
 // Runtime-generated DecodeLimit::decode_all_With_depth_limit
 #![allow(clippy::unnecessary_mut_passed)]
 
+use sp_core::Hasher as HasherT;
+use sp_runtime::traits::BlakeTwo256;
 use sp_std::prelude::*;
 
 /// Block number type used in Millau.
 pub type BlockNumber = u32;
 
 /// Hash type used in Millau.
-pub type Hash = sp_core::H256;
+pub type Hash = <BlakeTwo256 as HasherT>::Out;
+
+/// The type of an object that can produce hashes on Millau.
+pub type Hasher = BlakeTwo256;
+
+/// The header type used by Millau.
+pub type Header = sp_runtime::generic::Header<BlockNumber, Hasher>;
 
 sp_api::decl_runtime_apis! {
 	/// API for querying information about Millau headers from the Bridge Pallet instance.
@@ -42,8 +50,13 @@ sp_api::decl_runtime_apis! {
 		/// Returns number and hash of the best finalized block known to the bridge module.
 		fn finalized_block() -> (BlockNumber, Hash);
 		/// Returns numbers and hashes of headers that require finality proofs.
+		///
+		/// An empty response means that there are no headers which currently require a
+		/// finality proof.
 		fn incomplete_headers() -> Vec<(BlockNumber, Hash)>;
-		/// Returns true if header is known to the runtime.
+		/// Returns true if the header is known to the runtime.
 		fn is_known_block(hash: Hash) -> bool;
+		/// Returns true if the header is considered finalized by the runtime.
+		fn is_finalized_block(hash: Hash) -> bool;
 	}
 }

bridges/primitives/rialto/Cargo.toml

@@ -11,6 +11,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 # Substrate Based Dependencies
 sp-api = { version = "2.0.0", default-features = false }
 sp-core = { version = "2.0.0", default-features = false }
+sp-runtime = { version = "2.0.0", default-features = false }
 sp-std = { version = "2.0.0", default-features = false }
 
 [features]
@@ -18,5 +19,6 @@ default = ["std"]
 std = [
 	"sp-api/std",
 	"sp-core/std",
+	"sp-runtime/std",
 	"sp-std/std",
 ]

bridges/primitives/rialto/src/lib.rs

@@ -20,13 +20,21 @@
 // Runtime-generated DecodeLimit::decode_all_With_depth_limit
 #![allow(clippy::unnecessary_mut_passed)]
 
+use sp_core::Hasher as HasherT;
+use sp_runtime::traits::BlakeTwo256;
 use sp_std::prelude::*;
 
 /// Block number type used in Rialto.
 pub type BlockNumber = u32;
 
 /// Hash type used in Rialto.
-pub type Hash = sp_core::H256;
+pub type Hash = <BlakeTwo256 as HasherT>::Out;
+
+/// The type of an object that can produce hashes on Rialto.
+pub type Hasher = BlakeTwo256;
+
+/// The header type used by Rialto.
+pub type Header = sp_runtime::generic::Header<BlockNumber, Hasher>;
 
 sp_api::decl_runtime_apis! {
 	/// API for querying information about Rialto headers from the Bridge Pallet instance.
@@ -42,8 +50,13 @@ sp_api::decl_runtime_apis! {
 		/// Returns number and hash of the best finalized block known to the bridge module.
 		fn finalized_block() -> (BlockNumber, Hash);
 		/// Returns numbers and hashes of headers that require finality proofs.
+		///
+		/// An empty response means that there are no headers which currently require a
+		/// finality proof.
 		fn incomplete_headers() -> Vec<(BlockNumber, Hash)>;
-		/// Returns true if header is known to the runtime.
+		/// Returns true if the header is known to the runtime.
 		fn is_known_block(hash: Hash) -> bool;
+		/// Returns true if the header is considered finalized by the runtime.
+		fn is_finalized_block(hash: Hash) -> bool;
 	}
 }