fix: incremented nonce on out of fund failure (#671)

engine-tests/src/tests/sanity.rs

@@ -150,19 +150,19 @@ fn test_state_format() {
     assert_eq!(hex::encode(state.try_to_vec().unwrap()), expected_hex);
 }
 
+fn generate_code(len: usize) -> Vec<u8> {
+    let mut rng = rand::thread_rng();
+    let mut buf = vec![0u8; len];
+    rng.fill_bytes(&mut buf);
+    buf
+}
+
 #[test]
 fn test_deploy_contract() {
     let (mut runner, mut signer, _) = initialize_transfer();
 
     // Randomly generate some "contract code"
-    const LEN: usize = 567;
-    let code: Vec<u8> = {
-        let mut rng = rand::thread_rng();
-        let mut buf = vec![0u8; LEN];
-        rng.fill_bytes(&mut buf);
-        buf
-    };
-
+    let code = generate_code(567);
     // Deploy that code
     let result = runner
         .submit_with_signer(&mut signer, |nonce| {
@@ -183,12 +183,7 @@ fn test_deploy_largest_contract() {
     let (mut runner, mut signer, _) = initialize_transfer();
 
     let len = evm::Config::berlin().create_contract_limit.unwrap();
-    let code: Vec<u8> = {
-        let mut rng = rand::thread_rng();
-        let mut buf = vec![0u8; len];
-        rng.fill_bytes(&mut buf);
-        buf
-    };
+    let code = generate_code(len);
 
     // Deploy that code
     let (result, profile) = runner
@@ -352,9 +347,7 @@ fn test_solidity_pure_bench() {
     );
 
     // Pure rust version of the same contract
-    let base_path = std::path::Path::new("../etc")
-        .join("tests")
-        .join("benchmark-contract");
+    let base_path = Path::new("../etc").join("tests").join("benchmark-contract");
     let output_path =
         base_path.join("target/wasm32-unknown-unknown/release/benchmark_contract.wasm");
     test_utils::rust::compile(base_path);
@@ -404,7 +397,7 @@ fn test_revert_during_contract_deploy() {
         .submit_transaction(&signer.secret_key, deploy_tx)
         .unwrap();
 
-    let revert_bytes = crate::test_utils::unwrap_revert(submit_result);
+    let revert_bytes = test_utils::unwrap_revert(submit_result);
     // First 4 bytes is a function selector with signature `Error(string)`
     assert_eq!(&revert_bytes[0..4], &[8, 195, 121, 160]);
     // Remaining data is an ABI-encoded string
@@ -562,9 +555,7 @@ fn test_override_state() {
             ))
             .unwrap();
         match result {
-            crate::prelude::parameters::TransactionStatus::Succeed(bytes) => {
-                Address::try_from_slice(&bytes[12..32]).unwrap()
-            }
+            TransactionStatus::Succeed(bytes) => Address::try_from_slice(&bytes[12..32]).unwrap(),
             _ => panic!("tx failed"),
         }
     };
@@ -808,6 +799,8 @@ fn test_transfer_charging_gas_success() {
 
 #[test]
 fn test_eth_transfer_charging_gas_not_enough_balance() {
+    use near_vm_errors::{FunctionCallError, HostError, VMError};
+
     let (mut runner, mut source_account, dest_address) = initialize_transfer();
     let source_address = test_utils::address_from_secret_key(&source_account.secret_key);
     let transaction = |nonce| {
@@ -829,10 +822,13 @@ fn test_eth_transfer_charging_gas_not_enough_balance() {
     test_utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
 
     // attempt transfer
-    let result = runner
+    let error = runner
         .submit_with_signer(&mut source_account, transaction)
-        .unwrap();
-    assert_eq!(result.status, TransactionStatus::OutOfFund);
+        .unwrap_err();
+    assert!(matches!(error, VMError::FunctionCallError(
+        FunctionCallError::HostError(
+            HostError::GuestPanic { panic_msg })) if panic_msg == "ERR_OUT_OF_FUND"
+    ));
 
     // validate post-state
     let relayer = sdk::types::near_account_to_evm_address(
@@ -843,8 +839,8 @@ fn test_eth_transfer_charging_gas_not_enough_balance() {
         &runner,
         source_address,
         INITIAL_BALANCE,
-        // nonce is still incremented since the transaction was otherwise valid
-        (INITIAL_NONCE + 1).into(),
+        // nonce is still not incremented since the transaction was invalid
+        INITIAL_NONCE.into(),
     );
     test_utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
     test_utils::validate_address_balance_and_nonce(&runner, relayer, Wei::zero(), 0.into());
@@ -996,6 +992,8 @@ fn test_eth_transfer_insufficient_balance_sim() {
 // Same as `test_eth_transfer_charging_gas_not_enough_balance` but run through `near-sdk-sim`.
 #[test]
 fn test_eth_transfer_charging_gas_not_enough_balance_sim() {
+    use near_primitives::{errors::TxExecutionError, transaction::ExecutionStatus};
+
     let (aurora, mut signer, address) = initialize_evm_sim();
 
     // Run transaction which will fail (not enough balance to cover gas)
@@ -1009,13 +1007,17 @@ fn test_eth_transfer_charging_gas_not_enough_balance_sim() {
         &signer.secret_key,
     );
     let call_result = aurora.call("submit", rlp::encode(&signed_tx).as_ref());
-    let result: SubmitResult = call_result.unwrap_borsh();
-    assert_eq!(result.status, TransactionStatus::OutOfFund);
+    let outcome = call_result.outcome();
+    assert!(matches!(
+    &outcome.status,
+    ExecutionStatus::Failure(
+        TxExecutionError::ActionError(e)) if e.to_string().contains("ERR_OUT_OF_FUND")
+    ));
 
     // validate post-state
     assert_eq!(
         query_address_sim(&address, "get_nonce", &aurora),
-        U256::from(INITIAL_NONCE + 1),
+        INITIAL_NONCE.into(), // nonce hasn't been changed because an error occurs
     );
     assert_eq!(
         query_address_sim(&address, "get_balance", &aurora),

engine/src/engine.rs

@@ -111,13 +111,6 @@ pub enum EngineErrorKind {
 }
 
 impl EngineErrorKind {
-    pub fn with_gas_used(self, gas_used: u64) -> EngineError {
-        EngineError {
-            kind: self,
-            gas_used,
-        }
-    }
-
     pub fn as_bytes(&self) -> &[u8] {
         use EngineErrorKind::*;
         match self {
@@ -490,13 +483,7 @@ impl<'env, I: IO + Copy, E: Env> Engine<'env, I, E> {
         };
 
         let used_gas = executor.used_gas();
-        let status = match exit_reason.into_result(result) {
-            Ok(status) => status,
-            Err(e) => {
-                increment_nonce(&mut self.io, &origin);
-                return Err(e.with_gas_used(used_gas));
-            }
-        };
+        let status = exit_reason.into_result(result)?;
 
         let (values, logs) = executor.into_state().deconstruct();
         let logs = filter_promises_from_logs(&self.io, handler, logs, &self.current_account_id);
@@ -571,13 +558,7 @@ impl<'env, I: IO + Copy, E: Env> Engine<'env, I, E> {
         );
 
         let used_gas = executor.used_gas();
-        let status = match exit_reason.into_result(result) {
-            Ok(status) => status,
-            Err(e) => {
-                increment_nonce(&mut self.io, origin);
-                return Err(e.with_gas_used(used_gas));
-            }
-        };
+        let status = exit_reason.into_result(result)?;
 
         let (values, logs) = executor.into_state().deconstruct();
         let logs = filter_promises_from_logs(&self.io, handler, logs, &self.current_account_id);
@@ -903,11 +884,6 @@ pub fn submit<I: IO + Copy, E: Env, P: PromiseHandler>(
     let mut engine = Engine::new_with_state(state, sender, current_account_id, io, env);
     let prepaid_amount = match engine.charge_gas(&sender, &transaction) {
         Ok(gas_result) => gas_result,
-        Err(GasPaymentError::OutOfFund) => {
-            increment_nonce(&mut io, &sender);
-            let result = SubmitResult::new(TransactionStatus::OutOfFund, 0, vec![]);
-            return Ok(result);
-        }
         Err(err) => {
             return Err(EngineErrorKind::GasPayment(err).into());
         }
@@ -1208,6 +1184,7 @@ pub fn get_nonce<I: IO>(io: &I, address: &Address) -> U256 {
         .unwrap_or_else(|_| U256::zero())
 }
 
+#[cfg(test)]
 pub fn increment_nonce<I: IO>(io: &mut I, address: &Address) {
     let account_nonce = get_nonce(io, address);
     let new_nonce = account_nonce.saturating_add(U256::one());