From 22ae5220fe87b4a3065a24b21eff869161c76b78 Mon Sep 17 00:00:00 2001 From: David Wolinsky Date: Mon, 11 Dec 2023 22:13:27 -0800 Subject: [PATCH] [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 --- api/src/tests/transactions_test.rs | 7 +- .../src/components/feature_flags.rs | 4 +- aptos-move/aptos-vm/src/aptos_vm.rs | 288 ++++++++++++------ aptos-move/aptos-vm/src/gas.rs | 32 +- .../aptos-vm/src/move_vm_ext/session.rs | 13 + .../aptos-vm/src/transaction_validation.rs | 29 +- .../e2e-move-tests/src/tests/fee_payer.rs | 191 +++++++++++- .../src/tests/failed_transaction_tests.rs | 6 +- aptos-move/vm-genesis/src/lib.rs | 2 +- types/src/on_chain_config/aptos_features.rs | 2 +- 10 files changed, 448 insertions(+), 126 deletions(-) diff --git a/api/src/tests/transactions_test.rs b/api/src/tests/transactions_test.rs index 509e3b3a34577..cb402fef3310f 100644 --- a/api/src/tests/transactions_test.rs +++ b/api/src/tests/transactions_test.rs @@ -255,7 +255,7 @@ async fn test_multi_agent_signed_transaction() { async fn test_fee_payer_signed_transaction() { let mut context = new_test_context(current_function_name!()); let account = context.gen_account(); - let fee_payer = context.gen_account(); + let fee_payer = context.create_account().await; let factory = context.transaction_factory(); let mut root_account = context.root_account().await; @@ -324,7 +324,10 @@ async fn test_fee_payer_signed_transaction() { .sign_fee_payer_with_transaction_builder( vec![], &fee_payer, - factory.create_user_account(yet_another_account.public_key()), + factory + .create_user_account(yet_another_account.public_key()) + .max_gas_amount(200_000) + .gas_unit_price(1), ) .into_raw_transaction(); let another_txn = another_raw_txn diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs index 8b522629e3ab3..15f9ca96273a5 100644 --- a/aptos-move/aptos-release-builder/src/components/feature_flags.rs +++ b/aptos-move/aptos-release-builder/src/components/feature_flags.rs @@ -230,7 +230,7 @@ impl From for AptosFeatureFlag { FeatureFlag::SaferMetadata => AptosFeatureFlag::SAFER_METADATA, FeatureFlag::SingleSenderAuthenticator => AptosFeatureFlag::SINGLE_SENDER_AUTHENTICATOR, FeatureFlag::SponsoredAutomaticAccountCreation => { - AptosFeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION + AptosFeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_V1_CREATION }, FeatureFlag::FeePayerAccountOptional => AptosFeatureFlag::FEE_PAYER_ACCOUNT_OPTIONAL, FeatureFlag::AggregatorV2DelayedFields => { @@ -304,7 +304,7 @@ impl From for FeatureFlag { AptosFeatureFlag::SAFER_RESOURCE_GROUPS => FeatureFlag::SaferResourceGroups, AptosFeatureFlag::SAFER_METADATA => FeatureFlag::SaferMetadata, AptosFeatureFlag::SINGLE_SENDER_AUTHENTICATOR => FeatureFlag::SingleSenderAuthenticator, - AptosFeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION => { + AptosFeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_V1_CREATION => { FeatureFlag::SponsoredAutomaticAccountCreation }, AptosFeatureFlag::FEE_PAYER_ACCOUNT_OPTIONAL => FeatureFlag::FeePayerAccountOptional, diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 0022aae5bdaf4..4711b84245abb 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -21,7 +21,7 @@ use anyhow::{anyhow, Result}; use aptos_block_executor::txn_commit_hook::NoOpTransactionCommitHook; use aptos_crypto::HashValue; use aptos_framework::{natives::code::PublishRequest, RuntimeModuleMetadataV1}; -use aptos_gas_algebra::Gas; +use aptos_gas_algebra::{Gas, GasQuantity, Octa}; use aptos_gas_meter::{AptosGasMeter, GasAlgebra, StandardGasAlgebra, StandardGasMeter}; use aptos_gas_schedule::{AptosGasParameters, VMGasParameters}; use aptos_logger::{enabled, prelude::*, Level}; @@ -84,8 +84,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::{ @@ -322,7 +322,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, @@ -384,7 +384,7 @@ 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, @@ -412,10 +412,6 @@ impl AptosVM { } } - // Clear side effects: create new session and clear refunds from fee statement. - let mut session = self.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.features @@ -440,26 +436,26 @@ impl AptosVM { }, _ => status, }; - // The transaction should be charged for gas, so run the epilogue to do that. - // This is running in a new session that drops any side effects from the - // attempted transaction (e.g., spending funds that were needed to pay for gas), - // so even if the previous failure occurred while running the epilogue, it + + // The transaction should be kept. Run the appropriate post transaction workflows + // including epilogue. This runs a new session that ignores any side effects that + // might abort the execution (e.g., spending additional funds needed to pay for + // gas). Eeven 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. - transaction_validation::run_failure_epilogue( - &mut session, - gas_meter.balance(), - fee_statement, - &self.features, + let txn_output = match self.finish_aborted_transaction( + gas_meter, txn_data, + resolver, log_context, - ) - .unwrap_or_else(|| { - let txn_output = - get_transaction_output(session, fee_statement, status, change_set_configs) - .unwrap_or_else(|e| discarded_output(e.status_code())); - (error_code, txn_output) - }) + change_set_configs, + ) { + Ok((change_set, fee_statement)) => { + VMOutput::new(change_set, fee_statement, TransactionStatus::Keep(status)) + }, + Err(err) => discarded_output(err.status_code()), + }; + (error_code, txn_output) }, TransactionStatus::Discard(status_code) => ( VMStatus::error(status_code, None), @@ -469,6 +465,115 @@ impl AptosVM { } } + fn finish_aborted_transaction( + &self, + gas_meter: &mut impl AptosGasMeter, + txn_data: &TransactionMetadata, + resolver: &impl AptosMoveResolver, + log_context: &AdapterLogSchema, + change_set_configs: &ChangeSetConfigs, + ) -> Result<(VMChangeSet, FeeStatement), VMStatus> { + // Storage refund is zero since no slots are deleted in aborted transactions. + const ZERO_STORAGE_REFUND: u64 = 0; + + if is_account_init_for_sponsored_transaction(txn_data, &self.features) { + let mut session = self.new_session(resolver, SessionId::run_on_abort(txn_data)); + + create_account_if_does_not_exist(&mut session, gas_meter, txn_data.sender()) + // if this fails, it is likely due to out of gas, so we try again without metering + // and then validate below that we charged sufficiently. + .or_else(|_err| { + create_account_if_does_not_exist( + &mut 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, + ) + })?; + + let mut change_set = session.finish(change_set_configs)?; + if let Err(err) = self.charge_change_set(&mut change_set, gas_meter, txn_data) { + info!( + *log_context, + "Failed during charge_change_set: {:?}. Most likely exceded gas limited.", err, + ); + }; + + let fee_statement = + AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, ZERO_STORAGE_REFUND); + + // Verify we charged sufficiently for creating an account slot + let gas_params = get_or_vm_startup_failure(&self.gas_params, log_context)?; + let gas_unit_price = u64::from(txn_data.gas_unit_price()); + let gas_used = fee_statement.gas_used(); + let storage_fee = fee_statement.storage_fee_used(); + let storage_refund = fee_statement.storage_fee_refund(); + + let actual = gas_used * gas_unit_price + storage_fee - storage_refund; + let expected = u64::from(gas_params.vm.txn.storage_fee_per_state_slot_create); + // With the current gas schedule actual is expected to be 50_000 or higher, the cost + // for the account slot. + if actual < expected { + expect_only_successful_execution( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message( + "Insufficient fee for storing account for sponsored transaction" + .to_string(), + ) + .finish(Location::Undefined), + &format!("{:?}::{}", ACCOUNT_MODULE, CREATE_ACCOUNT_IF_DOES_NOT_EXIST), + log_context, + )?; + } + + let session_id = SessionId::epilogue_meta(txn_data); + let mut respawned_session = RespawnedSession::spawn( + self, + session_id, + resolver, + change_set, + ZERO_STORAGE_REFUND.into(), + )?; + respawned_session.execute(|session| { + transaction_validation::run_failure_epilogue( + session, + gas_meter.balance(), + fee_statement, + &self.features, + txn_data, + log_context, + ) + })?; + respawned_session + .finish(change_set_configs) + .map(|set| (set, fee_statement)) + } else { + let mut session = self.new_session(resolver, SessionId::epilogue_meta(txn_data)); + + let fee_statement = + AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, ZERO_STORAGE_REFUND); + transaction_validation::run_failure_epilogue( + &mut session, + gas_meter.balance(), + fee_statement, + &self.features, + txn_data, + log_context, + )?; + session + .finish(change_set_configs) + .map(|set| (set, fee_statement)) + .map_err(|e| e.into()) + } + } + fn success_transaction_cleanup( &self, mut respawned_session: RespawnedSession, @@ -634,22 +739,18 @@ impl AptosVM { } } - fn charge_change_set_and_respawn_session<'r, 'l>( - &'l self, - session: SessionExt, - resolver: &'r impl AptosMoveResolver, + fn charge_change_set( + &self, + change_set: &mut VMChangeSet, gas_meter: &mut impl AptosGasMeter, - change_set_configs: &ChangeSetConfigs, txn_data: &TransactionMetadata, - ) -> Result, VMStatus> { - let mut change_set = session.finish(change_set_configs)?; - + ) -> Result, VMStatus> { for (key, op_size) in change_set.write_set_size_iter() { gas_meter.charge_io_gas_for_write(key, &op_size)?; } let mut storage_refund = gas_meter.process_storage_fee_for_all( - &mut change_set, + change_set, txn_data.transaction_size, txn_data.gas_unit_price, )?; @@ -657,6 +758,20 @@ impl AptosVM { storage_refund = 0.into(); } + Ok(storage_refund) + } + + fn charge_change_set_and_respawn_session<'r, 'l>( + &'l self, + session: SessionExt, + resolver: &'r impl AptosMoveResolver, + gas_meter: &mut impl AptosGasMeter, + change_set_configs: &ChangeSetConfigs, + txn_data: &TransactionMetadata, + ) -> Result, VMStatus> { + let mut change_set = session.finish(change_set_configs)?; + let storage_refund = self.charge_change_set(&mut change_set, gas_meter, txn_data)?; + // TODO[agg_v1](fix): Charge for aggregator writes let session_id = SessionId::epilogue_meta(txn_data); RespawnedSession::spawn(self, session_id, resolver, change_set, storage_refund) @@ -1349,26 +1464,14 @@ impl AptosVM { session = self.new_session(resolver, SessionId::txn_meta(&txn_data)); } - if let aptos_types::transaction::authenticator::TransactionAuthenticator::FeePayer { - .. - } = &txn.authenticator_ref() - { - if self - .features - .is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION) + if is_account_init_for_sponsored_transaction(&txn_data, &self.features) { + 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, - ) { - let vm_status = err.into_vm_status(); - let discarded_output = discarded_output(vm_status.status_code()); - return (vm_status, discarded_output); - }; - } + let vm_status = err.into_vm_status(); + let discarded_output = discarded_output(vm_status.status_code()); + return (vm_status, discarded_output); + }; } let storage_gas_params = unwrap_or_discard!(get_or_vm_startup_failure( @@ -1729,36 +1832,27 @@ impl AptosVM { txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, ) -> Result<(), VMStatus> { + // Deprecated. Will be removed in the future. + if matches!(payload, TransactionPayload::ModuleBundle(_)) + && MODULE_BUNDLE_DISALLOWED.load(Ordering::Relaxed) + { + return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None)); + } + + check_gas( + get_or_vm_startup_failure(&self.gas_params, log_context)?, + self.gas_feature_version, + resolver, + txn_data, + &self.features, + log_context, + )?; + match payload { - TransactionPayload::Script(_) => { - check_gas( - get_or_vm_startup_failure(&self.gas_params, log_context)?, - self.gas_feature_version, - resolver, - txn_data, - log_context, - )?; - transaction_validation::run_script_prologue(session, txn_data, log_context) - }, - TransactionPayload::EntryFunction(_) => { - // NOTE: Script and EntryFunction shares the same prologue - check_gas( - get_or_vm_startup_failure(&self.gas_params, log_context)?, - self.gas_feature_version, - resolver, - txn_data, - log_context, - )?; + TransactionPayload::Script(_) | TransactionPayload::EntryFunction(_) => { transaction_validation::run_script_prologue(session, txn_data, log_context) }, TransactionPayload::Multisig(multisig_payload) => { - check_gas( - get_or_vm_startup_failure(&self.gas_params, log_context)?, - self.gas_feature_version, - resolver, - txn_data, - log_context, - )?; // Still run script prologue for multisig transaction to ensure the same tx // validations are still run for this multisig execution tx, which is submitted by // one of the owners. @@ -1777,19 +1871,8 @@ impl AptosVM { Ok(()) } }, - // Deprecated. Will be removed in the future. TransactionPayload::ModuleBundle(_module) => { - if MODULE_BUNDLE_DISALLOWED.load(Ordering::Relaxed) { - return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None)); - } - check_gas( - get_or_vm_startup_failure(&self.gas_params, log_context)?, - self.gas_feature_version, - resolver, - txn_data, - log_context, - )?; transaction_validation::run_module_prologue(session, txn_data, log_context) }, } @@ -2107,6 +2190,31 @@ 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| ()) +} + +pub(crate) fn is_account_init_for_sponsored_transaction( + txn_data: &TransactionMetadata, + features: &Features, +) -> bool { + txn_data.fee_payer.is_some() + && txn_data.sequence_number == 0 + && features.is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_V1_CREATION) +} + #[test] fn vm_thread_safe() { fn assert_send() {} diff --git a/aptos-move/aptos-vm/src/gas.rs b/aptos-move/aptos-vm/src/gas.rs index e0923dc459965..648929241d36e 100644 --- a/aptos-move/aptos-vm/src/gas.rs +++ b/aptos-move/aptos-vm/src/gas.rs @@ -8,7 +8,7 @@ use aptos_gas_schedule::{ }; use aptos_logger::{enabled, Level}; use aptos_types::on_chain_config::{ - ApprovedExecutionHashes, ConfigStorage, GasSchedule, GasScheduleV2, OnChainConfig, + ApprovedExecutionHashes, ConfigStorage, Features, GasSchedule, GasScheduleV2, OnChainConfig, }; use aptos_vm_logging::{log_schema::AdapterLogSchema, speculative_log, speculative_warn}; use aptos_vm_types::storage::{io_pricing::IoPricing, StorageGasParameters}; @@ -114,6 +114,7 @@ pub(crate) fn check_gas( gas_feature_version: u64, resolver: &impl AptosMoveResolver, txn_metadata: &TransactionMetadata, + features: &Features, log_context: &AdapterLogSchema, ) -> Result<(), VMStatus> { let txn_gas_params = &gas_params.vm.txn; @@ -237,5 +238,34 @@ pub(crate) fn check_gas( None, )); } + + // If this is a sponsored transaction for a potentially new account, ensure there's enough + // gas to cover storage, execution, and IO costs. + // TODO: This isn't the cleaning code, thus we localize it just here and will remove it + // once accountv2 is available and we no longer need to create accounts. + if crate::aptos_vm::is_account_init_for_sponsored_transaction(txn_metadata, features) { + let gas_unit_price: u64 = txn_metadata.gas_unit_price().into(); + let max_gas_amount: u64 = txn_metadata.max_gas_amount().into(); + let storage_fee_per_state_slot_create: u64 = + txn_gas_params.storage_fee_per_state_slot_create.into(); + + let expected = gas_unit_price * 10 + 2 * storage_fee_per_state_slot_create; + let actual = gas_unit_price * max_gas_amount; + + if actual < expected { + speculative_warn!( + log_context, + format!( + "[VM] Insufficient gas for sponsored transaction; min {}, submitted {}", + expected, actual, + ), + ); + return Err(VMStatus::error( + StatusCode::MAX_GAS_UNITS_BELOW_MIN_TRANSACTION_GAS_UNITS, + None, + )); + } + } + Ok(()) } diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session.rs b/aptos-move/aptos-vm/src/move_vm_ext/session.rs index 93ca6f10f10ae..539b805794f1c 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session.rs @@ -74,6 +74,11 @@ pub enum SessionId { }, // For those runs that are not a transaction and the output of which won't be committed. Void, + RunOnAbort { + sender: AccountAddress, + sequence_number: u64, + script_hash: Vec, + }, } impl SessionId { @@ -103,6 +108,14 @@ impl SessionId { } } + pub fn run_on_abort(txn_metadata: &TransactionMetadata) -> Self { + Self::RunOnAbort { + sender: txn_metadata.sender, + sequence_number: txn_metadata.sequence_number, + script_hash: txn_metadata.script_hash.clone(), + } + } + pub fn epilogue_meta(txn_metadata: &TransactionMetadata) -> Self { Self::Epilogue { sender: txn_metadata.sender, diff --git a/aptos-move/aptos-vm/src/transaction_validation.rs b/aptos-move/aptos-vm/src/transaction_validation.rs index 42534409235a3..0aac37c8d438f 100644 --- a/aptos-move/aptos-vm/src/transaction_validation.rs +++ b/aptos-move/aptos-vm/src/transaction_validation.rs @@ -2,10 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - errors::{ - convert_epilogue_error, convert_prologue_error, discarded_output, - expect_only_successful_execution, - }, + errors::{convert_epilogue_error, convert_prologue_error, expect_only_successful_execution}, move_vm_ext::SessionExt, system_module_names::{ EMIT_FEE_STATEMENT, MULTISIG_ACCOUNT_MODULE, TRANSACTION_FEE_MODULE, @@ -20,7 +17,6 @@ use aptos_types::{ on_chain_config::Features, transaction::Multisig, }; use aptos_vm_logging::log_schema::AdapterLogSchema; -use aptos_vm_types::output::VMOutput; use fail::fail_point; use move_binary_format::errors::VMResult; use move_core_types::{ @@ -322,19 +318,12 @@ pub(crate) fn run_failure_epilogue( features: &Features, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, -) -> Option<(VMStatus, VMOutput)> { - if let Err(e) = - run_epilogue(session, gas_remaining, fee_statement, txn_data, features).or_else(|e| { - expect_only_successful_execution( - e, - APTOS_TRANSACTION_VALIDATION.user_epilogue_name.as_str(), - log_context, - ) - }) - { - let discarded_output = discarded_output(e.status_code()); - Some((e, discarded_output)) - } else { - None - } +) -> Result<(), VMStatus> { + run_epilogue(session, gas_remaining, fee_statement, txn_data, features).or_else(|e| { + expect_only_successful_execution( + e, + APTOS_TRANSACTION_VALIDATION.user_epilogue_name.as_str(), + log_context, + ) + }) } diff --git a/aptos-move/e2e-move-tests/src/tests/fee_payer.rs b/aptos-move/e2e-move-tests/src/tests/fee_payer.rs index 227517146b57c..e2a14f1e1bb88 100644 --- a/aptos-move/e2e-move-tests/src/tests/fee_payer.rs +++ b/aptos-move/e2e-move-tests/src/tests/fee_payer.rs @@ -1,20 +1,36 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use crate::{assert_success, MoveHarness}; +use crate::{assert_success, tests::common, MoveHarness}; use aptos_cached_packages::aptos_stdlib; +use aptos_crypto::HashValue; use aptos_language_e2e_tests::{ account::{Account, TransactionBuilder}, transaction_status_eq, }; use aptos_types::{ - account_address::AccountAddress, account_config::CoinStoreResource, - on_chain_config::FeatureFlag, transaction::TransactionStatus, + account_address::AccountAddress, + account_config::CoinStoreResource, + move_utils::MemberId, + on_chain_config::{ApprovedExecutionHashes, FeatureFlag, OnChainConfig}, + transaction::{EntryFunction, ExecutionStatus, Script, TransactionPayload, TransactionStatus}, }; use move_core_types::{move_resource::MoveStructType, vm_status::StatusCode}; +// Fee payer has several modes and requires several tests to validate: +// Account exists: +// * Account exists and transaction executes successfully +// * Account exists and transaction aborts but is kept +// * Account doesn't exist (seq num 0) and transaction executes successfully +// * Account doesn't exist (seq num 0), transaction aborts due to move abort, and account is created +// * Account doesn't exist (seq num 0), transaction aborts due to out of gas, and account is created +// * Account doesn't exist (seq num 0), transaction aborts due to move abort, during charging of +// account creation changeset, we run out of gas, but account must still be created. Note, this is +// likely a duplicate of the first out of gas, but included. +// * Invalid transactions are discarded during prologue, specifically the special case of seq num 0 + #[test] -fn test_normal_tx_with_signer_with_fee_payer() { +fn test_existing_account_with_fee_payer() { let mut h = MoveHarness::new_with_features(vec![FeatureFlag::GAS_PAYER_ENABLED], vec![]); let alice = h.new_account_at(AccountAddress::from_hex_literal("0xa11ce").unwrap()); @@ -43,7 +59,7 @@ fn test_normal_tx_with_signer_with_fee_payer() { } #[test] -fn test_account_not_exist_with_fee_payer_create_account() { +fn test_account_not_exist_with_fee_payer() { let mut h = MoveHarness::new_with_features(vec![FeatureFlag::GAS_PAYER_ENABLED], vec![]); let alice = Account::new(); @@ -74,10 +90,173 @@ fn test_account_not_exist_with_fee_payer_create_account() { assert!(bob_start > bob_after); } +#[test] +fn test_account_not_exist_with_fee_payer_insufficient_gas() { + 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::(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::(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_move_abort_with_fee_payer_create_account() { + 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::(alice.address(), CoinStoreResource::struct_tag()); + assert!(alice_start.is_none()); + let bob_start = h.read_aptos_balance(bob.address()); + + // script { + // fun main() { + // 1/0; + // } + // } + let data = + hex::decode("a11ceb0b030000000105000100000000050601000000000000000600000000000000001a0102") + .unwrap(); + let script = Script::new(data, vec![], vec![]); + + // Offered max fee is 10000 + gas_units * 10, the minimum to execute this transaction + let transaction = TransactionBuilder::new(alice.clone()) + .fee_payer(bob.clone()) + .script(script) + .sequence_number(0) + .max_gas_amount(50_010) + .gas_unit_price(2) + .sign_fee_payer(); + + let output = h.run_raw(transaction); + assert!(matches!( + output.status(), + TransactionStatus::Keep(ExecutionStatus::ExecutionFailure { .. }) + )); + // We need to charge less than or equal to the max and at least more than a storage slot + assert!(output.gas_used() * 2 <= 100020); + assert!(output.gas_used() * 2 > 50000); + + let alice_after = + h.read_resource::(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_out_of_gas_with_fee_payer() { + let mut h = MoveHarness::new_with_features(vec![FeatureFlag::GAS_PAYER_ENABLED], vec![]); + + let alice = Account::new(); + let beef = h.new_account_at(AccountAddress::from_hex_literal("0xbeef").unwrap()); + + // Load the code + assert_success!(h.publish_package_cache_building( + &beef, + &common::test_dir_path("infinite_loop.data/empty_loop"), + )); + + let MemberId { + module_id, + member_id, + } = str::parse("0xbeef::test::run").unwrap(); + let payload = + TransactionPayload::EntryFunction(EntryFunction::new(module_id, member_id, vec![], vec![])); + let transaction = TransactionBuilder::new(alice.clone()) + .fee_payer(beef.clone()) + .payload(payload) + .sequence_number(0) + .max_gas_amount(100_010) // This is the minimum to execute this transaction + .gas_unit_price(1) + .sign_fee_payer(); + let result = h.run_raw(transaction); + + assert_eq!( + result.status(), + &TransactionStatus::Keep(ExecutionStatus::MiscellaneousError(Some( + aptos_types::vm_status::StatusCode::EXECUTION_LIMIT_REACHED + ))), + ); +} + +#[test] +fn test_account_not_exist_move_abort_with_fee_payer_out_of_gas() { + // Very large transaction to trigger the out of gas error aborted seqno 0 sponsored transactions + 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 root = h.aptos_framework_account(); + + let data = vec![0; 1000 * 1024]; + let entries = ApprovedExecutionHashes { + entries: vec![(0, HashValue::sha3_256_of(&data).to_vec())], + }; + + let script = Script::new(data, vec![], vec![]); + + h.set_resource( + *root.address(), + ApprovedExecutionHashes::struct_tag(), + &entries, + ); + let transaction = TransactionBuilder::new(alice.clone()) + .fee_payer(bob.clone()) + .script(script.clone()) + .sequence_number(0) + .max_gas_amount(100_010) // This is the minimum to execute this transaction + .gas_unit_price(1) + .sign_fee_payer(); + let result = h.run_raw(transaction); + assert_eq!(result.gas_used(), 100_010); + + let new_alice = Account::new(); + let transaction = TransactionBuilder::new(new_alice.clone()) + .fee_payer(bob.clone()) + .script(script.clone()) + .sequence_number(0) + .max_gas_amount(100_011) // Bump by one to ensure more gas can be used + .gas_unit_price(1) + .sign_fee_payer(); + let result = h.run_raw(transaction); + assert_eq!(result.gas_used(), 100_011); +} + #[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![ - FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION, + FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_V1_CREATION, ]); let alice = Account::new(); diff --git a/aptos-move/e2e-testsuite/src/tests/failed_transaction_tests.rs b/aptos-move/e2e-testsuite/src/tests/failed_transaction_tests.rs index 034bc2d68204e..df18fdfa345ca 100644 --- a/aptos-move/e2e-testsuite/src/tests/failed_transaction_tests.rs +++ b/aptos-move/e2e-testsuite/src/tests/failed_transaction_tests.rs @@ -44,7 +44,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, @@ -54,7 +54,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, @@ -78,7 +78,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, diff --git a/aptos-move/vm-genesis/src/lib.rs b/aptos-move/vm-genesis/src/lib.rs index 4d9ee7421b9ba..8443ba957e090 100644 --- a/aptos-move/vm-genesis/src/lib.rs +++ b/aptos-move/vm-genesis/src/lib.rs @@ -436,7 +436,7 @@ pub fn default_features() -> Vec { FeatureFlag::SAFER_RESOURCE_GROUPS, FeatureFlag::SAFER_METADATA, FeatureFlag::SINGLE_SENDER_AUTHENTICATOR, - FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION, + FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_V1_CREATION, FeatureFlag::FEE_PAYER_ACCOUNT_OPTIONAL, FeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH, FeatureFlag::OPERATOR_BENEFICIARY_CHANGE, diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs index f06b38eced5f4..faa668973900b 100644 --- a/types/src/on_chain_config/aptos_features.rs +++ b/types/src/on_chain_config/aptos_features.rs @@ -41,7 +41,7 @@ pub enum FeatureFlag { SAFER_RESOURCE_GROUPS = 31, SAFER_METADATA = 32, SINGLE_SENDER_AUTHENTICATOR = 33, - SPONSORED_AUTOMATIC_ACCOUNT_CREATION = 34, + SPONSORED_AUTOMATIC_ACCOUNT_V1_CREATION = 34, FEE_PAYER_ACCOUNT_OPTIONAL = 35, AGGREGATOR_V2_DELAYED_FIELDS = 36, CONCURRENT_ASSETS = 37,