Update TargetMinGasPrice once in a block (polkadot-evm#435)

* Update `TargetMinGasPrice` once in a block * Update unit test * Add dispatch class * Update change log * Fix review * Use assert! instead of ensure!
web3labs · Aug 3, 2021 · dbb8d06 · dbb8d06
1 parent a207dc5
commit dbb8d06
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 5 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/frame/dynamic-fee/CHANGELOG.md b/frame/dynamic-fee/CHANGELOG.md
@@ -2,4 +2,5 @@
 
 ## Unreleased
 
-* Uses unreleased pallet-evm 5.0.0-dev
+* Uses unreleased pallet-evm 5.0.0-dev
+* Update `TargetGasPrice` once in a block
diff --git a/frame/dynamic-fee/Cargo.toml b/frame/dynamic-fee/Cargo.toml
@@ -17,6 +17,10 @@ sp-inherents = { version = "3.0.0", default-features = false, git = "https://git
 frame-system = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 frame-support = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 
+[dev-dependencies]
+pallet-timestamp = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
+sp-io = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
+
 [features]
 default = ["std"]
 std = [

diff --git a/frame/dynamic-fee/src/lib.rs b/frame/dynamic-fee/src/lib.rs
@@ -19,7 +19,11 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 use codec::{Decode, Encode};
-use frame_support::{decl_module, decl_storage, traits::Get, weights::Weight};
+use frame_support::{
+	decl_module, decl_storage,
+	traits::Get,
+	weights::{DispatchClass, Weight},
+};
 use frame_system::ensure_none;
 use sp_core::U256;
 #[cfg(feature = "std")]
@@ -57,7 +61,7 @@ decl_module! {
 		}
 
 		fn on_finalize(_n: T::BlockNumber) {
-			if let Some(target) = TargetMinGasPrice::get() {
+			if let Some(target) = TargetMinGasPrice::take() {
 				let bound = MinGasPrice::get() / T::MinGasPriceBoundDivisor::get() + U256::one();
 
 				let upper_limit = MinGasPrice::get().saturating_add(bound);
@@ -67,12 +71,13 @@ decl_module! {
 			}
 		}
 
-		#[weight = T::DbWeight::get().writes(1)]
-		fn note_min_gas_price_target(
+		#[weight = (T::DbWeight::get().writes(1), DispatchClass::Mandatory)]
+		pub fn note_min_gas_price_target(
 			origin,
 			target: U256,
 		) {
 			ensure_none(origin)?;
+			assert!(TargetMinGasPrice::get().is_none(), "TargetMinGasPrice must be updated only once in the block");
 
 			TargetMinGasPrice::set(Some(target));
 		}
@@ -146,3 +151,115 @@ impl<T: Config> ProvideInherent for Module<T> {
 		Ok(())
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate as pallet_dynamic_fee;
+
+	use frame_support::{
+		assert_ok, parameter_types,
+		traits::{OnFinalize, OnInitialize},
+	};
+	use sp_core::{H256, U256};
+	use sp_io::TestExternalities;
+	use sp_runtime::{
+		testing::Header,
+		traits::{BlakeTwo256, IdentityLookup},
+	};
+
+	pub fn new_test_ext() -> TestExternalities {
+		let t = frame_system::GenesisConfig::default()
+			.build_storage::<Test>()
+			.unwrap();
+		TestExternalities::new(t)
+	}
+
+	type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
+	type Block = frame_system::mocking::MockBlock<Test>;
+
+	parameter_types! {
+		pub const BlockHashCount: u64 = 250;
+		pub BlockWeights: frame_system::limits::BlockWeights =
+			frame_system::limits::BlockWeights::simple_max(1024);
+	}
+	impl frame_system::Config for Test {
+		type BaseCallFilter = ();
+		type BlockWeights = ();
+		type BlockLength = ();
+		type DbWeight = ();
+		type Origin = Origin;
+		type Index = u64;
+		type BlockNumber = u64;
+		type Call = Call;
+		type Hash = H256;
+		type Hashing = BlakeTwo256;
+		type AccountId = u64;
+		type Lookup = IdentityLookup<Self::AccountId>;
+		type Header = Header;
+		type Event = Event;
+		type BlockHashCount = BlockHashCount;
+		type Version = ();
+		type PalletInfo = PalletInfo;
+		type AccountData = ();
+		type OnNewAccount = ();
+		type OnKilledAccount = ();
+		type SystemWeightInfo = ();
+		type SS58Prefix = ();
+	}
+
+	frame_support::parameter_types! {
+		pub const MinimumPeriod: u64 = 1000;
+	}
+	impl pallet_timestamp::Config for Test {
+		type Moment = u64;
+		type OnTimestampSet = ();
+		type MinimumPeriod = MinimumPeriod;
+		type WeightInfo = ();
+	}
+
+	frame_support::parameter_types! {
+		pub BoundDivision: U256 = 1024.into();
+	}
+	impl Config for Test {
+		type MinGasPriceBoundDivisor = BoundDivision;
+	}
+
+	frame_support::construct_runtime!(
+		pub enum Test where
+			Block = Block,
+			NodeBlock = Block,
+			UncheckedExtrinsic = UncheckedExtrinsic,
+		{
+			System: frame_system::{Module, Call, Config, Storage, Event<T>},
+			Timestamp: pallet_timestamp::{Module, Call, Storage},
+			DynamicFee: pallet_dynamic_fee::{Module, Call, Storage, Inherent},
+		}
+	);
+
+	fn run_to_block(n: u64) {
+		while System::block_number() < n {
+			DynamicFee::on_finalize(System::block_number());
+			System::set_block_number(System::block_number() + 1);
+			DynamicFee::on_initialize(System::block_number());
+		}
+	}
+
+	#[test]
+	#[should_panic(expected = "TargetMinGasPrice must be updated only once in the block")]
+	fn double_set_in_a_block_failed() {
+		new_test_ext().execute_with(|| {
+			run_to_block(3);
+			assert_ok!(DynamicFee::note_min_gas_price_target(
+				Origin::none(),
+				U256::zero()
+			));
+			let _ = DynamicFee::note_min_gas_price_target(Origin::none(), U256::zero());
+			run_to_block(4);
+			assert_ok!(DynamicFee::note_min_gas_price_target(
+				Origin::none(),
+				U256::zero()
+			));
+		});
+	}
+}