Skip to content

Commit

Permalink
[gas] charge for loading resources even when they have been cached by…
Browse files Browse the repository at this point in the history
… the prologue (aptos-labs#4256)
  • Loading branch information
vgao1996 authored Sep 16, 2022
1 parent cad96f0 commit d624915
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 9 deletions.
9 changes: 9 additions & 0 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,15 @@ impl AptosVM {
return discard_error_vm_status(err);
};

if unwrap_or_discard!(self.0.get_gas_feature_version(log_context)) >= 1 {
// Create a new session so that the data cache is flushed.
// This is to ensure we correctly charge for loading certain resources, even if they
// have been previously cached in the prologue.
//
// TODO(Gas): Do this in a better way in the future, perhaps without forcing the data cache to be flushed.
session = self.0.new_session(storage, SessionId::txn(txn));
}

let gas_params = unwrap_or_discard!(self.0.get_gas_parameters(log_context));
let txn_data = TransactionMetadata::new(txn);
let mut gas_meter = AptosGasMeter::new(gas_params.clone(), txn_data.max_gas_amount());
Expand Down
4 changes: 3 additions & 1 deletion aptos-move/vm-genesis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ fn exec_function(
});
}

const LATEST_GAS_FEATURE_VERSION: u64 = 1;

fn initialize(
session: &mut SessionExt<impl MoveResolver>,
consensus_config: OnChainConsensusConfig,
Expand All @@ -264,7 +266,7 @@ fn initialize(
// We should get rid of it after we make another testnet release.
let gas_schedule_blob = if use_gas_schedule_v2 {
let gas_schedule = GasScheduleV2 {
feature_version: 0,
feature_version: LATEST_GAS_FEATURE_VERSION,
entries: genesis_gas_params.to_on_chain_gas_schedule(),
};
bcs::to_bytes(&gas_schedule).expect("Failure serializing genesis gas schedule")
Expand Down
3 changes: 2 additions & 1 deletion crates/aptos/src/account/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use cached_packages::aptos_stdlib;
use clap::Parser;

// TODO(Gas): double check if this is correct
pub const DEFAULT_FUNDED_COINS: u64 = 50_000;
// TODO(greg): revisit after fixing gas estimation
pub const DEFAULT_FUNDED_COINS: u64 = 500_000;

/// Command to create a new account on-chain
///
Expand Down
18 changes: 16 additions & 2 deletions crates/aptos/src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,14 @@ impl CliTestFramework {

pub async fn add_stake(&self, index: usize, amount: u64) -> CliTypedResult<TransactionSummary> {
AddStake {
txn_options: self.transaction_options(index, None),
txn_options: self.transaction_options(
index,
// TODO(greg): revisit after fixing gas estimation
Some(GasOptions {
gas_unit_price: Some(1),
max_gas: Some(10000),
}),
),
amount,
}
.execute()
Expand Down Expand Up @@ -486,7 +493,14 @@ impl CliTestFramework {
operator_index: Option<usize>,
) -> CliTypedResult<TransactionSummary> {
InitializeStakeOwner {
txn_options: self.transaction_options(owner_index, None),
txn_options: self.transaction_options(
owner_index,
// TODO(greg): revisit after fixing gas estimation
Some(GasOptions {
gas_unit_price: Some(1),
max_gas: Some(10000),
}),
),
initial_stake_amount,
operator_address: operator_index.map(|idx| self.account_id(idx)),
voter_address: voter_index.map(|idx| self.account_id(idx)),
Expand Down
12 changes: 7 additions & 5 deletions testsuite/smoke-test/src/rosetta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,11 @@ async fn test_block() {
let account_id_1 = cli.account_id(1);
let account_id_3 = cli.account_id(3);

cli.fund_account(0, Some(10000000)).await.unwrap();
cli.fund_account(1, Some(650000)).await.unwrap();
cli.fund_account(2, Some(50000)).await.unwrap();
cli.fund_account(3, Some(20000)).await.unwrap();
// TODO(greg): revisit after fixing gas estimation
cli.fund_account(0, Some(100000000)).await.unwrap();
cli.fund_account(1, Some(6500000)).await.unwrap();
cli.fund_account(2, Some(500000)).await.unwrap();
cli.fund_account(3, Some(200000)).await.unwrap();

let private_key_0 = cli.private_key(0);
let private_key_1 = cli.private_key(1);
Expand Down Expand Up @@ -512,7 +513,8 @@ async fn test_block() {
20,
Duration::from_secs(5),
Some(seq_no_0 + 1),
None,
// TODO(greg): revisit after fixing gas estimation
Some(10000),
None,
)
.await
Expand Down

0 comments on commit d624915

Please sign in to comment.