Skip to content

Commit

Permalink
[movevm] improve automatic creation of account for sponsored txn
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
davidiw committed Dec 12, 2023
1 parent 8c3e8bb commit 401af5d
Show file tree
Hide file tree
Showing 10 changed files with 352 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl From<FeatureFlag> 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 => {
Expand Down Expand Up @@ -302,7 +302,7 @@ impl From<AptosFeatureFlag> 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,
Expand Down
255 changes: 165 additions & 90 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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::{
Expand Down Expand Up @@ -322,7 +322,7 @@ impl AptosVM {
pub fn failed_transaction_cleanup(
&self,
error_code: VMStatus,
gas_meter: &impl AptosGasMeter,
gas_meter: &mut impl AptosGasMeter,

Check warning on line 325 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L325 was not covered by tests
txn_data: &TransactionMetadata,
resolver: &impl AptosMoveResolver,
log_context: &AdapterLogSchema,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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()),

Check warning on line 456 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L456 was not covered by tests
};
(error_code, txn_output)
},
TransactionStatus::Discard(status_code) => (
VMStatus::error(status_code, None),
Expand All @@ -469,6 +465,82 @@ 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())
.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,
)

Check warning on line 489 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L485-L489

Added lines #L485 - L489 were not covered by tests
})?;

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,

Check warning on line 496 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L496 was not covered by tests
);
};

let fee_statement =
AptosVM::fee_statement_from_gas_meter(txn_data, gas_meter, ZERO_STORAGE_REFUND);

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,
Expand Down Expand Up @@ -634,29 +706,39 @@ 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<RespawnedSession<'r, 'l>, VMStatus> {
let mut change_set = session.finish(change_set_configs)?;

) -> Result<GasQuantity<Octa>, 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,
)?;
if !self.features.is_storage_deletion_refund_enabled() {
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<RespawnedSession<'r, 'l>, 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)
Expand Down Expand Up @@ -1349,26 +1431,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) =

Check warning on line 1435 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L1435 was not covered by tests
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);

Check warning on line 1440 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1438-L1440

Added lines #L1438 - L1440 were not covered by tests
};
}

let storage_gas_params = unwrap_or_discard!(get_or_vm_startup_failure(
Expand Down Expand Up @@ -1729,36 +1799,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 warning on line 1806 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L1806 was not covered by tests
}

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.
Expand All @@ -1777,19 +1838,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)
},
}
Expand Down Expand Up @@ -2107,6 +2157,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<T: Send>() {}
Expand Down
Loading

0 comments on commit 401af5d

Please sign in to comment.