Fix(engine): handle transaction gas limit properly

src/engine.rs

@@ -145,7 +145,7 @@ pub struct Engine {
 }
 
 // TODO: upgrade to Berlin HF
-const CONFIG: &Config = &Config::istanbul();
+pub(crate) const CONFIG: &Config = &Config::istanbul();
 
 /// Key for storing the state of the engine.
 const STATE_KEY: &[u8; 5] = b"STATE";
@@ -294,23 +294,27 @@ impl Engine {
     pub fn deploy_code_with_input(&mut self, input: Vec<u8>) -> EngineResult<SubmitResult> {
         let origin = self.origin();
         let value = Wei::zero();
-        self.deploy_code(origin, value, input)
+        self.deploy_code(origin, value, input, u64::MAX)
     }
 
     pub fn deploy_code(
         &mut self,
         origin: Address,
         value: Wei,
         input: Vec<u8>,
+        gas_limit: u64,
     ) -> EngineResult<SubmitResult> {
-        let mut executor = self.make_executor();
+        let mut executor = self.make_executor(gas_limit);
         let address = executor.create_address(CreateScheme::Legacy { caller: origin });
         let (status, result) = (
-            executor.transact_create(origin, value.raw(), input, u64::MAX),
+            executor.transact_create(origin, value.raw(), input, gas_limit),
             address,
         );
         let is_succeed = status.is_succeed();
-        status.into_result()?;
+        if let Err(e) = status.into_result() {
+            Engine::increment_nonce(&origin);
+            return Err(e);
+        }
         let used_gas = executor.used_gas();
         let (values, logs) = executor.into_state().deconstruct();
         self.apply(values, Vec::<Log>::new(), true);
@@ -327,7 +331,7 @@ impl Engine {
         let origin = self.origin();
         let contract = Address(args.contract);
         let value = Wei::zero();
-        self.call(origin, contract, value, args.input)
+        self.call(origin, contract, value, args.input, u64::MAX)
     }
 
     pub fn call(
@@ -336,10 +340,11 @@ impl Engine {
         contract: Address,
         value: Wei,
         input: Vec<u8>,
+        gas_limit: u64,
     ) -> EngineResult<SubmitResult> {
-        let mut executor = self.make_executor();
+        let mut executor = self.make_executor(gas_limit);
         let (status, result) =
-            executor.transact_call(origin, contract, value.raw(), input, u64::MAX);
+            executor.transact_call(origin, contract, value.raw(), input, gas_limit);
 
         let used_gas = executor.used_gas();
         let (values, logs) = executor.into_state().deconstruct();
@@ -372,7 +377,7 @@ impl Engine {
         let origin = Address::from_slice(&args.sender);
         let contract = Address::from_slice(&args.address);
         let value = U256::from_big_endian(&args.amount);
-        self.view(origin, contract, Wei::new(value), args.input)
+        self.view(origin, contract, Wei::new(value), args.input, u64::MAX)
     }
 
     pub fn view(
@@ -381,16 +386,17 @@ impl Engine {
         contract: Address,
         value: Wei,
         input: Vec<u8>,
+        gas_limit: u64,
     ) -> EngineResult<Vec<u8>> {
-        let mut executor = self.make_executor();
+        let mut executor = self.make_executor(gas_limit);
         let (status, result) =
-            executor.transact_call(origin, contract, value.raw(), input, u64::MAX);
+            executor.transact_call(origin, contract, value.raw(), input, gas_limit);
         status.into_result()?;
         Ok(result)
     }
 
-    fn make_executor(&self) -> StackExecutor<MemoryStackState<Engine>> {
-        let metadata = StackSubstateMetadata::new(u64::MAX, &CONFIG);
+    fn make_executor(&self, gas_limit: u64) -> StackExecutor<MemoryStackState<Engine>> {
+        let metadata = StackSubstateMetadata::new(gas_limit, &CONFIG);
         let state = MemoryStackState::new(metadata, self);
         StackExecutor::new_with_precompile(state, &CONFIG, precompiles::istanbul_precompiles)
     }

src/lib.rs

@@ -96,6 +96,7 @@ mod contract {
 
     const CODE_KEY: &[u8; 4] = b"CODE";
     const CODE_STAGE_KEY: &[u8; 10] = b"CODE_STAGE";
+    const GAS_OVERFLOW: &str = "ERR_GAS_OVERFLOW";
 
     ///
     /// ADMINISTRATIVE METHODS
@@ -239,16 +240,33 @@ mod contract {
 
         Engine::check_nonce(&sender, &signed_transaction.transaction.nonce).sdk_unwrap();
 
+        // Check intrinsic gas is covered by transaction gas limit
+        match signed_transaction
+            .transaction
+            .intrinsic_gas(&crate::engine::CONFIG)
+        {
+            None => sdk::panic_utf8(GAS_OVERFLOW.as_bytes()),
+            Some(intrinsic_gas) => {
+                if signed_transaction.transaction.gas < intrinsic_gas.into() {
+                    sdk::panic_utf8(b"ERR_INTRINSIC_GAS")
+                }
+            }
+        }
+
         // Figure out what kind of a transaction this is, and execute it:
         let mut engine = Engine::new_with_state(state, sender);
         let value = signed_transaction.transaction.value;
+        let gas_limit = signed_transaction
+            .transaction
+            .get_gas_limit()
+            .sdk_expect(GAS_OVERFLOW);
         let data = signed_transaction.transaction.data;
         let result = if let Some(receiver) = signed_transaction.transaction.to {
-            Engine::call(&mut engine, sender, receiver, value, data)
+            Engine::call(&mut engine, sender, receiver, value, data, gas_limit)
             // TODO: charge for storage
         } else {
             // Execute a contract deployment:
-            Engine::deploy_code(&mut engine, sender, value, data)
+            Engine::deploy_code(&mut engine, sender, value, data, gas_limit)
             // TODO: charge for storage
         };
         result
@@ -276,6 +294,7 @@ mod contract {
             meta_call_args.contract_address,
             meta_call_args.value,
             meta_call_args.input,
+            u64::MAX, // TODO: is there a gas limit with meta calls?
         );
         result
             .map(|res| res.try_to_vec().sdk_expect("ERR_SERIALIZE"))

src/test_utils/erc20.rs

@@ -43,7 +43,7 @@ impl ERC20Constructor {
         EthTransaction {
             nonce,
             gas_price: Default::default(),
-            gas: Default::default(),
+            gas: u64::MAX.into(),
             to: None,
             value: Default::default(),
             data,
@@ -86,7 +86,7 @@ impl ERC20 {
         EthTransaction {
             nonce,
             gas_price: Default::default(),
-            gas: Default::default(),
+            gas: u64::MAX.into(),
             to: Some(self.0.address),
             value: Default::default(),
             data,
@@ -107,7 +107,7 @@ impl ERC20 {
         EthTransaction {
             nonce,
             gas_price: Default::default(),
-            gas: Default::default(),
+            gas: u64::MAX.into(),
             to: Some(self.0.address),
             value: Default::default(),
             data,
@@ -125,7 +125,7 @@ impl ERC20 {
         EthTransaction {
             nonce,
             gas_price: Default::default(),
-            gas: Default::default(),
+            gas: u64::MAX.into(),
             to: Some(self.0.address),
             value: Default::default(),
             data,

src/test_utils/mod.rs

@@ -299,7 +299,7 @@ pub(crate) fn create_eth_transaction(
     let tx = EthTransaction {
         nonce: Default::default(),
         gas_price: Default::default(),
-        gas: Default::default(),
+        gas: u64::MAX.into(),
         to,
         value,
         data,

src/test_utils/standard_precompiles.rs

@@ -34,7 +34,7 @@ impl PrecompilesConstructor {
         EthTransaction {
             nonce,
             gas_price: Default::default(),
-            gas: Default::default(),
+            gas: u64::MAX.into(),
             to: None,
             value: Default::default(),
             data,
@@ -62,7 +62,7 @@ impl PrecompilesContract {
         EthTransaction {
             nonce,
             gas_price: Default::default(),
-            gas: Default::default(),
+            gas: u64::MAX.into(),
             to: Some(self.0.address),
             value: Default::default(),
             data,

src/tests/erc20.rs

@@ -46,6 +46,49 @@ fn erc20_mint() {
     );
 }
 
+#[test]
+fn erc20_mint_out_of_gas() {
+    let (mut runner, source_account, dest_address, contract) = initialize_erc20();
+
+    // Validate pre-state
+    assert_eq!(
+        U256::zero(),
+        get_address_erc20_balance(
+            &mut runner,
+            &source_account,
+            (INITIAL_NONCE + 1).into(),
+            dest_address,
+            &contract
+        )
+    );
+
+    // Try mint transaction
+    let mint_amount: u64 = rand::random();
+    let mut mint_tx = contract.mint(dest_address, mint_amount.into(), (INITIAL_NONCE + 2).into());
+
+    // not enough gas to cover intrinsic cost
+    mint_tx.gas = (mint_tx.intrinsic_gas(&evm::Config::istanbul()).unwrap() - 1).into();
+    let outcome = runner.submit_transaction(&source_account, mint_tx.clone());
+    let error = outcome.unwrap_err();
+    let error_message = format!("{:?}", error);
+    assert!(error_message.contains("ERR_INTRINSIC_GAS"));
+
+    // not enough gas to complete transaction
+    mint_tx.gas = U256::from(67_000);
+    let outcome = runner.submit_transaction(&source_account, mint_tx);
+    let error = outcome.unwrap_err();
+    let error_message = format!("{:?}", error);
+    assert!(error_message.contains("ERR_OUT_OF_GAS"));
+
+    // Validate post-state
+    test_utils::validate_address_balance_and_nonce(
+        &runner,
+        test_utils::address_from_secret_key(&source_account),
+        Wei::new_u64(INITIAL_BALANCE),
+        (INITIAL_NONCE + 3).into(),
+    );
+}
+
 #[test]
 fn erc20_transfer_success() {
     let (mut runner, source_account, dest_address, contract) = initialize_erc20();
@@ -186,6 +229,48 @@ fn erc20_transfer_insufficient_balance() {
     );
 }
 
+#[test]
+fn deploy_erc_20_out_of_gas() {
+    let mut runner = test_utils::deploy_evm();
+    let mut rng = rand::thread_rng();
+    let source_account = SecretKey::random(&mut rng);
+    let source_address = test_utils::address_from_secret_key(&source_account);
+    runner.create_address(
+        source_address,
+        Wei::new_u64(INITIAL_BALANCE),
+        INITIAL_NONCE.into(),
+    );
+
+    let constructor = ERC20Constructor::load();
+    let mut deploy_transaction = constructor.deploy("OutOfGas", "OOG", INITIAL_NONCE.into());
+
+    // not enough gas to cover intrinsic cost
+    deploy_transaction.gas = (deploy_transaction
+        .intrinsic_gas(&evm::Config::istanbul())
+        .unwrap()
+        - 1)
+    .into();
+    let outcome = runner.submit_transaction(&source_account, deploy_transaction.clone());
+    let error = outcome.unwrap_err();
+    let error_message = format!("{:?}", error);
+    assert!(error_message.contains("ERR_INTRINSIC_GAS"));
+
+    // not enough gas to complete transaction
+    deploy_transaction.gas = U256::from(3_200_000);
+    let outcome = runner.submit_transaction(&source_account, deploy_transaction);
+    let error = outcome.unwrap_err();
+    let error_message = format!("{:?}", error);
+    assert!(error_message.contains("ERR_OUT_OF_GAS"));
+
+    // Validate post-state
+    test_utils::validate_address_balance_and_nonce(
+        &runner,
+        test_utils::address_from_secret_key(&source_account),
+        Wei::new_u64(INITIAL_BALANCE),
+        (INITIAL_NONCE + 1).into(),
+    );
+}
+
 fn get_address_erc20_balance(
     runner: &mut test_utils::AuroraRunner,
     signing_account: &SecretKey,

src/tests/sanity.rs

@@ -135,6 +135,47 @@ fn test_eth_transfer_incorrect_nonce() {
     test_utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
 }
 
+#[test]
+fn test_eth_transfer_not_enough_gas() {
+    let (mut runner, source_account, dest_address) = initialize_transfer();
+    let source_address = test_utils::address_from_secret_key(&source_account);
+    let transaction = EthTransaction {
+        nonce: INITIAL_NONCE.into(),
+        gas_price: Default::default(),
+        gas: 10_000.into(), // this is not enough gas
+        to: Some(dest_address),
+        value: TRANSFER_AMOUNT.into(),
+        data: vec![],
+    };
+    let transaction =
+        test_utils::sign_transaction(transaction, Some(runner.chain_id), &source_account);
+    let input = rlp::encode(&transaction).to_vec();
+    let calling_account_id = "some-account.near".to_string();
+
+    // validate pre-state
+    test_utils::validate_address_balance_and_nonce(
+        &runner,
+        source_address,
+        INITIAL_BALANCE,
+        INITIAL_NONCE.into(),
+    );
+    test_utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
+
+    // attempt transfer
+    let (_, maybe_err) = runner.call(test_utils::SUBMIT, calling_account_id, input);
+    let error_message = format!("{:?}", maybe_err);
+    assert!(error_message.contains("ERR_INTRINSIC_GAS"));
+
+    // validate post-state (which is the same as pre-state in this case)
+    test_utils::validate_address_balance_and_nonce(
+        &runner,
+        source_address,
+        INITIAL_BALANCE,
+        INITIAL_NONCE.into(),
+    );
+    test_utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
+}
+
 fn initialize_transfer() -> (test_utils::AuroraRunner, SecretKey, Address) {
     // set up Aurora runner and accounts
     let mut runner = test_utils::deploy_evm();