Patch substrate bug for refund on 0 ED chains (polkadot-evm#509) (#22)

* Patch substrate bug for refund on 0 ED chains

* Add integration test

* is_zero for readibility

(cherry picked from commit 7be5145)

frame/evm/src/lib.rs

@@ -76,7 +76,7 @@ use frame_support::{
 	dispatch::DispatchResultWithPostInfo,
 	traits::{
 		tokens::fungible::Inspect, Currency, ExistenceRequirement, FindAuthor, Get, Imbalance,
-		OnUnbalanced, WithdrawReasons,
+		OnUnbalanced, SignedImbalance, WithdrawReasons,
 	},
 	weights::{Pays, PostDispatchInfo, Weight},
 };
@@ -85,7 +85,7 @@ use frame_system::RawOrigin;
 use serde::{Deserialize, Serialize};
 use sp_core::{Hasher, H160, H256, U256};
 use sp_runtime::{
-	traits::{BadOrigin, Saturating, UniqueSaturatedInto},
+	traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero},
 	AccountId32,
 };
 use sp_std::vec::Vec;
@@ -709,6 +709,25 @@ where
 			// that case we don't refund anything.
 			let refund_imbalance = C::deposit_into_existing(&account_id, refund_amount)
 				.unwrap_or_else(|_| C::PositiveImbalance::zero());
+
+			// Make sure this works with 0 ExistentialDeposit
+			// https://github.com/paritytech/substrate/issues/10117
+			// If we tried to refund something, the account still empty and the ED is set to 0,
+			// we call `make_free_balance_be` with the refunded amount.
+			let refund_imbalance = if C::minimum_balance().is_zero()
+				&& refund_amount > C::Balance::zero()
+				&& C::total_balance(&account_id).is_zero()
+			{
+				// Known bug: Substrate tried to refund to a zeroed AccountData, but
+				// interpreted the account to not exist.
+				match C::make_free_balance_be(&account_id, refund_amount) {
+					SignedImbalance::Positive(p) => p,
+					_ => C::PositiveImbalance::zero(),
+				}
+			} else {
+				refund_imbalance
+			};
+
 			// merge the imbalance caused by paying the fees and refunding parts of it again.
 			let adjusted_paid = paid
 				.offset(refund_imbalance)

frame/evm/src/tests.rs

@@ -113,6 +113,66 @@ fn fee_deduction() {
 	});
 }
 
+#[test]
+fn ed_0_refund_patch_works() {
+	new_test_ext().execute_with(|| {
+		// Verifies that the OnChargeEVMTransaction patch is applied and fixes a known bug in Substrate for evm transactions.
+		// https://github.com/paritytech/substrate/issues/10117
+		let evm_addr = H160::from_str("1000000000000000000000000000000000000003").unwrap();
+		let substrate_addr = <Test as Config>::AddressMapping::into_account_id(evm_addr);
+
+		let _ = <Test as Config>::Currency::deposit_creating(&substrate_addr, 21777);
+		assert_eq!(Balances::free_balance(&substrate_addr), 21777);
+
+		let _ = EVM::call(
+			Origin::root(),
+			evm_addr,
+			H160::from_str("1000000000000000000000000000000000000001").unwrap(),
+			Vec::new(),
+			U256::from(1),
+			21776,
+			U256::from(1),
+			Some(U256::from(0)),
+		);
+		// All that was due, was refunded.
+		assert_eq!(Balances::free_balance(&substrate_addr), 776);
+	});
+}
+
+#[test]
+fn ed_0_refund_patch_is_required() {
+	new_test_ext().execute_with(|| {
+		// This test proves that the patch is required, verifying that the current Substrate behaviour is incorrect
+		// for ED 0 configured chains.
+		let evm_addr = H160::from_str("1000000000000000000000000000000000000003").unwrap();
+		let substrate_addr = <Test as Config>::AddressMapping::into_account_id(evm_addr);
+
+		let _ = <Test as Config>::Currency::deposit_creating(&substrate_addr, 100);
+		assert_eq!(Balances::free_balance(&substrate_addr), 100);
+
+		// Drain funds
+		let _ =
+			<<Test as Config>::OnChargeTransaction as OnChargeEVMTransaction<Test>>::withdraw_fee(
+				&evm_addr,
+				U256::from(100),
+			)
+			.unwrap();
+		assert_eq!(Balances::free_balance(&substrate_addr), 0);
+
+		// Try to refund. With ED 0, although the balance is now 0, the account still exists.
+		// So its expected that calling `deposit_into_existing` results in the AccountData to increase the Balance.
+		//
+		// Is not the case, and this proves that the refund logic needs to be handled taking this into account.
+		assert_eq!(
+			<Test as Config>::Currency::deposit_into_existing(&substrate_addr, 5u32.into())
+				.is_err(),
+			true
+		);
+		// Balance didn't change, and should be 5.
+		assert_eq!(Balances::free_balance(&substrate_addr), 0);
+	});
+}
+
 #[test]
 fn find_author() {
 	new_test_ext().execute_with(|| {