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.

Note, the min gas amount is effectively 2, where we need roughly 4 for this operation, so more needs to be figured out.
  • Loading branch information
davidiw committed Nov 25, 2023
1 parent 0c7147d commit 419e9d4
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 22 deletions.
30 changes: 13 additions & 17 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,30 +1216,26 @@ 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) = 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());
};
}
{
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,

Check warning on line 1231 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#L1231

Added line #L1231 was not covered by tests
) {
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
Expand Down
29 changes: 27 additions & 2 deletions aptos-move/aptos-vm/src/aptos_vm_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::{
errors::{convert_epilogue_error, convert_prologue_error, expect_only_successful_execution},
move_vm_ext::{AptosMoveResolver, MoveVmExt, SessionExt, SessionId},
system_module_names::{
EMIT_FEE_STATEMENT, MULTISIG_ACCOUNT_MODULE, TRANSACTION_FEE_MODULE,
VALIDATE_MULTISIG_TRANSACTION,
ACCOUNT_MODULE, CREATE_ACCOUNT_IF_DOES_NOT_EXIST, EMIT_FEE_STATEMENT,
MULTISIG_ACCOUNT_MODULE, TRANSACTION_FEE_MODULE, VALIDATE_MULTISIG_TRANSACTION,
},
testing::{maybe_raise_injected_error, InjectedError},
transaction_metadata::TransactionMetadata,
Expand Down Expand Up @@ -615,6 +615,31 @@ impl AptosVMImpl {
txn_data: &TransactionMetadata,
log_context: &AdapterLogSchema,
) -> Result<(), VMStatus> {
if txn_data.fee_payer().is_some()
&& txn_data.sequence_number == 0
&& self
.get_features()
.is_enabled(FeatureFlag::SPONSORED_AUTOMATIC_ACCOUNT_CREATION)
{
session
.execute_function_bypass_visibility(
&ACCOUNT_MODULE,
CREATE_ACCOUNT_IF_DOES_NOT_EXIST,
vec![],
serialize_values(&vec![MoveValue::Address(txn_data.sender())]),
&mut UnmeteredGasMeter,
)
.map(|_return_vals| ())
.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 639 in aptos-move/aptos-vm/src/aptos_vm_impl.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm_impl.rs#L635-L639

Added lines #L635 - L639 were not covered by tests
})?;
}

self.run_epilogue(session, gas_remaining, fee_statement, txn_data)
.or_else(|e| {
expect_only_successful_execution(
Expand Down
80 changes: 77 additions & 3 deletions aptos-move/e2e-move-tests/src/tests/fee_payer.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
// 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},
transaction_status_eq,
};
use aptos_types::{
account_address::AccountAddress, account_config::CoinStoreResource,
on_chain_config::FeatureFlag, transaction::TransactionStatus,
account_address::AccountAddress,
account_config::CoinStoreResource,
on_chain_config::FeatureFlag,
transaction::{ExecutionStatus, TransactionStatus},
};
use move_core_types::{move_resource::MoveStructType, vm_status::StatusCode};

Expand Down Expand Up @@ -74,6 +76,78 @@ 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(2)
.gas_unit_price(1)
.sign_fee_payer();

let output = h.run_raw(transaction);
assert_eq!(
output.status(),
&TransactionStatus::Keep(ExecutionStatus::OutOfGas)
);

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_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(4)
.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![
Expand Down

0 comments on commit 419e9d4

Please sign in to comment.