Use bounded vector to store headers with the same number (paritytech#529

) * added bounded vec * removed empty expects calls * added tests for import header * renamed variables * get max headers from config * fixed checks * applied some refactorings * replaced custom insert/update code with try_append * removed dependency on bounded slice * removed map * made import headers transactional * remvoed whitespace
Snowfork · Oct 28, 2021 · 85f0599 · 85f0599
1 parent bd09924
commit 85f0599
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 31 deletions.
diff --git a/parachain/pallets/ethereum-light-client/src/lib.rs b/parachain/pallets/ethereum-light-client/src/lib.rs
@@ -34,10 +34,12 @@ use frame_system::ensure_signed;
 use frame_support::{
 	dispatch::{DispatchError, DispatchResult},
 	traits::Get,
+	transactional,
 	log,
 };
 use sp_runtime::RuntimeDebug;
 use sp_std::prelude::*;
+use sp_std::convert::TryInto;
 use codec::{Encode, Decode};
 
 use snowbridge_core::{Message, Verifier, Proof};
@@ -90,7 +92,7 @@ pub mod pallet {
 
 	use super::*;
 
-	use frame_support::pallet_prelude::*;
+	use frame_support::{BoundedVec, pallet_prelude::*};
 	use frame_system::pallet_prelude::*;
 
 	#[pallet::pallet]
@@ -113,6 +115,9 @@ pub mod pallet {
 		type VerifyPoW: Get<bool>;
 		/// Weight information for extrinsics in this pallet
 		type WeightInfo: WeightInfo;
+		/// The maximum numbers of headers to store in storage per block number.
+		#[pallet::constant]
+		type MaxHeadersForNumber: Get<u32>;
 	}
 
 	#[pallet::event]
@@ -139,6 +144,8 @@ pub mod pallet {
 		InvalidProof,
 		/// Log could not be decoded
 		DecodeFailed,
+		// Maximum quantity of headers for number reached
+		AtMaxHeadersForNumber,
 		/// This should never be returned - indicates a bug
 		Unknown,
 	}
@@ -164,7 +171,7 @@ pub mod pallet {
 
 	/// Map of imported header hashes by number.
 	#[pallet::storage]
-	pub(super) type HeadersByNumber<T: Config> = StorageMap<_, Twox64Concat, u64, Vec<H256>, OptionQuery>;
+	pub(super) type HeadersByNumber<T: Config> = StorageMap<_, Twox64Concat, u64, BoundedVec<H256, T::MaxHeadersForNumber>, OptionQuery>;
 
 	#[pallet::genesis_config]
 	pub struct GenesisConfig {
@@ -213,6 +220,7 @@ pub mod pallet {
 		/// - Iterating over ancestors: min `DescendantsUntilFinalized` reads to find the
 		///   newly finalized ancestor of a header.
 		#[pallet::weight(T::WeightInfo::import_header())]
+		#[transactional]
 		pub fn import_header(
 			origin: OriginFor<T>,
 			header: EthereumHeader,
@@ -370,19 +378,10 @@ pub mod pallet {
 				finalized: false,
 			};
 
-			<Headers<T>>::insert(hash, header_to_store);
+			<HeadersByNumber<T>>::try_append(header.number, hash)
+				.map_err(|_| Error::<T>::AtMaxHeadersForNumber)?;
 
-			if <HeadersByNumber<T>>::contains_key(header.number) {
-				<HeadersByNumber<T>>::mutate(header.number, |option| -> DispatchResult {
-					if let Some(hashes) = option {
-						hashes.push(hash);
-						return Ok(());
-					}
-					Err(Error::<T>::Unknown.into())
-				})?;
-			} else {
-				<HeadersByNumber<T>>::insert(header.number, vec![hash]);
-			}
+			<Headers<T>>::insert(hash, header_to_store);
 
 			// Maybe track new highest difficulty chain
 			let (_, highest_difficulty) = <BestBlock<T>>::get();
@@ -416,7 +415,7 @@ pub mod pallet {
 					&pruning_range,
 					HEADERS_TO_PRUNE_IN_SINGLE_IMPORT,
 					new_finalized_block_id.number.saturating_sub(FINALIZED_HEADERS_TO_KEEP),
-				);
+				)?;
 				if new_pruning_range != pruning_range {
 					<BlocksToPrune<T>>::put(new_pruning_range);
 				}
@@ -462,7 +461,7 @@ pub mod pallet {
 			pruning_range: &PruningRange,
 			max_headers_to_prune: u64,
 			prune_end: u64,
-		) -> PruningRange {
+		) -> Result<PruningRange, DispatchError> {
 			let mut new_pruning_range = pruning_range.clone();
 
 			// We can only increase this since pruning cannot be reverted...
@@ -490,7 +489,10 @@ pub mod pallet {
 					}
 
 					if remaining > 0 {
-						let remainder = &hashes_at_number[hashes_at_number.len() - remaining..];
+						let remainder: BoundedVec<H256, T::MaxHeadersForNumber> = hashes_at_number[hashes_at_number.len() - remaining..]
+							.to_vec()
+							.try_into()
+							.map_err(|_| Error::<T>::AtMaxHeadersForNumber)?;
 						<HeadersByNumber<T>>::insert(number, remainder);
 					} else {
 						new_pruning_range.oldest_unpruned_block = number + 1;
@@ -500,7 +502,7 @@ pub mod pallet {
 				}
 			}
 
-			new_pruning_range
+			Ok(new_pruning_range)
 		}
 
 		// Verifies that the receipt encoded in proof.data is included
@@ -576,7 +578,7 @@ pub mod pallet {
 			initial_difficulty: U256,
 			descendants_until_final: u8,
 		) -> Result<(), &'static str> {
-			let insert_header_fn = |header: &EthereumHeader, total_difficulty: U256| {
+			let insert_header_fn = |header: &EthereumHeader, total_difficulty: U256| -> Result<EthereumHeaderId, &'static str> {
 				let hash = header.compute_hash();
 				<Headers<T>>::insert(
 					hash,
@@ -587,17 +589,19 @@ pub mod pallet {
 						finalized: false,
 					},
 				);
-				<HeadersByNumber<T>>::append(header.number, hash);
 
-				EthereumHeaderId {
-					number: header.number,
-					hash: hash,
-				}
+				<HeadersByNumber<T>>::try_append(header.number, hash)
+					.map_err(|_| "Could not append header")?;
+
+					Ok(EthereumHeaderId {
+						number: header.number,
+						hash: hash,
+					})
 			};
 
 			let oldest_header = headers.get(0).ok_or("Need at least one header")?;
 			let mut best_block_difficulty = initial_difficulty;
-			let mut best_block_id = insert_header_fn(&oldest_header, best_block_difficulty);
+			let mut best_block_id = insert_header_fn(&oldest_header, best_block_difficulty)?;
 
 			for (i, header) in headers.iter().enumerate().skip(1) {
 				let prev_block_num = headers[i - 1].number;
@@ -611,7 +615,7 @@ pub mod pallet {
 					parent.total_difficulty + header.difficulty
 				};
 
-				let block_id = insert_header_fn(&header, total_difficulty);
+				let block_id = insert_header_fn(&header, total_difficulty)?;
 
 				if total_difficulty > best_block_difficulty {
 					best_block_difficulty = total_difficulty;

diff --git a/parachain/pallets/ethereum-light-client/src/mock.rs b/parachain/pallets/ethereum-light-client/src/mock.rs
@@ -71,6 +71,7 @@ pub mod mock_verifier {
 		pub const DescendantsUntilFinalized: u8 = 2;
 		pub const DifficultyConfig: EthereumDifficultyConfig = EthereumDifficultyConfig::mainnet();
 		pub const VerifyPoW: bool = false;
+		pub const MaxHeadersForNumber: u32 = 10;
 	}
 
 	impl verifier::Config for Test {
@@ -79,6 +80,7 @@ pub mod mock_verifier {
 		type DifficultyConfig = DifficultyConfig;
 		type VerifyPoW = VerifyPoW;
 		type WeightInfo = ();
+		type MaxHeadersForNumber = MaxHeadersForNumber;
 	}
 }
 
@@ -130,6 +132,7 @@ pub mod mock_verifier_with_pow {
 		pub const DescendantsUntilFinalized: u8 = 2;
 		pub const DifficultyConfig: EthereumDifficultyConfig = EthereumDifficultyConfig::mainnet();
 		pub const VerifyPoW: bool = true;
+		pub const MaxHeadersForNumber: u32 = 10;
 	}
 
 	impl verifier::Config for Test {
@@ -138,6 +141,7 @@ pub mod mock_verifier_with_pow {
 		type DifficultyConfig = DifficultyConfig;
 		type VerifyPoW = VerifyPoW;
 		type WeightInfo = ();
+		type MaxHeadersForNumber = MaxHeadersForNumber;
 	}
 }
 

diff --git a/parachain/pallets/ethereum-light-client/src/tests.rs b/parachain/pallets/ethereum-light-client/src/tests.rs
@@ -14,6 +14,7 @@ use crate::mock::mock_verifier::{
 	Verifier,
 	Test,
 	Origin,
+	MaxHeadersForNumber,
 };
 
 use frame_support::{assert_err, assert_ok};
@@ -24,7 +25,6 @@ use crate::{
 	HeadersByNumber, PruningRange,
 };
 
-
 #[test]
 fn it_tracks_highest_difficulty_ethereum_chain() {
 	new_tester::<Test>().execute_with(|| {
@@ -181,8 +181,9 @@ fn it_prunes_ethereum_headers_correctly() {
 			2,
 			1,
 		);
+		assert_ok!(&new_range);
 		assert_eq!(
-			new_range,
+			new_range.unwrap(),
 			PruningRange { oldest_unpruned_block: 1, oldest_block_to_keep: 1 },
 		);
 		assert!(!<Headers<Test>>::contains_key(genesis_ethereum_block_hash()));
@@ -194,8 +195,9 @@ fn it_prunes_ethereum_headers_correctly() {
 			1,
 			2,
 		);
+		assert_ok!(&new_range);
 		assert_eq!(
-			new_range,
+			new_range.unwrap(),
 			PruningRange { oldest_unpruned_block: 1, oldest_block_to_keep: 2 },
 		);
 		assert!(!<Headers<Test>>::contains_key(block1_hash));
@@ -208,8 +210,9 @@ fn it_prunes_ethereum_headers_correctly() {
 			2,
 			4,
 		);
+		assert_ok!(&new_range);
 		assert_eq!(
-			new_range,
+			new_range.unwrap(),
 			PruningRange { oldest_unpruned_block: 3, oldest_block_to_keep: 4 },
 		);
 		assert!(!<Headers<Test>>::contains_key(block4_hash));
@@ -352,7 +355,6 @@ fn it_confirms_receipt_inclusion_in_ropsten_london_header() {
 	});
 }
 
-
 #[test]
 fn it_denies_receipt_inclusion_for_invalid_proof() {
 	new_tester::<Test>().execute_with(|| {
@@ -480,3 +482,44 @@ fn it_denies_receipt_inclusion_for_invalid_header() {
 		));
 	});
 }
+
+#[test]
+fn it_can_only_import_max_headers_worth_of_headers() {
+	new_tester::<Test>().execute_with(|| {
+		const MAX_BLOCKS: u32 = MaxHeadersForNumber::get();
+		let ferdie: AccountId = Keyring::Ferdie.into();
+
+		let first_block = child_of_genesis_ethereum_header();
+
+		let mut blocks = Vec::new();
+
+		for idx in 1..(MAX_BLOCKS+1) {
+			let mut child = child_of_header(&first_block);
+			child.difficulty = idx.into();
+			blocks.push(child);
+		}
+
+		let mut last_block = child_of_header(&first_block);
+		last_block.difficulty = (MAX_BLOCKS + 1).into();
+
+		assert_ok!(Verifier::import_header(
+			Origin::signed(ferdie.clone()),
+			first_block,
+			Default::default(),
+		));
+
+		for block in blocks {
+			assert_ok!(Verifier::import_header(
+				Origin::signed(ferdie.clone()),
+				block,
+				Default::default(),
+			));
+		}
+
+		assert_err!(Verifier::import_header(
+			Origin::signed(ferdie.clone()),
+			last_block,
+			Default::default(),
+		), Error::<Test>::AtMaxHeadersForNumber);
+	});
+}
diff --git a/parachain/runtime/local/src/lib.rs b/parachain/runtime/local/src/lib.rs
@@ -563,6 +563,7 @@ parameter_types! {
 	pub const DescendantsUntilFinalized: u8 = 1;
 	pub const DifficultyConfig: EthereumDifficultyConfig = EthereumDifficultyConfig::ropsten();
 	pub const VerifyPoW: bool = false;
+	pub const MaxHeadersForNumber: u32 = 100;
 }
 
 impl ethereum_light_client::Config for Runtime {
@@ -571,6 +572,7 @@ impl ethereum_light_client::Config for Runtime {
 	type DifficultyConfig = DifficultyConfig;
 	type VerifyPoW = VerifyPoW;
 	type WeightInfo = ();
+	type MaxHeadersForNumber = MaxHeadersForNumber;
 }
 
 impl assets::Config for Runtime {

diff --git a/parachain/runtime/rococo/src/lib.rs b/parachain/runtime/rococo/src/lib.rs
@@ -567,6 +567,7 @@ parameter_types! {
 	pub const DescendantsUntilFinalized: u8 = 8;
 	pub const DifficultyConfig: EthereumDifficultyConfig = EthereumDifficultyConfig::ropsten();
 	pub const VerifyPoW: bool = true;
+	pub const MaxHeadersForNumber: u32 = 100;
 }
 
 impl ethereum_light_client::Config for Runtime {
@@ -575,6 +576,7 @@ impl ethereum_light_client::Config for Runtime {
 	type DifficultyConfig = DifficultyConfig;
 	type VerifyPoW = VerifyPoW;
 	type WeightInfo = weights::ethereum_light_client_weights::WeightInfo<Runtime>;
+	type MaxHeadersForNumber = MaxHeadersForNumber;
 }
 
 impl assets::Config for Runtime {

diff --git a/parachain/runtime/snowbridge/src/lib.rs b/parachain/runtime/snowbridge/src/lib.rs
@@ -567,6 +567,7 @@ parameter_types! {
 	pub const DescendantsUntilFinalized: u8 = 3;
 	pub const DifficultyConfig: EthereumDifficultyConfig = EthereumDifficultyConfig::mainnet();
 	pub const VerifyPoW: bool = true;
+	pub const MaxHeadersForNumber: u32 = 100;
 }
 
 impl ethereum_light_client::Config for Runtime {
@@ -575,6 +576,7 @@ impl ethereum_light_client::Config for Runtime {
 	type DifficultyConfig = DifficultyConfig;
 	type VerifyPoW = VerifyPoW;
 	type WeightInfo = weights::ethereum_light_client_weights::WeightInfo<Runtime>;
+	type MaxHeadersForNumber = MaxHeadersForNumber;
 }
 
 impl assets::Config for Runtime {