diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 3348bf482b5444..915645f486b76b 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -20,7 +20,7 @@ use anyhow::{anyhow, Result}; use aptos_block_executor::txn_commit_hook::NoOpTransactionCommitHook; use aptos_crypto::HashValue; use aptos_framework::natives::code::PublishRequest; -use aptos_gas_algebra::Gas; +use aptos_gas_algebra::{Gas, GasQuantity, Octa}; use aptos_gas_meter::{AptosGasMeter, GasAlgebra, StandardGasAlgebra, StandardGasMeter}; use aptos_gas_schedule::VMGasParameters; use aptos_logger::{enabled, prelude::*, Level}; @@ -309,13 +309,6 @@ impl AptosVM { 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. // @@ -338,52 +331,12 @@ impl AptosVM { } } - // Clear side effects: create new session and clear refunds from fee statement. - let mut session = self - .vm_impl - .new_session(resolver, SessionId::run_on_abort(txn_data)); - - if matches!(transaction_status, TransactionStatus::Keep(_)) - && txn_data.fee_payer().is_some() - && txn_data.sequence_number == 0 - && self - .vm_impl + match TransactionStatus::from_vm_status( + error_code.clone(), + 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_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); - } - } - - // Need to zero this out, because that's the legacy behavior. Otherwise we charge for txn - // size where we used to not. - let mut null_txn_data = txn_data.clone(); - null_txn_data.transaction_size = 0.into(); - let mut respawned_session = match self.charge_change_set_and_respawn_session( - session, - resolver, - gas_meter, - change_set_configs, - &null_txn_data, + .is_enabled(FeatureFlag::CHARGE_INVARIANT_VIOLATION), ) { - 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 { @@ -401,28 +354,21 @@ 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 // should not fail now. If it somehow fails here, there is no choice but to // discard the transaction. - - - 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 = match respawned_session.finish(change_set_configs) { - Ok(change_set) => { + let txn_output = match self.finish_aborted_transaction( + gas_meter, + txn_data, + resolver, + log_context, + change_set_configs, + ) { + Ok((change_set, fee_statement)) => { VMOutput::new(change_set, fee_statement, TransactionStatus::Keep(status)) }, Err(err) => discard_error_vm_status(err).1, @@ -437,6 +383,90 @@ 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> { + if txn_data.fee_payer().is_some() + && txn_data.sequence_number == 0 + && self + .vm_impl + .get_features() + .is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION) + { + // Clear side effects: create new session and clear refunds from fee statement. + let mut session = self + .vm_impl + .new_session(resolver, SessionId::run_on_abort(txn_data)); + + 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, + ) + })?; + + 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, + ); + }; + + // zero out storage refund + let storage_refund = 0; + let fee_statement = + AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, storage_refund); + + let session_id = SessionId::epilogue_meta(txn_data); + let mut respawned_session = RespawnedSession::spawn( + self, + session_id, + resolver, + change_set, + storage_refund.into(), + )?; + respawned_session.execute(|session| { + self.vm_impl.run_failure_epilogue( + session, + gas_meter.balance(), + fee_statement, + txn_data, + log_context, + ) + })?; + respawned_session + .finish(change_set_configs) + .map(|set| (set, fee_statement)) + } else { + // Clear side effects: create new session and clear refunds from fee statement. + let mut session = self + .vm_impl + .new_session(resolver, SessionId::run_on_abort(txn_data)); + + let fee_statement = AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, 0); + self.vm_impl.run_failure_epilogue( + &mut session, + gas_meter.balance(), + fee_statement, + 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, @@ -606,16 +636,12 @@ 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) in change_set.write_set_iter() { gas_meter.charge_io_gas_for_write(key, op)?; } @@ -640,10 +666,11 @@ impl AptosVM { } 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, )?; + if !self .vm_impl .get_features() @@ -652,6 +679,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) @@ -1356,7 +1397,6 @@ impl AptosVM { .new_session(resolver, SessionId::txn_meta(&txn_data)); } - let txn_data = TransactionMetadata::new(txn); if txn_data.fee_payer().is_some() && txn_data.sequence_number == 0 && self diff --git a/aptos-move/aptos-vm/src/aptos_vm_impl.rs b/aptos-move/aptos-vm/src/aptos_vm_impl.rs index 7a0ecb9e6396df..88591935c9d5d5 100644 --- a/aptos-move/aptos-vm/src/aptos_vm_impl.rs +++ b/aptos-move/aptos-vm/src/aptos_vm_impl.rs @@ -339,8 +339,7 @@ impl AptosVMImpl { } let max_gas_amount: u64 = txn_data.max_gas_amount().into(); - let storage_slot_cost: u64 = - txn_gas_params.storage_fee_per_state_slot_create.into(); + let storage_slot_cost: u64 = txn_gas_params.storage_fee_per_state_slot_create.into(); // If this is a sponsored transaction for a potentially new account, ensure there's enough // gas to cover storage, execution, and IO costs.