More refactoring and tests.

core/sr-primitives/src/weights.rs

@@ -30,7 +30,7 @@ pub use crate::transaction_validity::TransactionPriority;
 use crate::traits::Bounded;
 
 /// Numeric range of a transaction weight.
-pub type Weight = u64;
+pub type Weight = u32;
 
 /// A generalized group of dispatch types. This is only distinguishing normal, user-triggered transactions
 /// (`Normal`) and anything beyond which serves a higher purpose to the system (`Operational`).
@@ -144,11 +144,11 @@ pub enum SimpleDispatchInfo {
 impl<T> WeighData<T> for SimpleDispatchInfo {
 	fn weigh_data(&self, _: T) -> Weight {
 		match self {
-			SimpleDispatchInfo::FixedNormal(w) => *w * 1000,
+			SimpleDispatchInfo::FixedNormal(w) => *w,
 			SimpleDispatchInfo::MaxNormal => Bounded::max_value(),
 			SimpleDispatchInfo::FreeNormal => Bounded::min_value(),
 
-			SimpleDispatchInfo::FixedOperational(w) => *w * 1000,
+			SimpleDispatchInfo::FixedOperational(w) => *w,
 			SimpleDispatchInfo::MaxOperational => Bounded::max_value(),
 			SimpleDispatchInfo::FreeOperational => Bounded::min_value(),
 		}

node-template/runtime/src/lib.rs

@@ -8,16 +8,11 @@
 #[cfg(feature = "std")]
 include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));
 
-#[cfg(feature = "std")]
-use serde::{Serialize, Deserialize};
-use parity_codec::{Encode, Decode};
 use rstd::prelude::*;
-#[cfg(feature = "std")]
-use primitives::bytes;
 use primitives::{ed25519, sr25519, OpaqueMetadata};
 use sr_primitives::{
 	ApplyResult, transaction_validity::TransactionValidity, generic, create_runtime_str,
-	traits::{self, NumberFor, BlakeTwo256, Block as BlockT, StaticLookup, Verify}, weights::Weight,
+	traits::{NumberFor, BlakeTwo256, Block as BlockT, StaticLookup, Verify}, weights::Weight,
 };
 use client::{
 	block_builder::api::{CheckInherentsResult, InherentData, self as block_builder_api},
@@ -169,8 +164,9 @@ parameter_types! {
 	pub const ExistentialDeposit: u128 = 500;
 	pub const TransferFee: u128 = 0;
 	pub const CreationFee: u128 = 0;
-	pub const TransactionBaseFee: u128 = 1;
-	pub const TransactionByteFee: u128 = 0;
+	pub const TransactionBaseFee: u128 = 0;
+	pub const TransactionByteFee: u128 = 1;
+	pub const TransactionWeightFee: u128 = 1;
 }
 
 impl balances::Trait for Runtime {
@@ -191,6 +187,7 @@ impl balances::Trait for Runtime {
 	type CreationFee = CreationFee;
 	type TransactionBaseFee = TransactionBaseFee;
 	type TransactionByteFee = TransactionByteFee;
+	type TransactionWeightFee = TransactionWeightFee;
 }
 
 impl sudo::Trait for Runtime {

node-template/runtime/src/template.rs

@@ -89,7 +89,7 @@ mod tests {
 		pub const BlockHashCount: u64 = 250;
 		pub const MaximumBlockWeight: Weight = 1024;
 		pub const MaximumBlockLength: u32 = 2 * 1024;
-		pub const AvailableBlockRatio: Perbill = Perbill::from_percent(75));
+		pub const AvailableBlockRatio: Perbill = Perbill::from_percent(75);
 	}
 	impl system::Trait for Test {
 		type Origin = Origin;

node/executor/src/lib.rs

@@ -49,7 +49,7 @@ mod tests {
 	use node_primitives::{Hash, BlockNumber, AccountId, Balance, Index};
 	use runtime_primitives::traits::{Header as HeaderT, Hash as HashT};
 	use runtime_primitives::{generic::Era, ApplyOutcome, ApplyError, ApplyResult, Perbill};
-	use runtime_primitives::weights::{WeightMultiplier, SimpleDispatchInfo, GetDispatchInfo, WeighData, Weight};
+	use runtime_primitives::weights::{WeightMultiplier, GetDispatchInfo, Weight};
 	use {balances, contracts, indices, staking, system, timestamp};
 	use contracts::ContractAddressFor;
 	use system::{EventRecord, Phase};
@@ -82,22 +82,25 @@ mod tests {
 
 	type TestExternalities<H> = CoreTestExternalities<H, u64>;
 
+	/// Default transfer fee
 	fn transfer_fee<E: Encode>(extrinsic: &E) -> Balance {
 		let length_fee = <TransactionBaseFee as Get<Balance>>::get() +
 			<TransactionByteFee as Get<Balance>>::get() *
 			(extrinsic.encode().len() as Balance);
-		let weight_fee = default_transfer_call().get_dispatch_info().weight as Balance;
-		// TODO: with this we have 100% accurate fee estimation. Should probably be able to get rid
-		// of `assert_eq_error_rate!` now. But it makes test mockup much more complicated because
-		// the fee for each block can only be estimated _before_ that block.
+		let mut weight_fee = default_transfer_call().get_dispatch_info().weight as Balance;
+		weight_fee = weight_fee * 1_000_000 as Balance;
+		// NOTE: we don't need this anymore, but in case needed, this makes the fee accurate over
+		// multiple blocks.
 		// weight_fee = <system::Module<Runtime>>::next_weight_multiplier().apply_to(weight_fee as u32) as Balance;
 		length_fee + weight_fee
 	}
 
-	fn multiplier_target() -> Weight {
+	/// Get the target weight of the fee multiplier.
+	fn _multiplier_target() -> Weight {
 		<MaximumBlockWeight as Get<Weight>>::get() / 4
 	}
 
+	/// Default creation fee.
 	fn creation_fee() -> Balance {
 		<CreationFee as Get<Balance>>::get()
 	}
@@ -972,21 +975,6 @@ mod tests {
 
 		let mut tt = new_test_ext(COMPACT_CODE, false);
 
-		// This test can either use `fill_block()`, which is controversial, or should be dropped.
-		// NOTE: This assumes that system::remark has the default.
-		// let num_to_exhaust = (multiplier_target() + 100) / SimpleDispatchInfo::default().weigh_data(());
-		// println!("++ Generating {} transactions to fill {} weight units", num_to_exhaust, multiplier_target() + 100);
-		// let mut xts = (0..num_to_exhaust).map(|i| CheckedExtrinsic {
-		// 	signed: Some((charlie(), signed_extra(i.into(), 0))),
-		// 	function: Call::System(system::Call::remark(vec![0; 1])),
-		// }).collect::<Vec<CheckedExtrinsic>>();
-		// xts.insert(0, CheckedExtrinsic {
-		// 	signed: None,
-		// 	function: Call::Timestamp(timestamp::Call::set(42)),
-		// });
-
-		println!("++ Generated all extrinsics. Constructing blocks.");
-
 		// big one in terms of weight.
 		let block1 = construct_block(
 			&mut tt,
@@ -1057,6 +1045,85 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn transaction_fee_is_correct_ultimate() {
+		// This uses the exact values of substrate-node.
+		//
+		// weight of transfer call as of now: 1_000
+		// if weight of cheapest weight would be 10^7, this would be 10^9, which is:
+		//   - 1 MILLICENTS in substrate node.
+		//   - 1 milldot based on current polkadot runtime.
+		// (this baed on assigning 0.1 CENT to the cheapest tx with `weight = 100`)
+		let mut t = TestExternalities::<Blake2Hasher>::new_with_code(COMPACT_CODE, map![
+			blake2_256(&<balances::FreeBalance<Runtime>>::key_for(alice())).to_vec() => {
+				(100 * DOLLARS).encode()
+			},
+			blake2_256(&<balances::FreeBalance<Runtime>>::key_for(bob())).to_vec() => {
+				(10 * DOLLARS).encode()
+			},
+			twox_128(<balances::TotalIssuance<Runtime>>::key()).to_vec() => {
+				(110 * DOLLARS).encode()
+			},
+			twox_128(<indices::NextEnumSet<Runtime>>::key()).to_vec() => vec![0u8; 16],
+			blake2_256(&<system::BlockHash<Runtime>>::key_for(0)).to_vec() => vec![0u8; 32]
+		]);
+
+		let tip = 1_000_000;
+		let xt = sign(CheckedExtrinsic {
+			signed: Some((alice(), signed_extra(0, tip))),
+			function: Call::Balances(default_transfer_call()),
+		});
+
+		let r = executor().call::<_, NeverNativeValue, fn() -> _>(
+			&mut t,
+			"Core_initialize_block",
+			&vec![].and(&from_block_number(1u64)),
+			true,
+			None,
+		).0;
+
+		assert!(r.is_ok());
+		let r = executor().call::<_, NeverNativeValue, fn() -> _>(
+			&mut t,
+			"BlockBuilder_apply_extrinsic",
+			&vec![].and(&xt.clone()),
+			true,
+			None,
+		).0;
+		assert!(r.is_ok());
+
+		runtime_io::with_externalities(&mut t, || {
+			assert_eq!(Balances::total_balance(&bob()), (10 + 69) * DOLLARS);
+			// Components deducted from alice's balances:
+			// - Weight fee
+			// - Length fee
+			// - Tip
+			// - Creation-fee of bob's account.
+			let mut balance_alice = (100 - 69) * DOLLARS;
+
+			let length_fee = <TransactionBaseFee as Get<Balance>>::get() +
+				<TransactionByteFee as Get<Balance>>::get() *
+				(xt.clone().encode().len() as Balance);
+			println!("++ len fee = {:?}", length_fee);
+			balance_alice -= length_fee;
+
+			// TODO: the 1_000_000 should go away.
+			let weight_fee = default_transfer_call().get_dispatch_info().weight as Balance * 1_000_000;
+			// we know that weight to fee multiplier is effect-less in block 1.
+			assert_eq!(weight_fee as Balance, MILLICENTS);
+			balance_alice -= weight_fee;
+			println!("++ wei fee = {:?}", weight_fee);
+
+			balance_alice -= tip;
+			println!("++ tip fee = {:?}", tip);
+
+			// TODO: why again??? creation fee should not be deducted here.
+			balance_alice -= creation_fee();
+
+			assert_eq!(Balances::total_balance(&alice()), balance_alice);
+		});
+	}
+
 	#[cfg(feature = "benchmarks")]
 	mod benches {
 		use super::*;

node/runtime/src/impls.rs

@@ -222,8 +222,8 @@ mod tests {
 			});
 
 		// Some values that are all above the target and will cause an increase.
-		let target_weight = TARGET_BLOCK_FULLNESS * <MaximumBlockWeight as Get<Weight>>::get();
-		vec![target_weight + 100, target_weight * 2, target_weight * 4]
+		let t = target();
+		vec![t + 100, t * 2, t * 4]
 			.into_iter()
 			.for_each(|i| {
 				let fm = WeightMultiplierUpdateHandler::convert((

node/runtime/src/lib.rs

@@ -103,7 +103,7 @@ pub type DealWithFees = SplitTwoWays<
 
 parameter_types! {
 	pub const BlockHashCount: BlockNumber = 250;
-	pub const MaximumBlockWeight: Weight = 1_000_000_000;
+	pub const MaximumBlockWeight: Weight = 1_000_000;
 	pub const AvailableBlockRatio: Perbill = Perbill::from_percent(75);
 	pub const MaximumBlockLength: u32 = 5 * 1024 * 1024;
 }
@@ -143,6 +143,7 @@ parameter_types! {
 	pub const CreationFee: Balance = 1 * CENTS;
 	pub const TransactionBaseFee: Balance = 1 * CENTS;
 	pub const TransactionByteFee: Balance = 10 * MILLICENTS;
+	pub const TransactionWeightFee: Balance = MILLICENTS / 1000;
 }
 
 impl balances::Trait for Runtime {
@@ -158,6 +159,7 @@ impl balances::Trait for Runtime {
 	type CreationFee = CreationFee;
 	type TransactionBaseFee = TransactionBaseFee;
 	type TransactionByteFee = TransactionByteFee;
+	type TransactionWeightFee = TransactionWeightFee;
 }
 
 parameter_types! {

srml/balances/src/lib.rs

@@ -206,6 +206,9 @@ pub trait Subtrait<I: Instance = DefaultInstance>: system::Trait {
 
 	/// The fee to be paid for making a transaction; the per-byte portion.
 	type TransactionByteFee: Get<Self::Balance>;
+
+	/// The amount of fee deducted oer unit of weight.
+	type TransactionWeightFee: Get<Self::Balance>;
 }
 
 pub trait Trait<I: Instance = DefaultInstance>: system::Trait {
@@ -249,6 +252,9 @@ pub trait Trait<I: Instance = DefaultInstance>: system::Trait {
 
 	/// The fee to be paid for making a transaction; the per-byte portion.
 	type TransactionByteFee: Get<Self::Balance>;
+
+	/// The amount of fee deducted oer unit of weight.
+	type TransactionWeightFee: Get<Self::Balance>;
 }
 
 impl<T: Trait<I>, I: Instance> Subtrait<I> for T {
@@ -260,6 +266,7 @@ impl<T: Trait<I>, I: Instance> Subtrait<I> for T {
 	type CreationFee = T::CreationFee;
 	type TransactionBaseFee = T::TransactionBaseFee;
 	type TransactionByteFee = T::TransactionByteFee;
+	type TransactionWeightFee = T::TransactionWeightFee;
 }
 
 decl_event!(
@@ -783,6 +790,7 @@ impl<T: Subtrait<I>, I: Instance> Trait<I> for ElevatedTrait<T, I> {
 	type CreationFee = T::CreationFee;
 	type TransactionBaseFee = T::TransactionBaseFee;
 	type TransactionByteFee = T::TransactionByteFee;
+	type TransactionWeightFee = T::TransactionWeightFee;
 }
 
 impl<T: Trait<I>, I: Instance> Currency<T::AccountId> for Module<T, I>
@@ -1174,20 +1182,24 @@ impl<T: Trait<I>, I: Instance> TakeFees<T, I> {
 	///   - (optional) _tip_: if included in the transaction, it will be added on top. Only signed
 	///      transactions can have a tip.
 	fn compute_fee(len: usize, info: DispatchInfo, tip: T::Balance) -> T::Balance {
-		// length fee
 		let len_fee = if info.pay_length_fee() {
 			let len = T::Balance::from(len as u32);
 			let base = T::TransactionBaseFee::get();
-			let byte = T::TransactionByteFee::get();
-			base.saturating_add(byte.saturating_mul(len))
+			let per_byte = T::TransactionByteFee::get();
+			base.saturating_add(per_byte.saturating_mul(len))
 		} else {
 			Zero::zero()
 		};
 
-		// weight fee
-		let weight = info.weight;
-		let weight_fee: T::Balance = <system::Module<T>>::next_weight_multiplier()
-			.apply_to(weight).saturated_into();
+		let weight_fee = {
+			// cap the weight to the maximum defined in runtime, otherwise it will be the `Bounded`
+			// maximum of its data type, which is not desired.
+			let capped_weight = info.weight.min(<T as system::Trait>::MaximumBlockWeight::get());
+			let weight_fee: T::Balance = <system::Module<T>>::next_weight_multiplier()
+				.apply_to(capped_weight).saturated_into();
+			let per_weight = T::TransactionWeightFee::get();
+			weight_fee.saturating_mul(per_weight)
+		};
 
 		len_fee.saturating_add(weight_fee).saturating_add(tip)
 	}

srml/balances/src/mock.rs

@@ -36,6 +36,7 @@ thread_local! {
 	static CREATION_FEE: RefCell<u64> = RefCell::new(0);
 	static TRANSACTION_BASE_FEE: RefCell<u64> = RefCell::new(0);
 	static TRANSACTION_BYTE_FEE: RefCell<u64> = RefCell::new(1);
+	static TRANSACTION_WEIGHT_FEE: RefCell<u64> = RefCell::new(1);
 }
 
 pub struct ExistentialDeposit;
@@ -63,6 +64,11 @@ impl Get<u64> for TransactionByteFee {
 	fn get() -> u64 { TRANSACTION_BYTE_FEE.with(|v| *v.borrow()) }
 }
 
+pub struct TransactionWeightFee;
+impl Get<u64> for TransactionWeightFee {
+	fn get() -> u64 { TRANSACTION_WEIGHT_FEE.with(|v| *v.borrow()) }
+}
+
 // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct Runtime;
@@ -101,11 +107,13 @@ impl Trait for Runtime {
 	type CreationFee = CreationFee;
 	type TransactionBaseFee = TransactionBaseFee;
 	type TransactionByteFee = TransactionByteFee;
+	type TransactionWeightFee = TransactionWeightFee;
 }
 
 pub struct ExtBuilder {
 	transaction_base_fee: u64,
 	transaction_byte_fee: u64,
+	transaction_weight_fee: u64,
 	existential_deposit: u64,
 	transfer_fee: u64,
 	creation_fee: u64,
@@ -117,6 +125,7 @@ impl Default for ExtBuilder {
 		Self {
 			transaction_base_fee: 0,
 			transaction_byte_fee: 0,
+			transaction_weight_fee: 0,
 			existential_deposit: 0,
 			transfer_fee: 0,
 			creation_fee: 0,
@@ -126,9 +135,10 @@ impl Default for ExtBuilder {
 	}
 }
 impl ExtBuilder {
-	pub fn transaction_fees(mut self, base_fee: u64, byte_fee: u64) -> Self {
+	pub fn transaction_fees(mut self, base_fee: u64, byte_fee: u64, weight_fee: u64) -> Self {
 		self.transaction_base_fee = base_fee;
 		self.transaction_byte_fee = byte_fee;
+		self.transaction_weight_fee = weight_fee;
 		self
 	}
 	pub fn existential_deposit(mut self, existential_deposit: u64) -> Self {
@@ -161,6 +171,7 @@ impl ExtBuilder {
 		CREATION_FEE.with(|v| *v.borrow_mut() = self.creation_fee);
 		TRANSACTION_BASE_FEE.with(|v| *v.borrow_mut() = self.transaction_base_fee);
 		TRANSACTION_BYTE_FEE.with(|v| *v.borrow_mut() = self.transaction_byte_fee);
+		TRANSACTION_WEIGHT_FEE.with(|v| *v.borrow_mut() = self.transaction_weight_fee);
 	}
 	pub fn build(self) -> runtime_io::TestExternalities<Blake2Hasher> {
 		self.set_associated_consts();