Fix transaction payment fee/tip unbalanceds (paritytech#8860)

* fix and test

* fmt

Author: gui1117

frame/transaction-payment/src/lib.rs

@@ -609,7 +609,7 @@ mod tests {
 			DispatchClass, DispatchInfo, PostDispatchInfo, GetDispatchInfo, Weight,
 			WeightToFeePolynomial, WeightToFeeCoefficients, WeightToFeeCoefficient,
 		},
-		traits::Currency,
+		traits::{Currency, OnUnbalanced, Imbalance},
 	};
 	use pallet_balances::Call as BalancesCall;
 	use sp_core::H256;
@@ -718,8 +718,27 @@ mod tests {
 		}
 	}
 
+	thread_local! {
+		static TIP_UNBALANCED_AMOUNT: RefCell<u64> = RefCell::new(0);
+		static FEE_UNBALANCED_AMOUNT: RefCell<u64> = RefCell::new(0);
+	}
+
+	pub struct DealWithFees;
+	impl OnUnbalanced<pallet_balances::NegativeImbalance<Runtime>> for DealWithFees {
+		fn on_unbalanceds<B>(
+			mut fees_then_tips: impl Iterator<Item=pallet_balances::NegativeImbalance<Runtime>>
+		) {
+			if let Some(fees) = fees_then_tips.next() {
+				FEE_UNBALANCED_AMOUNT.with(|a| *a.borrow_mut() += fees.peek());
+				if let Some(tips) = fees_then_tips.next() {
+					TIP_UNBALANCED_AMOUNT.with(|a| *a.borrow_mut() += tips.peek());
+				}
+			}
+		}
+	}
+
 	impl Config for Runtime {
-		type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
+		type OnChargeTransaction = CurrencyAdapter<Balances, DealWithFees>;
 		type TransactionByteFee = TransactionByteFee;
 		type WeightToFee = WeightToFee;
 		type FeeMultiplierUpdate = ();
@@ -832,6 +851,10 @@ mod tests {
 					::post_dispatch(pre, &info_from_weight(5), &default_post_info(), len, &Ok(()))
 			);
 			assert_eq!(Balances::free_balance(1), 100 - 5 - 5 - 10);
+			assert_eq!(FEE_UNBALANCED_AMOUNT.with(|a| a.borrow().clone()), 5 + 5 + 10);
+			assert_eq!(TIP_UNBALANCED_AMOUNT.with(|a| a.borrow().clone()), 0);
+
+			FEE_UNBALANCED_AMOUNT.with(|a| *a.borrow_mut() = 0);
 
 			let pre = ChargeTransactionPayment::<Runtime>::from(5 /* tipped */)
 				.pre_dispatch(&2, CALL, &info_from_weight(100), len)
@@ -843,6 +866,8 @@ mod tests {
 					::post_dispatch(pre, &info_from_weight(100), &post_info_from_weight(50), len, &Ok(()))
 			);
 			assert_eq!(Balances::free_balance(2), 200 - 5 - 10 - 50 - 5);
+			assert_eq!(FEE_UNBALANCED_AMOUNT.with(|a| a.borrow().clone()), 5 + 10 + 50);
+			assert_eq!(TIP_UNBALANCED_AMOUNT.with(|a| a.borrow().clone()), 5);
 		});
 	}
 

frame/transaction-payment/src/payment.rs

@@ -50,9 +50,15 @@ pub trait OnChargeTransaction<T: Config> {
 /// Implements the transaction payment for a module implementing the `Currency`
 /// trait (eg. the pallet_balances) using an unbalance handler (implementing
 /// `OnUnbalanced`).
+///
+/// The unbalance handler is given 2 unbalanceds in [`OnUnbalanced::on_unbalanceds`]: fee and
+/// then tip.
 pub struct CurrencyAdapter<C, OU>(PhantomData<(C, OU)>);
 
 /// Default implementation for a Currency and an OnUnbalanced handler.
+///
+/// The unbalance handler is given 2 unbalanceds in [`OnUnbalanced::on_unbalanceds`]: fee and
+/// then tip.
 impl<T, C, OU> OnChargeTransaction<T> for CurrencyAdapter<C, OU>
 where
 	T: Config,
@@ -97,7 +103,7 @@ where
 	/// Since the predicted fee might have been too high, parts of the fee may
 	/// be refunded.
 	///
-	/// Note: The `fee` already includes the `tip`.
+	/// Note: The `corrected_fee` already includes the `tip`.
 	fn correct_and_deposit_fee(
 		who: &T::AccountId,
 		_dispatch_info: &DispatchInfoOf<T::Call>,
@@ -120,8 +126,8 @@ where
 				.same()
 				.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?;
 			// Call someone else to handle the imbalance (fee and tip separately)
-			let imbalances = adjusted_paid.split(tip);
-			OU::on_unbalanceds(Some(imbalances.0).into_iter().chain(Some(imbalances.1)));
+			let (tip, fee) = adjusted_paid.split(tip);
+			OU::on_unbalanceds(Some(fee).into_iter().chain(Some(tip)));
 		}
 		Ok(())
 	}