Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove per slot free quota for write gas #11278

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion aptos-move/aptos-abstract-gas-usage/src/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use aptos_gas_algebra::{
};
use aptos_gas_meter::GasAlgebra;
use aptos_gas_schedule::VMGasParameters;
use aptos_vm_types::storage::io_pricing::IoPricing;
use aptos_vm_types::storage::{io_pricing::IoPricing, space_pricing::DiskSpacePricing};
use move_binary_format::errors::PartialVMResult;
use std::sync::{Arc, Mutex};

Expand All @@ -33,6 +33,10 @@ impl<A: GasAlgebra> GasAlgebra for CalibrationAlgebra<A> {
self.base.io_pricing()
}

fn disk_space_pricing(&self) -> &DiskSpacePricing {
self.base.disk_space_pricing()
}

fn balance_internal(&self) -> InternalGas {
self.base.balance_internal()
}
Expand Down
8 changes: 7 additions & 1 deletion aptos-move/aptos-gas-meter/src/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::traits::GasAlgebra;
use aptos_gas_algebra::{Fee, FeePerGasUnit, Gas, GasExpression, Octa};
use aptos_gas_schedule::VMGasParameters;
use aptos_logger::error;
use aptos_vm_types::storage::{io_pricing::IoPricing, StorageGasParameters};
use aptos_vm_types::storage::{
io_pricing::IoPricing, space_pricing::DiskSpacePricing, StorageGasParameters,
};
use move_binary_format::errors::{PartialVMError, PartialVMResult};
use move_core_types::{
gas_algebra::{InternalGas, InternalGasUnit},
Expand Down Expand Up @@ -87,6 +89,10 @@ impl GasAlgebra for StandardGasAlgebra {
&self.storage_gas_params.io_pricing
}

fn disk_space_pricing(&self) -> &DiskSpacePricing {
&self.storage_gas_params.space_pricing
}

fn balance_internal(&self) -> InternalGas {
self.balance
}
Expand Down
34 changes: 1 addition & 33 deletions aptos-move/aptos-gas-meter/src/meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
use crate::traits::{AptosGasMeter, GasAlgebra};
use aptos_gas_algebra::{Fee, FeePerGasUnit};
use aptos_gas_schedule::gas_params::{instr::*, txn::*};
use aptos_types::{
contract_event::ContractEvent, state_store::state_key::StateKey, write_set::WriteOpSize,
};
use aptos_types::{state_store::state_key::StateKey, write_set::WriteOpSize};
use move_binary_format::{
errors::{Location, PartialVMError, PartialVMResult, VMResult},
file_format::CodeOffset,
Expand Down Expand Up @@ -495,36 +493,6 @@ where
.map_err(|e| e.finish(Location::Undefined))
}

fn storage_fee_for_state_slot(&self, op_size: &WriteOpSize) -> Fee {
self.vm_gas_params().txn.storage_fee_for_slot(op_size)
}

fn storage_fee_refund_for_state_slot(&self, op_size: &WriteOpSize) -> Fee {
self.vm_gas_params()
.txn
.storage_fee_refund_for_slot(op_size)
}

fn storage_fee_for_state_bytes(&self, key: &StateKey, op_size: &WriteOpSize) -> Fee {
self.vm_gas_params().txn.storage_fee_for_bytes(key, op_size)
}

fn storage_fee_per_event(&self, event: &ContractEvent) -> Fee {
self.vm_gas_params().txn.storage_fee_per_event(event)
}

fn storage_discount_for_events(&self, total_cost: Fee) -> Fee {
self.vm_gas_params()
.txn
.storage_discount_for_events(total_cost)
}

fn storage_fee_for_transaction_storage(&self, txn_size: NumBytes) -> Fee {
self.vm_gas_params()
.txn
.storage_fee_for_transaction_storage(txn_size)
}

fn charge_intrinsic_gas_for_transaction(&mut self, txn_size: NumBytes) -> VMResult<()> {
let excess = txn_size
.checked_sub(self.vm_gas_params().txn.large_transaction_cutoff)
Expand Down
71 changes: 25 additions & 46 deletions aptos-move/aptos-gas-meter/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@

use aptos_gas_algebra::{Fee, FeePerGasUnit, Gas, GasExpression, GasScalingFactor, Octa};
use aptos_gas_schedule::VMGasParameters;
use aptos_types::{
contract_event::ContractEvent,
state_store::{state_key::StateKey, state_value::StateValueMetadata},
write_set::WriteOpSize,
use aptos_types::{state_store::state_key::StateKey, write_set::WriteOpSize};
use aptos_vm_types::{
change_set::VMChangeSet,
storage::{
io_pricing::IoPricing,
space_pricing::{ChargeAndRefund, DiskSpacePricing},
},
};
use aptos_vm_types::{change_set::VMChangeSet, storage::io_pricing::IoPricing};
use move_binary_format::errors::{Location, PartialVMResult, VMResult};
use move_core_types::gas_algebra::{InternalGas, InternalGasUnit, NumBytes};
use move_vm_types::gas::GasMeter as MoveGasMeter;
Expand All @@ -26,6 +28,9 @@ pub trait GasAlgebra {
/// Returns the struct containing the storage-specific gas parameters.
fn io_pricing(&self) -> &IoPricing;

/// Returns the disk space pricing strategy.
fn disk_space_pricing(&self) -> &DiskSpacePricing;

/// Returns the current balance, measured in internal gas units.
fn balance_internal(&self) -> InternalGas;

Expand Down Expand Up @@ -108,24 +113,6 @@ pub trait AptosGasMeter: MoveGasMeter {
/// storage costs.
fn charge_io_gas_for_write(&mut self, key: &StateKey, op: &WriteOpSize) -> VMResult<()>;

/// Calculates the storage fee for a state slot allocation.
fn storage_fee_for_state_slot(&self, op: &WriteOpSize) -> Fee;

/// Calculates the storage fee refund for a state slot deallocation.
fn storage_fee_refund_for_state_slot(&self, op: &WriteOpSize) -> Fee;

/// Calculates the storage fee for state byte.
fn storage_fee_for_state_bytes(&self, key: &StateKey, op: &WriteOpSize) -> Fee;

/// Calculates the storage fee for an event.
fn storage_fee_per_event(&self, event: &ContractEvent) -> Fee;

/// Calculates the discount applied to the event storage fees, based on a free quota.
fn storage_discount_for_events(&self, total_cost: Fee) -> Fee;

/// Calculates the storage fee for the transaction.
fn storage_fee_for_transaction_storage(&self, txn_size: NumBytes) -> Fee;

/// Charges the storage fees for writes, events & txn storage in a lump sum, minimizing the
/// loss of precision. Refundable portion of the charge is recorded on the WriteOp itself,
/// due to which mutable references are required on the parameter list wherever proper.
Expand All @@ -152,31 +139,31 @@ pub trait AptosGasMeter: MoveGasMeter {
return Ok(0.into());
}

let pricing = self.disk_space_pricing();
let params = &self.vm_gas_params().txn;

// Calculate the storage fees.
let mut write_fee = Fee::new(0);
let mut total_refund = Fee::new(0);
for (key, op_size, op_creation_metadata) in change_set.write_set_iter_mut() {
let slot_fee = self.storage_fee_for_state_slot(&op_size);
let refund = self.storage_fee_refund_for_state_slot(&op_size);
let bytes_fee = self.storage_fee_for_state_bytes(key, &op_size);

Self::maybe_record_storage_deposit(op_creation_metadata, slot_fee);
for (key, op_size, metadata_opt) in change_set.write_set_iter_mut() {
let ChargeAndRefund { charge, refund } =
pricing.charge_refund_write_op(params, key, &op_size, metadata_opt);
total_refund += refund;

write_fee += slot_fee + bytes_fee;
write_fee += charge;
}

let event_fee = change_set
.events()
.iter()
.fold(Fee::new(0), |acc, (event, _)| {
acc + self.storage_fee_per_event(event)
acc + pricing.storage_fee_per_event(params, event)
});
let event_discount = self.storage_discount_for_events(event_fee);
let event_discount = pricing.storage_discount_for_events(params, event_fee);
let event_net_fee = event_fee
.checked_sub(event_discount)
.expect("discount should always be less than or equal to total amount");
let txn_fee = self.storage_fee_for_transaction_storage(txn_size);
let txn_fee = pricing.storage_fee_for_transaction_storage(params, txn_size);
let fee = write_fee + event_net_fee + txn_fee;

self.charge_storage_fee(fee, gas_unit_price)
Expand All @@ -185,19 +172,6 @@ pub trait AptosGasMeter: MoveGasMeter {
Ok(total_refund)
}

// The slot fee is refundable, we record it on the WriteOp itself and it'll end up in
// the state DB.
fn maybe_record_storage_deposit(
creation_metadata: Option<&mut StateValueMetadata>,
slot_fee: Fee,
) {
if !slot_fee.is_zero() {
if let Some(metadata) = creation_metadata {
metadata.set_deposit(slot_fee.into())
}
}
}

// Below are getters reexported from the gas algebra.
// Gas meter instances should not reimplement these themselves.

Expand All @@ -216,6 +190,11 @@ pub trait AptosGasMeter: MoveGasMeter {
self.algebra().io_pricing()
}

/// Returns the disk space pricing strategy.
fn disk_space_pricing(&self) -> &DiskSpacePricing {
self.algebra().disk_space_pricing()
}

/// Returns the remaining balance, measured in (external) gas units.
///
/// The number should be rounded down when converting from internal to external gas units.
Expand Down
48 changes: 16 additions & 32 deletions aptos-move/aptos-gas-profiling/src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ use crate::log::{
};
use aptos_gas_algebra::{Fee, FeePerGasUnit, InternalGas, NumArgs, NumBytes};
use aptos_gas_meter::AptosGasMeter;
use aptos_types::{
contract_event::ContractEvent, state_store::state_key::StateKey, write_set::WriteOpSize,
};
use aptos_vm_types::change_set::VMChangeSet;
use aptos_types::{state_store::state_key::StateKey, write_set::WriteOpSize};
use aptos_vm_types::{change_set::VMChangeSet, storage::space_pricing::ChargeAndRefund};
use move_binary_format::{
errors::{Location, PartialVMResult, VMResult},
file_format::CodeOffset,
Expand Down Expand Up @@ -467,7 +465,7 @@ fn write_op_type(op: &WriteOpSize) -> WriteOpType {
match op {
O::Creation { .. } => T::Creation,
O::Modification { .. } => T::Modification,
O::Deletion | O::DeletionWithDeposit { .. } => T::Deletion,
O::Deletion => T::Deletion,
}
}

Expand All @@ -479,18 +477,6 @@ where

delegate! {
fn algebra(&self) -> &Self::Algebra;

fn storage_fee_for_state_slot(&self, op: &WriteOpSize) -> Fee;

fn storage_fee_refund_for_state_slot(&self, op: &WriteOpSize) -> Fee;

fn storage_fee_for_state_bytes(&self, key: &StateKey, op: &WriteOpSize) -> Fee;

fn storage_fee_per_event(&self, event: &ContractEvent) -> Fee;

fn storage_discount_for_events(&self, total_cost: Fee) -> Fee;

fn storage_fee_for_transaction_storage(&self, txn_size: NumBytes) -> Fee;
}

delegate_mut! {
Expand Down Expand Up @@ -534,47 +520,45 @@ where
return Ok(0.into());
}

let pricing = self.disk_space_pricing();
let params = &self.vm_gas_params().txn;

// Writes
let mut write_fee = Fee::new(0);
let mut write_set_storage = vec![];
let mut total_refund = Fee::new(0);
for (key, op_size, op_creation_metadata) in change_set.write_set_iter_mut() {
let slot_fee = self.storage_fee_for_state_slot(&op_size);
let slot_refund = self.storage_fee_refund_for_state_slot(&op_size);
let bytes_fee = self.storage_fee_for_state_bytes(key, &op_size);
for (key, op_size, metadata_opt) in change_set.write_set_iter_mut() {
let ChargeAndRefund { charge, refund } =
pricing.charge_refund_write_op(params, key, &op_size, metadata_opt);
write_fee += charge;
total_refund += refund;

Self::maybe_record_storage_deposit(op_creation_metadata, slot_fee);
total_refund += slot_refund;

let fee = slot_fee + bytes_fee;
write_set_storage.push(WriteStorage {
key: key.clone(),
op_type: write_op_type(&op_size),
cost: fee,
refund: slot_refund,
cost: charge,
refund,
});
// TODO(gas): track storage refund in the profiler
write_fee += fee;
}

// Events
let mut event_fee = Fee::new(0);
let mut event_fees = vec![];
for (event, _) in change_set.events().iter() {
let fee = self.storage_fee_per_event(event);
let fee = pricing.storage_fee_per_event(params, event);
event_fees.push(EventStorage {
ty: event.type_tag().clone(),
cost: fee,
});
event_fee += fee;
}
let event_discount = self.storage_discount_for_events(event_fee);
let event_discount = pricing.storage_discount_for_events(params, event_fee);
let event_fee_with_discount = event_fee
.checked_sub(event_discount)
.expect("discount should always be less than or equal to total amount");

// Txn
let txn_fee = self.storage_fee_for_transaction_storage(txn_size);
let txn_fee = pricing.storage_fee_for_transaction_storage(params, txn_size);

self.storage_fees = Some(StorageFees {
total: write_fee + event_fee + txn_fee,
Expand Down
52 changes: 2 additions & 50 deletions aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ use aptos_gas_algebra::{
AbstractValueSize, Fee, FeePerByte, FeePerGasUnit, FeePerSlot, Gas, GasExpression,
GasScalingFactor, GasUnit, NumSlots,
};
use aptos_types::{
contract_event::ContractEvent, state_store::state_key::StateKey, write_set::WriteOpSize,
};
use move_core_types::gas_algebra::{
InternalGas, InternalGasPerArg, InternalGasPerByte, InternalGasUnit, NumBytes, ToUnitWithParams,
};
Expand Down Expand Up @@ -110,8 +107,8 @@ crate::gas_schedule::macros::define_gas_parameters!(
],
[memory_quota: AbstractValueSize, { 1.. => "memory_quota" }, 10_000_000],
[
free_write_bytes_quota: NumBytes,
{ 5.. => "free_write_bytes_quota" },
legacy_free_write_bytes_quota: NumBytes,
{ 5..=11 => "free_write_bytes_quota", 12.. => "legacy_free_write_bytes_quota" },
1024, // 1KB free per state write
],
[
Expand Down Expand Up @@ -194,51 +191,6 @@ impl TransactionGasParameters {
}
}

pub fn storage_fee_for_slot(&self, op_size: &WriteOpSize) -> Fee {
use WriteOpSize::*;

match op_size {
Creation { .. } => self.storage_fee_per_state_slot_create * NumSlots::new(1),
Modification { .. } | Deletion | DeletionWithDeposit { .. } => 0.into(),
}
}

pub fn storage_fee_refund_for_slot(&self, op_size: &WriteOpSize) -> Fee {
op_size.deletion_deposit().map_or(0.into(), Fee::new)
}

/// Maybe value size is None for deletion Ops.
pub fn storage_fee_for_bytes(&self, key: &StateKey, op_size: &WriteOpSize) -> Fee {
if let Some(value_size) = op_size.write_len() {
let size = NumBytes::new(key.size() as u64) + NumBytes::new(value_size);
if let Some(excess) = size.checked_sub(self.free_write_bytes_quota) {
return excess * self.storage_fee_per_excess_state_byte;
}
}

0.into()
}

/// New formula to charge storage fee for an event, measured in APT.
pub fn storage_fee_per_event(&self, event: &ContractEvent) -> Fee {
NumBytes::new(event.size() as u64) * self.storage_fee_per_event_byte
}

pub fn storage_discount_for_events(&self, total_cost: Fee) -> Fee {
std::cmp::min(
total_cost,
self.free_event_bytes_quota * self.storage_fee_per_event_byte,
)
}

/// New formula to charge storage fee for transaction, measured in APT.
pub fn storage_fee_for_transaction_storage(&self, txn_size: NumBytes) -> Fee {
txn_size
.checked_sub(self.large_transaction_cutoff)
.unwrap_or(NumBytes::zero())
* self.storage_fee_per_transaction_byte
}

/// Calculate the intrinsic gas for the transaction based upon its size in bytes.
pub fn calculate_intrinsic_gas(
&self,
Expand Down
Loading
Loading