Split base fee and tip handling (#700)

* Split base fee and tip handling

* Return `NegativeImbalance`

* clippy

Author: tgmichel

frame/evm/src/lib.rs

@@ -673,15 +673,18 @@ pub trait OnChargeEVMTransaction<T: Config> {
 
 	/// After the transaction was executed the actual fee can be calculated.
 	/// This function should refund any overpaid fees and optionally deposit
-	/// the corrected amount.
+	/// the corrected amount, and handles the base fee rationing using the provided
+	/// `OnUnbalanced` implementation.
+	/// Returns the `NegativeImbalance` - if any - produced by the priority fee.
 	fn correct_and_deposit_fee(
 		who: &H160,
 		corrected_fee: U256,
+		base_fee: U256,
 		already_withdrawn: Self::LiquidityInfo,
-	);
+	) -> Self::LiquidityInfo;
 
-	/// Introduced in EIP1559 to handle the priority tip payment to the block Author.
-	fn pay_priority_fee(tip: U256);
+	/// Introduced in EIP1559 to handle the priority tip.
+	fn pay_priority_fee(tip: Self::LiquidityInfo);
 }
 
 /// Implements the transaction payment for a pallet implementing the `Currency`
@@ -725,8 +728,9 @@ where
 	fn correct_and_deposit_fee(
 		who: &H160,
 		corrected_fee: U256,
+		base_fee: U256,
 		already_withdrawn: Self::LiquidityInfo,
-	) {
+	) -> Self::LiquidityInfo {
 		if let Some(paid) = already_withdrawn {
 			let account_id = T::AddressMapping::into_account_id(*who);
 
@@ -763,13 +767,21 @@ where
 				.offset(refund_imbalance)
 				.same()
 				.unwrap_or_else(|_| C::NegativeImbalance::zero());
-			OU::on_unbalanced(adjusted_paid);
+
+			let (base_fee, tip) = adjusted_paid.split(base_fee.low_u128().unique_saturated_into());
+			// Handle base fee. Can be either burned, rationed, etc ...
+			OU::on_unbalanced(base_fee);
+			return Some(tip);
 		}
+		None
 	}
 
-	fn pay_priority_fee(tip: U256) {
-		let account_id = T::AddressMapping::into_account_id(<Pallet<T>>::find_author());
-		let _ = C::deposit_into_existing(&account_id, tip.low_u128().unique_saturated_into());
+	fn pay_priority_fee(tip: Self::LiquidityInfo) {
+		// Default Ethereum behaviour: issue the tip to the block author.
+		if let Some(tip) = tip {
+			let account_id = T::AddressMapping::into_account_id(<Pallet<T>>::find_author());
+			let _ = C::deposit_into_existing(&account_id, tip.peek());
+		}
 	}
 }
 
@@ -794,12 +806,13 @@ impl<T> OnChargeEVMTransaction<T> for ()
 	fn correct_and_deposit_fee(
 		who: &H160,
 		corrected_fee: U256,
+		base_fee: U256,
 		already_withdrawn: Self::LiquidityInfo,
-	) {
-		<EVMCurrencyAdapter::<<T as Config>::Currency, ()> as OnChargeEVMTransaction<T>>::correct_and_deposit_fee(who, corrected_fee, already_withdrawn)
+	) -> Self::LiquidityInfo {
+		<EVMCurrencyAdapter::<<T as Config>::Currency, ()> as OnChargeEVMTransaction<T>>::correct_and_deposit_fee(who, corrected_fee, base_fee, already_withdrawn)
 	}
 
-	fn pay_priority_fee(tip: U256) {
+	fn pay_priority_fee(tip: Self::LiquidityInfo) {
 		<EVMCurrencyAdapter::<<T as Config>::Currency, ()> as OnChargeEVMTransaction<T>>::pay_priority_fee(tip);
 	}
 }

frame/evm/src/runner/stack.rs

@@ -154,20 +154,18 @@ impl<T: Config> Runner<T> {
 
 		// Post execution.
 		let used_gas = U256::from(executor.used_gas());
-		let (actual_fee, actual_priority_fee) =
-			if let Some(max_priority_fee) = max_priority_fee_per_gas {
-				let actual_priority_fee = max_fee_per_gas
-					.saturating_sub(base_fee)
-					.min(max_priority_fee)
-					.saturating_mul(used_gas);
-				let actual_fee = executor
-					.fee(base_fee)
-					.checked_add(actual_priority_fee)
-					.unwrap_or_else(U256::max_value);
-				(actual_fee, Some(actual_priority_fee))
-			} else {
-				(executor.fee(base_fee), None)
-			};
+		let actual_fee = if let Some(max_priority_fee) = max_priority_fee_per_gas {
+			let actual_priority_fee = max_fee_per_gas
+				.saturating_sub(base_fee)
+				.min(max_priority_fee)
+				.saturating_mul(used_gas);
+			executor
+				.fee(base_fee)
+				.checked_add(actual_priority_fee)
+				.unwrap_or_else(U256::max_value)
+		} else {
+			executor.fee(base_fee)
+		};
 		log::debug!(
 			target: "evm",
 			"Execution {:?} [source: {:?}, value: {}, gas_limit: {}, actual_fee: {}, is_transactional: {}]",
@@ -199,10 +197,16 @@ impl<T: Config> Runner<T> {
 		// Refunded 200 - 40 = 160.
 		// Tip 5 * 6 = 30.
 		// Burned 200 - (160 + 30) = 10. Which is equivalent to gas_used * base_fee.
-		T::OnChargeTransaction::correct_and_deposit_fee(&source, actual_fee, fee);
-		if let Some(actual_priority_fee) = actual_priority_fee {
-			T::OnChargeTransaction::pay_priority_fee(actual_priority_fee);
-		}
+		let actual_priority_fee = T::OnChargeTransaction::correct_and_deposit_fee(
+			&source,
+			// Actual fee after evm execution, including tip.
+			actual_fee,
+			// Base fee.
+			executor.fee(base_fee),
+			// Fee initially withdrawn.
+			fee,
+		);
+		T::OnChargeTransaction::pay_priority_fee(actual_priority_fee);
 
 		let state = executor.into_state();
 

frame/evm/src/tests.rs

@@ -127,7 +127,7 @@ fn fee_deduction() {
 		assert_eq!(Balances::free_balance(&substrate_addr), 90);
 
 		// Refund fees as 5 units
-		<<Test as Config>::OnChargeTransaction as OnChargeEVMTransaction<Test>>::correct_and_deposit_fee(&evm_addr, U256::from(5), imbalance);
+		<<Test as Config>::OnChargeTransaction as OnChargeEVMTransaction<Test>>::correct_and_deposit_fee(&evm_addr, U256::from(5), U256::from(5), imbalance);
 		assert_eq!(Balances::free_balance(&substrate_addr), 95);
 	});
 }
@@ -249,6 +249,30 @@ fn author_should_get_tip() {
 	});
 }
 
+#[test]
+fn issuance_after_tip() {
+	new_test_ext().execute_with(|| {
+		let before_tip = <Test as Config>::Currency::total_issuance();
+		let result = EVM::call(
+			Origin::root(),
+			H160::default(),
+			H160::from_str("1000000000000000000000000000000000000001").unwrap(),
+			Vec::new(),
+			U256::from(1),
+			1000000,
+			U256::from(2_000_000_000),
+			Some(U256::from(1)),
+			None,
+			Vec::new(),
+		);
+		result.expect("EVM can be called");
+		let after_tip = <Test as Config>::Currency::total_issuance();
+		// Only base fee is burned
+		let (base_fee, _) = <Test as Config>::FeeCalculator::min_gas_price();
+		assert_eq!(after_tip, (before_tip - (base_fee.low_u64() * 21_000)));
+	});
+}
+
 #[test]
 fn author_same_balance_without_tip() {
 	new_test_ext().execute_with(|| {