[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

More motivation to version the VM at some point in time :)

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

@@ -76,8 +76,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::{
@@ -245,7 +245,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,
@@ -307,12 +307,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.
             //
@@ -339,14 +346,30 @@ 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) = self.must_create_account_if_does_not_exist(
+                &mut session,
+                gas_meter,
+                txn_data,
+                resolver,
+                log_context,
+                change_set_configs,
+            ) {
+                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 {
@@ -396,6 +419,51 @@ impl AptosVM {
         }
     }
 
+    fn must_create_account_if_does_not_exist(
+        &self,
+        session: &mut SessionExt,
+        gas_meter: &mut impl AptosGasMeter,
+        txn_data: &TransactionMetadata,
+        resolver: &impl AptosMoveResolver,
+        log_context: &AdapterLogSchema,
+        change_set_configs: &ChangeSetConfigs,
+    ) -> Result<(), VMStatus> {
+        // Use this session to perform the write operations
+        create_account_if_does_not_exist(session, &mut UnmeteredGasMeter, 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,
+                )
+            })?;
+
+        // Use this session to charge gas
+        let mut storage_session = self
+            .vm_impl
+            .new_session(resolver, SessionId::epilogue_meta(txn_data));
+
+        create_account_if_does_not_exist(&mut storage_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,
+                )
+            })?;
+
+        self.charge_change_set_and_respawn_session(
+            storage_session,
+            resolver,
+            gas_meter,
+            change_set_configs,
+            txn_data,
+        )
+        .map(|_return_vals| ())
+    }
+
     fn success_transaction_cleanup(
         &self,
         mut respawned_session: RespawnedSession,
@@ -1216,30 +1284,22 @@ impl AptosVM {
             session = self.vm_impl.new_session(resolver, SessionId::txn(txn));
         }
 
-        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 =
             unwrap_or_discard!(self.vm_impl.get_storage_gas_parameters(log_context));
-        let txn_data = TransactionMetadata::new(txn);
 
         // We keep track of whether any newly published modules are loaded into the Vm's loader
         // cache as part of executing transactions. This would allow us to decide whether the cache
@@ -2070,7 +2130,7 @@ impl AptosSimulationVM {
                 } else {
                     let (vm_status, output) = self.0.failed_transaction_cleanup_and_keep_vm_status(
                         err,
-                        &gas_meter,
+                        &mut gas_meter,
                         &txn_data,
                         resolver,
                         log_context,
@@ -2082,3 +2142,19 @@ impl AptosSimulationVM {
         }
     }
 }
+
+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

@@ -355,6 +355,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![