[movevm] improve automatic creation of account for sponsored txn

* If an account is created during sponsored transaction and the txn aborts.
* The transaction is now kept but aborted.
* The state associated with account creation is wiped and The account is no longer created.
* During epilogue, we then hit a invariant violation, because the sequence number for the account cannot be incremented.

This ensures that we create an account even on transaction failure in the epilogue:

* during validation, sponsored transactions on sequence number 0, must
  have sufficient gas to create an account
* if the transaction aborts, call create account and charge gas appropriately

aptos-move/aptos-vm/src/aptos_vm.rs

@@ -77,8 +77,8 @@ use move_core_types::{
     value::{serialize_values, MoveValue},
     vm_status::StatusType,
 };
-use move_vm_runtime::session::SerializedReturnValues;
-use move_vm_types::gas::UnmeteredGasMeter;
+use move_vm_runtime::{logging::expect_no_verification_errors, session::SerializedReturnValues};
+use move_vm_types::gas::{GasMeter, UnmeteredGasMeter};
 use num_cpus;
 use once_cell::sync::{Lazy, OnceCell};
 use std::{
@@ -241,7 +241,7 @@ impl AptosVM {
     pub fn failed_transaction_cleanup(
         &self,
         error_code: VMStatus,
-        gas_meter: &impl AptosGasMeter,
+        gas_meter: &mut impl AptosGasMeter,
         txn_data: &TransactionMetadata,
         resolver: &impl AptosMoveResolver,
         log_context: &AdapterLogSchema,
@@ -303,12 +303,19 @@ impl AptosVM {
     fn failed_transaction_cleanup_and_keep_vm_status(
         &self,
         error_code: VMStatus,
-        gas_meter: &impl AptosGasMeter,
+        gas_meter: &mut impl AptosGasMeter,
         txn_data: &TransactionMetadata,
         resolver: &impl AptosMoveResolver,
         log_context: &AdapterLogSchema,
         change_set_configs: &ChangeSetConfigs,
     ) -> (VMStatus, VMOutput) {
+        let transaction_status = TransactionStatus::from_vm_status(
+            error_code.clone(),
+            self.vm_impl
+                .get_features()
+                .is_enabled(FeatureFlag::CHARGE_INVARIANT_VIOLATION),
+        );
+
         if self.vm_impl.get_gas_feature_version() >= 12 {
             // Check if the gas meter's internal counters are consistent.
             //
@@ -335,14 +342,44 @@ impl AptosVM {
         let mut session = self
             .vm_impl
             .new_session(resolver, SessionId::epilogue_meta(txn_data));
-        let fee_statement = AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, 0);
 
-        match TransactionStatus::from_vm_status(
-            error_code.clone(),
-            self.vm_impl
+        if matches!(transaction_status, TransactionStatus::Keep(_))
+            && txn_data.fee_payer().is_some()
+            && txn_data.sequence_number == 0
+            && self
+                .vm_impl
                 .get_features()
-                .is_enabled(FeatureFlag::CHARGE_INVARIANT_VIOLATION),
+                .is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION)
+        {
+            if let Err(err) =
+                create_account_if_does_not_exist(&mut session, gas_meter, txn_data.sender())
+                    .map_err(expect_no_verification_errors)
+                    .or_else(|err| {
+                        expect_only_successful_execution(
+                            err,
+                            &format!("{:?}::{}", ACCOUNT_MODULE, CREATE_ACCOUNT_IF_DOES_NOT_EXIST),
+                            log_context,
+                        )
+                    })
+            {
+                return discard_error_vm_status(err);
+            }
+        }
+
+        let mut respawned_session = match self.charge_change_set_and_respawn_session(
+            session,
+            resolver,
+            gas_meter,
+            change_set_configs,
+            txn_data,
         ) {
+            Ok(respawned_session) => respawned_session,
+            Err(err) => return discard_error_vm_status(err),
+        };
+
+        let fee_statement = AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, 0);
+
+        match transaction_status {
             TransactionStatus::Keep(status) => {
                 // Inject abort info if available.
                 let status = match status {
@@ -366,18 +403,25 @@ impl AptosVM {
                 // so even if the previous failure occurred while running the epilogue, it
                 // should not fail now. If it somehow fails here, there is no choice but to
                 // discard the transaction.
-                if let Err(e) = self.vm_impl.run_failure_epilogue(
-                    &mut session,
-                    gas_meter.balance(),
-                    fee_statement,
-                    txn_data,
-                    log_context,
-                ) {
+                if let Err(e) = respawned_session.execute(|session| {
+                    self.vm_impl.run_failure_epilogue(
+                        session,
+                        gas_meter.balance(),
+                        fee_statement,
+                        txn_data,
+                        log_context,
+                    )
+                }) {
                     return discard_error_vm_status(e);
                 }
-                let txn_output =
-                    get_transaction_output(session, fee_statement, status, change_set_configs)
-                        .unwrap_or_else(|e| discard_error_vm_status(e).1);
+
+                let txn_output = match respawned_session.finish(change_set_configs) {
+                    Ok(change_set) => {
+                        VMOutput::new(change_set, fee_statement, TransactionStatus::Keep(status))
+                    },
+                    Err(err) => discard_error_vm_status(err).1,
+                };
+
                 (error_code, txn_output)
             },
             TransactionStatus::Discard(status) => {
@@ -1306,25 +1350,18 @@ impl AptosVM {
                 .new_session(resolver, SessionId::txn_meta(&txn_data));
         }
 
-        if let aptos_types::transaction::authenticator::TransactionAuthenticator::FeePayer {
-            ..
-        } = &txn.authenticator_ref()
-        {
-            if self
+        let txn_data = TransactionMetadata::new(txn);
+        if txn_data.fee_payer().is_some()
+            && self
                 .vm_impl
                 .get_features()
                 .is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION)
+        {
+            if let Err(err) =
+                create_account_if_does_not_exist(&mut session, gas_meter, txn.sender())
             {
-                if let Err(err) = session.execute_function_bypass_visibility(
-                    &ACCOUNT_MODULE,
-                    CREATE_ACCOUNT_IF_DOES_NOT_EXIST,
-                    vec![],
-                    serialize_values(&vec![MoveValue::Address(txn.sender())]),
-                    gas_meter,
-                ) {
-                    return discard_error_vm_status(err.into());
-                };
-            }
+                return discard_error_vm_status(err.into());
+            };
         }
 
         let storage_gas_params =
@@ -2042,3 +2079,19 @@ impl AptosSimulationVM {
         (vm_status, txn_output)
     }
 }
+
+fn create_account_if_does_not_exist(
+    session: &mut SessionExt,
+    gas_meter: &mut impl GasMeter,
+    account: AccountAddress,
+) -> VMResult<()> {
+    session
+        .execute_function_bypass_visibility(
+            &ACCOUNT_MODULE,
+            CREATE_ACCOUNT_IF_DOES_NOT_EXIST,
+            vec![],
+            serialize_values(&vec![MoveValue::Address(account)]),
+            gas_meter,
+        )
+        .map(|_return_vals| ())
+}

aptos-move/aptos-vm/src/aptos_vm_impl.rs

@@ -337,6 +337,31 @@ impl AptosVMImpl {
                 None,
             ));
         }
+
+        let max_gas_amount: u64 = txn_data.max_gas_amount().into();
+        let sequence_number_min_gas_amount: u64 =
+            txn_gas_params.storage_fee_per_state_slot_create.into();
+
+        if txn_data.fee_payer.is_some()
+            && txn_data.sequence_number == 0
+            && self
+                .get_features()
+                .is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION)
+            && max_gas_amount < 2 * sequence_number_min_gas_amount
+        {
+            speculative_warn!(
+                log_context,
+                format!(
+                    "[VM] Gas unit error (sponsored transaction); min {}, submitted {}",
+                    intrinsic_gas,
+                    txn_data.max_gas_amount()
+                ),
+            );
+            return Err(VMStatus::error(
+                StatusCode::MAX_GAS_UNITS_BELOW_MIN_TRANSACTION_GAS_UNITS,
+                None,
+            ));
+        }
         Ok(())
     }
 

aptos-move/e2e-move-tests/src/tests/fee_payer.rs

@@ -1,7 +1,7 @@
 // Copyright © Aptos Foundation
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::{assert_success, MoveHarness};
+use crate::{assert_abort, assert_success, MoveHarness};
 use aptos_cached_packages::aptos_stdlib;
 use aptos_language_e2e_tests::{
     account::{Account, TransactionBuilder},
@@ -74,6 +74,76 @@ fn test_account_not_exist_with_fee_payer_create_account() {
     assert!(bob_start > bob_after);
 }
 
+#[test]
+fn test_account_not_exist_and_out_of_gas_with_fee_payer_create_account() {
+    // This should result in an aborted, yet committed transaction and an account created.
+    let mut h = MoveHarness::new_with_features(vec![FeatureFlag::GAS_PAYER_ENABLED], vec![]);
+
+    let alice = Account::new();
+    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
+
+    let alice_start =
+        h.read_resource::<CoinStoreResource>(alice.address(), CoinStoreResource::struct_tag());
+    assert!(alice_start.is_none());
+    let bob_start = h.read_aptos_balance(bob.address());
+
+    let payload = aptos_stdlib::aptos_coin_transfer(*alice.address(), 1);
+    let transaction = TransactionBuilder::new(alice.clone())
+        .fee_payer(bob.clone())
+        .payload(payload)
+        .sequence_number(0)
+        .max_gas_amount(99_999) // This is not enough to execute this transaction
+        .gas_unit_price(1)
+        .sign_fee_payer();
+
+    let output = h.run_raw(transaction);
+    assert!(transaction_status_eq(
+        output.status(),
+        &TransactionStatus::Discard(StatusCode::MAX_GAS_UNITS_BELOW_MIN_TRANSACTION_GAS_UNITS)
+    ));
+
+    let alice_after =
+        h.read_resource::<CoinStoreResource>(alice.address(), CoinStoreResource::struct_tag());
+    assert!(alice_after.is_none());
+    let bob_after = h.read_aptos_balance(bob.address());
+    assert_eq!(bob_start, bob_after);
+}
+
+#[test]
+fn test_account_not_exist_and_abort_with_fee_payer_create_account() {
+    // This should result in an aborted, yet committed transaction and an account created.
+    let mut h = MoveHarness::new_with_features(vec![FeatureFlag::GAS_PAYER_ENABLED], vec![]);
+
+    let alice = Account::new();
+    let bob = h.new_account_at(AccountAddress::from_hex_literal("0xb0b").unwrap());
+
+    let alice_start =
+        h.read_resource::<CoinStoreResource>(alice.address(), CoinStoreResource::struct_tag());
+    assert!(alice_start.is_none());
+    let bob_start = h.read_aptos_balance(bob.address());
+
+    let payload = aptos_stdlib::aptos_coin_transfer(*alice.address(), 1);
+    let transaction = TransactionBuilder::new(alice.clone())
+        .fee_payer(bob.clone())
+        .payload(payload)
+        .sequence_number(0)
+        .max_gas_amount(100_000) // This is the minimum to execute this transaction
+        .gas_unit_price(1)
+        .sign_fee_payer();
+
+    let output = h.run_raw(transaction);
+    // ECOIN_STORE_NOT_PUBLISHED
+    assert_abort!(output.status(), 393221);
+
+    let alice_after =
+        h.read_resource::<CoinStoreResource>(alice.address(), CoinStoreResource::struct_tag());
+    assert!(alice_after.is_none());
+    let bob_after = h.read_aptos_balance(bob.address());
+
+    assert_eq!(h.sequence_number(alice.address()), 1);
+    assert!(bob_start > bob_after);
+}
+
 #[test]
 fn test_account_not_exist_with_fee_payer_without_create_account() {
     let mut h = MoveHarness::new_with_features(vec![FeatureFlag::GAS_PAYER_ENABLED], vec![

aptos-move/e2e-testsuite/src/tests/failed_transaction_tests.rs

@@ -43,7 +43,7 @@ fn failed_transaction_cleanup_test() {
 
     let change_set_configs = storage_gas_params.change_set_configs.clone();
 
-    let gas_meter = MemoryTrackedGasMeter::new(StandardGasMeter::new(StandardGasAlgebra::new(
+    let mut gas_meter = MemoryTrackedGasMeter::new(StandardGasMeter::new(StandardGasAlgebra::new(
         LATEST_GAS_FEATURE_VERSION,
         gas_params.vm,
         storage_gas_params,
@@ -53,7 +53,7 @@ fn failed_transaction_cleanup_test() {
     // TYPE_MISMATCH should be kept and charged.
     let out1 = aptos_vm.failed_transaction_cleanup(
         VMStatus::error(StatusCode::TYPE_MISMATCH, None),
-        &gas_meter,
+        &mut gas_meter,
         &txn_data,
         &data_cache,
         &log_context,
@@ -73,7 +73,7 @@ fn failed_transaction_cleanup_test() {
     // Invariant violations should be charged.
     let out2 = aptos_vm.failed_transaction_cleanup(
         VMStatus::error(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, None),
-        &gas_meter,
+        &mut gas_meter,
         &txn_data,
         &data_cache,
         &log_context,