diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 3bf9309dfbeda8b94dc8f687baf9ae8cfc1efd69..65cd08970f3cd627c3b085b062a842413e34f35f 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -300,17 +300,11 @@ impl pallet_substrate_bridge::Config for Runtime { type BridgedChain = bp_rialto::Rialto; } -parameter_types! { - // We'll use the length of a session on the bridged chain as our bound since GRANDPA is - // guaranteed to produce a justification every session. - pub const MaxHeadersInSingleProof: bp_rialto::BlockNumber = bp_rialto::SESSION_LENGTH; -} - impl pallet_finality_verifier::Config for Runtime { type BridgedChain = bp_rialto::Rialto; type HeaderChain = pallet_substrate_bridge::Module<Runtime>; + type AncestryProof = Vec<bp_rialto::Header>; type AncestryChecker = bp_header_chain::LinearAncestryChecker; - type MaxHeadersInSingleProof = MaxHeadersInSingleProof; } impl pallet_shift_session_manager::Config for Runtime {} diff --git a/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index f7717b1c769ec5858dc04b9a12bcd88f0d58a711..3ebb487fe99bc27baf92a3451b410c0b3e8df1a0 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -407,17 +407,11 @@ impl pallet_substrate_bridge::Config for Runtime { type BridgedChain = bp_millau::Millau; } -parameter_types! { - // We'll use the length of a session on the bridged chain as our bound since GRANDPA is - // guaranteed to produce a justification every session. - pub const MaxHeadersInSingleProof: bp_millau::BlockNumber = bp_millau::SESSION_LENGTH; -} - impl pallet_finality_verifier::Config for Runtime { type BridgedChain = bp_millau::Millau; type HeaderChain = pallet_substrate_bridge::Module<Runtime>; + type AncestryProof = Vec<bp_millau::Header>; type AncestryChecker = bp_header_chain::LinearAncestryChecker; - type MaxHeadersInSingleProof = MaxHeadersInSingleProof; } impl pallet_shift_session_manager::Config for Runtime {} diff --git a/bridges/modules/finality-verifier/Cargo.toml b/bridges/modules/finality-verifier/Cargo.toml index c1e09c7cf9798822d43335ed086b9cd912ab0203..b98f995b6169385a40e34ce668512892db227a4d 100644 --- a/bridges/modules/finality-verifier/Cargo.toml +++ b/bridges/modules/finality-verifier/Cargo.toml @@ -11,7 +11,6 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" codec = { package = "parity-scale-codec", version = "1.3.1", default-features = false } finality-grandpa = { version = "0.12.3", default-features = false } serde = { version = "1.0", optional = true } -num-traits = { version = "0.2", default-features = false } # Bridge Dependencies @@ -40,7 +39,6 @@ std = [ "finality-grandpa/std", "frame-support/std", "frame-system/std", - "num-traits/std", "serde", "sp-runtime/std", "sp-std/std", diff --git a/bridges/modules/finality-verifier/src/lib.rs b/bridges/modules/finality-verifier/src/lib.rs index 4e0a3583a2494a5c767f2e2a8808f7257ea5087f..cfcb610e7f5f850bddd62438dd737b573404c5c1 100644 --- a/bridges/modules/finality-verifier/src/lib.rs +++ b/bridges/modules/finality-verifier/src/lib.rs @@ -35,9 +35,8 @@ use bp_header_chain::{justification::verify_justification, AncestryChecker, HeaderChain}; use bp_runtime::{Chain, HeaderOf}; use finality_grandpa::voter_set::VoterSet; -use frame_support::{dispatch::DispatchError, ensure, traits::Get}; +use frame_support::{dispatch::DispatchError, ensure}; use frame_system::ensure_signed; -use num_traits::AsPrimitive; use sp_runtime::traits::Header as HeaderT; use sp_std::vec::Vec; @@ -64,17 +63,15 @@ pub mod pallet { /// The pallet which we will use as our underlying storage mechanism. type HeaderChain: HeaderChain<<Self::BridgedChain as Chain>::Header, DispatchError>; + /// The type of ancestry proof used by the pallet. + /// + /// Will be used by the ancestry checker to verify that the header being finalized is + /// related to the best finalized header in storage. + type AncestryProof: Parameter; + /// The type through which we will verify that a given header is related to the last /// finalized header in our storage pallet. - type AncestryChecker: AncestryChecker< - <Self::BridgedChain as Chain>::Header, - Vec<<Self::BridgedChain as Chain>::Header>, - >; - - /// The maximum length of headers we can have in a single ancestry proof. This prevents - /// unbounded iteration when verifying proofs. - #[pallet::constant] - type MaxHeadersInSingleProof: Get<<Self::BridgedChain as Chain>::BlockNumber>; + type AncestryChecker: AncestryChecker<<Self::BridgedChain as Chain>::Header, Self::AncestryProof>; } #[pallet::pallet] @@ -87,45 +84,33 @@ pub mod pallet { impl<T: Config> Pallet<T> { /// Verify a target header is finalized according to the given finality proof. /// - /// Will use the underlying storage pallet to fetch information about the current + /// It will use the underlying storage pallet to fetch information about the current /// authorities and best finalized header in order to verify that the header is finalized. /// - /// If successful in verification, it will write the header as well as its ancestors (from - /// the given `ancestry_proof`) to the underlying storage pallet. - /// - /// Note that the expected format for `ancestry_proof` is a continguous list of finalized - /// headers containing (current_best_finalized_header, finality_target] + /// If successful in verification, it will write the target header to the underlying storage + /// pallet. #[pallet::weight(0)] pub fn submit_finality_proof( origin: OriginFor<T>, finality_target: BridgedHeader<T>, justification: Vec<u8>, - ancestry_proof: Vec<BridgedHeader<T>>, + ancestry_proof: T::AncestryProof, ) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; frame_support::debug::trace!("Going to try and finalize header {:?}", finality_target); - frame_support::debug::trace!("Got ancestry proof of length {}", ancestry_proof.len()); - - ensure!( - ancestry_proof.len() <= T::MaxHeadersInSingleProof::get().as_(), - <Error<T>>::OversizedAncestryProof - ); let authority_set = T::HeaderChain::authority_set(); let voter_set = VoterSet::new(authority_set.authorities).ok_or(<Error<T>>::InvalidAuthoritySet)?; let set_id = authority_set.set_id; - verify_justification::<BridgedHeader<T>>( - (finality_target.hash(), *finality_target.number()), - set_id, - voter_set, - &justification, - ) - .map_err(|e| { - frame_support::debug::error!("Received invalid justification for {:?}: {:?}", finality_target, e); - <Error<T>>::InvalidJustification - })?; + let (hash, number) = (finality_target.hash(), *finality_target.number()); + verify_justification::<BridgedHeader<T>>((hash, number), set_id, voter_set, &justification).map_err( + |e| { + frame_support::debug::error!("Received invalid justification for {:?}: {:?}", finality_target, e); + <Error<T>>::InvalidJustification + }, + )?; let best_finalized = T::HeaderChain::best_finalized(); frame_support::debug::trace!("Checking ancestry against best finalized header: {:?}", &best_finalized); @@ -135,17 +120,8 @@ pub mod pallet { <Error<T>>::InvalidAncestryProof ); - // Note that this won't work if we ever change the `ancestry_proof` format to be - // sparse since this expects a contiguous set of finalized headers. - let _ = T::HeaderChain::append_finalized_chain(ancestry_proof).map_err(|_| { - frame_support::debug::error!("Failed to append finalized header chain."); - <Error<T>>::FailedToWriteHeader - })?; - - frame_support::debug::info!( - "Succesfully imported finalized header chain for target header {:?}!", - finality_target - ); + T::HeaderChain::append_header(finality_target); + frame_support::debug::info!("Succesfully imported finalized header with hash {:?}!", hash); Ok(().into()) } @@ -162,8 +138,6 @@ pub mod pallet { InvalidAuthoritySet, /// Failed to write a header to the underlying header chain. FailedToWriteHeader, - /// The given ancestry proof is too large to be verified in a single transaction. - OversizedAncestryProof, } } @@ -283,27 +257,6 @@ mod tests { }) } - #[test] - fn disallows_ancestry_proofs_which_are_too_large() { - run_test(|| { - initialize_substrate_bridge(); - - let header = test_header(1); - let justification = [1u8; 32].encode(); - - let mut ancestry_proof = vec![]; - let max_len = <TestRuntime as Config>::MaxHeadersInSingleProof::get(); - for i in 1..=max_len + 1 { - ancestry_proof.push(test_header(i as u64)); - } - - assert_err!( - Module::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification, ancestry_proof,), - <Error<TestRuntime>>::OversizedAncestryProof - ); - }) - } - #[test] fn disallows_invalid_authority_set() { run_test(|| { diff --git a/bridges/modules/finality-verifier/src/mock.rs b/bridges/modules/finality-verifier/src/mock.rs index daacc4584123c8a50de59e396a787f1d2269da26..1ef1249545906e06030559e14d1862511c632c0a 100644 --- a/bridges/modules/finality-verifier/src/mock.rs +++ b/bridges/modules/finality-verifier/src/mock.rs @@ -71,14 +71,14 @@ impl pallet_substrate_bridge::Config for TestRuntime { } parameter_types! { - pub const MaxHeadersInSingleProof: u8 = 5; + pub const MaxElementsInSingleProof: Option<u32> = Some(5); } impl crate::pallet::Config for TestRuntime { type BridgedChain = TestBridgedChain; type HeaderChain = pallet_substrate_bridge::Module<TestRuntime>; - type AncestryChecker = Checker<<Self::BridgedChain as Chain>::Header, Vec<<Self::BridgedChain as Chain>::Header>>; - type MaxHeadersInSingleProof = MaxHeadersInSingleProof; + type AncestryProof = Vec<<Self::BridgedChain as Chain>::Header>; + type AncestryChecker = Checker<<Self::BridgedChain as Chain>::Header, Self::AncestryProof>; } #[derive(Debug)] diff --git a/bridges/modules/substrate/src/lib.rs b/bridges/modules/substrate/src/lib.rs index 2a3ad1b0184fd346bbad033cfed29b8cbb24c518..c14db8596f89ee13bf5cfb2f407038eece2d9ff3 100644 --- a/bridges/modules/substrate/src/lib.rs +++ b/bridges/modules/substrate/src/lib.rs @@ -375,25 +375,8 @@ impl<T: Config> bp_header_chain::HeaderChain<BridgedHeader<T>, sp_runtime::Dispa PalletStorage::<T>::new().current_authority_set() } - fn append_finalized_chain( - headers: impl IntoIterator<Item = BridgedHeader<T>>, - ) -> Result<(), sp_runtime::DispatchError> { - let mut storage = PalletStorage::<T>::new(); - - let mut header_iter = headers.into_iter().peekable(); - let first_header = header_iter.peek().ok_or(Error::<T>::NotDescendant)?; - - // Quick ancestry check to make sure we're not writing complete nonsense to storage - ensure!( - <BestFinalized<T>>::get() == *first_header.parent_hash(), - Error::<T>::NotDescendant, - ); - - for header in header_iter { - import_header_unchecked::<_, T>(&mut storage, header); - } - - Ok(()) + fn append_header(header: BridgedHeader<T>) { + import_header_unchecked::<_, T>(&mut PalletStorage::<T>::new(), header); } } @@ -923,36 +906,15 @@ mod tests { init_with_origin(Origin::root()).unwrap(); let storage = PalletStorage::<TestRuntime>::new(); - let child = test_header(2); - let header = test_header(3); - - let header_chain = vec![child.clone(), header.clone()]; - assert_ok!(Module::<TestRuntime>::append_finalized_chain(header_chain)); + let header = test_header(2); + Module::<TestRuntime>::append_header(header.clone()); - assert!(storage.header_by_hash(child.hash()).unwrap().is_finalized); assert!(storage.header_by_hash(header.hash()).unwrap().is_finalized); - assert_eq!(storage.best_finalized_header().header, header); assert_eq!(storage.best_headers()[0].hash, header.hash()); }) } - #[test] - fn prevents_unchecked_header_import_if_headers_are_unrelated() { - run_test(|| { - init_with_origin(Origin::root()).unwrap(); - - // Pallet is expecting test_header(2) as the child - let not_a_child = test_header(3); - let header_chain = vec![not_a_child]; - - assert_noop!( - Module::<TestRuntime>::append_finalized_chain(header_chain), - Error::<TestRuntime>::NotDescendant, - ); - }) - } - #[test] fn importing_unchecked_headers_enacts_new_authority_set() { run_test(|| { @@ -968,7 +930,7 @@ mod tests { header.digest = fork_tests::change_log(0); // Let's import our test header - assert_ok!(Module::<TestRuntime>::append_finalized_chain(vec![header.clone()])); + Module::<TestRuntime>::append_header(header.clone()); // Make sure that our header is the best finalized assert_eq!(storage.best_finalized_header().header, header); @@ -998,8 +960,8 @@ mod tests { let header = test_header(3); // Let's import our test headers - let header_chain = vec![schedules_change, header.clone()]; - assert_ok!(Module::<TestRuntime>::append_finalized_chain(header_chain)); + Module::<TestRuntime>::append_header(schedules_change); + Module::<TestRuntime>::append_header(header.clone()); // Make sure that our header is the best finalized assert_eq!(storage.best_finalized_header().header, header); @@ -1039,8 +1001,7 @@ mod tests { // We are expecting an authority set change at height 2, so this header should enact // that upon being imported. - let header_chain = vec![test_header(2)]; - assert_ok!(Module::<TestRuntime>::append_finalized_chain(header_chain)); + Module::<TestRuntime>::append_header(test_header(2)); // Make sure that the authority set actually changed upon importing our header assert_eq!( diff --git a/bridges/primitives/header-chain/src/lib.rs b/bridges/primitives/header-chain/src/lib.rs index a9623b13dc4f5c2460fd9332df55c1b01f914ac7..1663717646021b89ee74bf0b4e367807ab38455e 100644 --- a/bridges/primitives/header-chain/src/lib.rs +++ b/bridges/primitives/header-chain/src/lib.rs @@ -69,7 +69,7 @@ pub trait InclusionProofVerifier { fn verify_transaction_inclusion_proof(proof: &Self::TransactionInclusionProof) -> Option<Self::Transaction>; } -/// A base trait for pallets which want to keep track of a full set of headers from a bridged chain. +/// A trait for pallets which want to keep track of finalized headers from a bridged chain. pub trait HeaderChain<H, E> { /// Get the best finalized header known to the header chain. fn best_finalized() -> H; @@ -77,14 +77,8 @@ pub trait HeaderChain<H, E> { /// Get the best authority set known to the header chain. fn authority_set() -> AuthoritySet; - /// Write a finalized chain of headers to the underlying pallet storage. - /// - /// It is assumed that each header in this chain been finalized, and that the given headers are - /// in order (e.g vec![header_1, header_2, ..., header_n]). - /// - /// This function should fail if the first header is not a child of the current best finalized - /// header known to the underlying pallet storage. - fn append_finalized_chain(headers: impl IntoIterator<Item = H>) -> Result<(), E>; + /// Write a header finalized by GRANDPA to the underlying pallet storage. + fn append_header(header: H); } impl<H: Default, E> HeaderChain<H, E> for () { @@ -96,9 +90,7 @@ impl<H: Default, E> HeaderChain<H, E> for () { AuthoritySet::default() } - fn append_finalized_chain(_headers: impl IntoIterator<Item = H>) -> Result<(), E> { - Ok(()) - } + fn append_header(_header: H) {} } /// A trait for checking if a given child header is a direct descendant of an ancestor.