Uncouple emptying gas_meter from updating the gas spent for the block (#68)

* Uncouple emptying gas_meter from updating the gas spent for the block.

Author: Alex Sed

frame/contracts/src/gas.rs

@@ -14,14 +14,13 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate. If not, see <http://www.gnu.org/licenses/>.
 
-use crate::{GasSpent, Module, Trait, BalanceOf};
+use crate::{Module, Trait, BalanceOf};
 use sp_std::convert::TryFrom;
 use sp_runtime::traits::{
 	CheckedMul, Zero, SaturatedConversion, AtLeast32Bit, UniqueSaturatedInto,
 };
 use frame_support::{
-	traits::{Currency, ExistenceRequirement, OnUnbalanced, WithdrawReason}, StorageValue,
-	dispatch::DispatchError,
+	traits::{Currency, ExistenceRequirement, OnUnbalanced, WithdrawReason},	dispatch::DispatchError,
 };
 
 #[cfg(test)]
@@ -257,14 +256,8 @@ pub fn refund_unused_gas<T: Trait>(
 	transactor: &T::AccountId,
 	gas_meter: GasMeter<T>,
 ) {
-	let gas_spent = gas_meter.spent();
 	let gas_left = gas_meter.gas_left();
 
-	// Increase total spent gas.
-	// This cannot overflow, since `gas_spent` is never greater than `block_gas_limit`, which
-	// also has Gas type.
-	GasSpent::mutate(|block_gas_spent| *block_gas_spent += gas_spent);
-
 	// Refund gas left by the price it was bought at.
 	let refund = gas_meter.gas_price * gas_left.unique_saturated_into();
 	let _imbalance = T::Currency::deposit_creating(transactor, refund);

frame/contracts/src/gas_tests.rs

@@ -262,6 +262,24 @@ impl GasHandler<GasTest> for TestGasHandler {
     }
 }
 
+pub struct NoChargeGasHandler;
+impl GasHandler<GasTest> for NoChargeGasHandler {
+    fn fill_gas(
+        _transactor: &<GasTest as system::Trait>::AccountId,
+        gas_limit: Gas,
+    ) -> Result<GasMeter<GasTest>, DispatchError> {
+        // fills the gas meter without charging the user
+        Ok(GasMeter::with_limit(gas_limit, 1))
+    }
+
+    fn empty_unused_gas(
+        transactor: &<GasTest as system::Trait>::AccountId,
+        gas_meter: GasMeter<GasTest>,
+    ) {
+        // Do not charge the transactor. Give gas for free.
+    }
+}
+
 #[test]
 // Tests that the user is not charged when filling up gas meters
 fn customized_fill_gas_does_not_charge_the_user() {
@@ -273,9 +291,11 @@ fn customized_fill_gas_does_not_charge_the_user() {
             // Create test account
             Balances::deposit_creating(&ALICE, 1000);
 
-            // fill gas
-            let gas_meter_result = TestGasHandler::fill_gas(&ALICE, 500);
-            assert!(gas_meter_result.is_ok());
+            let gas_limit = 500;
+            let mut gas_meter = NoChargeGasHandler::fill_gas(&ALICE, gas_limit).unwrap();
+            // Charge as if the whole gas_limit is used
+            gas_meter.charge(&(), SimpleToken(gas_limit));
+            NoChargeGasHandler::empty_unused_gas(&ALICE, gas_meter);
 
             // Check the user is not charged
             assert_eq!(Balances::free_balance(&ALICE), 1000);

frame/contracts/src/lib.rs

@@ -588,6 +588,11 @@ decl_module! {
 			if let Ok(code_hash) = result {
 				Self::deposit_event(RawEvent::CodeStored(code_hash));
 			}
+			// Increase total spent gas.
+			// This cannot overflow, since `gas_spent` is never greater than `block_gas_limit`, which
+			// also has Gas type.
+			GasSpent::mutate(|block_gas_spent| *block_gas_spent += gas_meter.spent());
+
 			T::GasHandler::empty_unused_gas(&origin, gas_meter);
 
 			result.map(|_| ()).map_err(Into::into)
@@ -752,6 +757,11 @@ impl<T: Trait> Module<T> {
 			DirectAccountDb.commit(ctx.overlay.into_change_set());
 		}
 
+		// Increase total spent gas.
+		// This cannot overflow, since `gas_spent` is never greater than `block_gas_limit`, which
+		// also has Gas type.
+		GasSpent::mutate(|block_gas_spent| *block_gas_spent += gas_meter.spent());
+
 		// Handle unused gas of the gas meter. Default behaviour is to refund cost of the unused gas.
 		//
 		// NOTE: This should go after the commit to the storage, since the storage changes