Skip to content

Commit

Permalink
[aptosvm] charge more for keyless (#13005)
Browse files Browse the repository at this point in the history
* [aptosvm] hack to charge more for keyless

add a 150x multiplier for intrinsic gas for keyless txns
* Swap `min_transaction_gas_units` from `InternalGas` to
  `InternalGasPerByte` -- that has no underlying change but the semantic
  change allows for it to be multiplied
* Add in a multiplier for all intrinsic gas charges which will be 1 by default
* Set a gas schedule of 150 for keyless -- defaults to 150 since this is
  live on testnet already...

* [aptos vm] refactor gas charging for keyless

This reimplements the gas charging for keyless in a more proper way,
decoupling the keyless charge from the exisitng intrinsic charge.

---------

Co-authored-by: Victor Gao <vgao1996@gmail.com>
  • Loading branch information
davidiw and vgao1996 committed Apr 24, 2024
1 parent fc8be49 commit 553f171
Show file tree
Hide file tree
Showing 16 changed files with 146 additions and 66 deletions.
6 changes: 6 additions & 0 deletions aptos-move/aptos-gas-meter/src/meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,4 +554,10 @@ where
.charge_execution(MIN_TRANSACTION_GAS_UNITS + INTRINSIC_GAS_PER_BYTE * excess)
.map_err(|e| e.finish(Location::Undefined))
}

fn charge_keyless(&mut self) -> VMResult<()> {
self.algebra
.charge_execution(KEYLESS_BASE_COST)
.map_err(|e| e.finish(Location::Undefined))
}
}
7 changes: 6 additions & 1 deletion aptos-move/aptos-gas-meter/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,14 @@ pub trait AptosGasMeter: MoveGasMeter {
/// Charges an intrinsic cost for executing the transaction.
///
/// The cost stays constant for transactions below a certain size, but will grow proportionally
/// for bigger ones.
/// for bigger ones. THe multiplier can be used to increase the unit cost for exceptional
/// transactions like keyless.
fn charge_intrinsic_gas_for_transaction(&mut self, txn_size: NumBytes) -> VMResult<()>;

/// Charges an additional cost for keyless transactions to compensate for the
/// expensive computation required.
fn charge_keyless(&mut self) -> VMResult<()>;

/// Charges IO gas for the transaction itself.
fn charge_io_gas_for_transaction(&mut self, txn_size: NumBytes) -> VMResult<()>;

Expand Down
2 changes: 2 additions & 0 deletions aptos-move/aptos-gas-profiling/src/erased.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ impl ExecutionAndIOCosts {

nodes.push(Node::new("intrinsic", self.intrinsic_cost));

nodes.push(Node::new("keyless", self.keyless_cost));

if !self.dependencies.is_empty() {
let deps = Node::new_with_children(
"dependencies",
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/aptos-gas-profiling/src/flamegraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ impl ExecutionAndIOCosts {

lines.push("intrinsic", self.intrinsic_cost);

lines.push("keyless", self.keyless_cost);

let mut path = vec![];

struct Rec<'a> {
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/aptos-gas-profiling/src/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub struct ExecutionAndIOCosts {
pub total: InternalGas,

pub intrinsic_cost: InternalGas,
pub keyless_cost: InternalGas,
pub dependencies: Vec<Dependency>,
pub call_graph: CallFrame,
pub transaction_transient: Option<InternalGas>,
Expand Down Expand Up @@ -238,6 +239,7 @@ impl ExecutionAndIOCosts {
let mut total = InternalGas::zero();

total += self.intrinsic_cost;
total += self.keyless_cost;

for dep in &self.dependencies {
total += dep.cost;
Expand Down
12 changes: 12 additions & 0 deletions aptos-move/aptos-gas-profiling/src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub struct GasProfiler<G> {
base: G,

intrinsic_cost: Option<InternalGas>,
keyless_cost: Option<InternalGas>,
dependencies: Vec<Dependency>,
frames: Vec<CallFrame>,
transaction_transient: Option<InternalGas>,
Expand Down Expand Up @@ -90,6 +91,7 @@ impl<G> GasProfiler<G> {
base,

intrinsic_cost: None,
keyless_cost: None,
dependencies: vec![],
frames: vec![CallFrame::new_script()],
transaction_transient: None,
Expand All @@ -109,6 +111,7 @@ impl<G> GasProfiler<G> {
base,

intrinsic_cost: None,
keyless_cost: None,
dependencies: vec![],
frames: vec![CallFrame::new_function(module_id, func_name, ty_args)],
transaction_transient: None,
Expand Down Expand Up @@ -650,6 +653,14 @@ where

res
}

fn charge_keyless(&mut self) -> VMResult<()> {
let (_cost, res) = self.delegate_charge(|base| base.charge_keyless());

// TODO: add keyless

res
}
}

impl<G> GasProfiler<G>
Expand All @@ -667,6 +678,7 @@ where
gas_scaling_factor: self.base.gas_unit_scaling_factor(),
total: self.algebra().execution_gas_used() + self.algebra().io_gas_used(),
intrinsic_cost: self.intrinsic_cost.unwrap_or_else(|| 0.into()),
keyless_cost: self.keyless_cost.unwrap_or_else(|| 0.into()),
dependencies: self.dependencies,
call_graph: self.frames.pop().expect("frame must exist"),
transaction_transient: self.transaction_transient,
Expand Down
14 changes: 14 additions & 0 deletions aptos-move/aptos-gas-profiling/src/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ impl TransactionGasLog {
data.insert("intrinsic-percentage".to_string(), json!(percentage));
}

// Keyless cost
if !self.exec_io.keyless_cost.is_zero() {
let cost_scaled = format!(
"{:.8}",
(u64::from(self.exec_io.keyless_cost) as f64 / scaling_factor)
);
let percentage = format!(
"{:.2}%",
u64::from(self.exec_io.keyless_cost) as f64 / total_exec_io * 100.0
);
data.insert("keyless".to_string(), json!(cost_scaled));
data.insert("keyless-percentage".to_string(), json!(percentage));
}

let mut deps = self.exec_io.dependencies.clone();
deps.sort_by(|lhs, rhs| rhs.cost.cmp(&lhs.cost));
data.insert(
Expand Down
110 changes: 58 additions & 52 deletions aptos-move/aptos-gas-profiling/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ <h4>Intrinsic Cost</h4>
{{#if intrinsic-percentage}}
, {{intrinsic-percentage}} of the total cost for execution & IO.
{{/if}}

{{#if keyless}}
<h4>Keyless Cost</h4>
{{keyless}} gas units, {{keyless-percentage}} of the total cost for execution & IO.
{{/if}}

<h4>Dependencies</h4>
{{#if deps}}
<table>
Expand Down Expand Up @@ -147,60 +153,60 @@ <h4>State Reads</h4>
{{/if}}
<h4>Ledger Writes</h4>
<h5>Transaction Itself</h5>
<table>
<tr>
<td style="text-align: right"><b>Cost in Gas Units</b></td>
<td style="text-align: right"><b>Percentage</b></td>
</tr>
{{#with transaction_write}}
<tr>
<td style="text-align: right">{{cost}}</td>
<td style="text-align: right">{{percentage}}</td>
</tr>
{{/with}}
</table>
<table>
<tr>
<td style="text-align: right"><b>Cost in Gas Units</b></td>
<td style="text-align: right"><b>Percentage</b></td>
</tr>
{{#with transaction_write}}
<tr>
<td style="text-align: right">{{cost}}</td>
<td style="text-align: right">{{percentage}}</td>
</tr>
{{/with}}
</table>
<h5>Events</h5>
{{#if event_writes}}
<table>
<tr>
<td><b>Event Type</b></td>
<td style="text-align: right"><b>Number of Hits</td>
<td style="text-align: right"><b>Cost in Gas Units</b></td>
<td style="text-align: right"><b>Percentage</b></td>
</tr>
{{#each event_writes}}
<tr>
<td>{{name}}</td>
<td style="text-align: right">{{hits}}</td>
<td style="text-align: right">{{cost}}</td>
<td style="text-align: right">{{percentage}}</td>
</tr>
{{/each}}
</table>
{{else}}
(No writes to show.)
{{/if}}
{{#if event_writes}}
<table>
<tr>
<td><b>Event Type</b></td>
<td style="text-align: right"><b>Number of Hits</td>
<td style="text-align: right"><b>Cost in Gas Units</b></td>
<td style="text-align: right"><b>Percentage</b></td>
</tr>
{{#each event_writes}}
<tr>
<td>{{name}}</td>
<td style="text-align: right">{{hits}}</td>
<td style="text-align: right">{{cost}}</td>
<td style="text-align: right">{{percentage}}</td>
</tr>
{{/each}}
</table>
{{else}}
(No writes to show.)
{{/if}}
<h5>State Write Ops</h5>
{{#if writes}}
<table>
<tr>
<td><b>Resource Name</b></td>
<td style="text-align: right"><b>Number of Hits</td>
<td style="text-align: right"><b>Cost in Gas Units</b></td>
<td style="text-align: right"><b>Percentage</b></td>
</tr>
{{#each writes}}
<tr>
<td>{{name}}</td>
<td style="text-align: right">{{hits}}</td>
<td style="text-align: right">{{cost}}</td>
<td style="text-align: right">{{percentage}}</td>
</tr>
{{/each}}
</table>
{{else}}
(No writes to show.)
{{/if}}
{{#if writes}}
<table>
<tr>
<td><b>Resource Name</b></td>
<td style="text-align: right"><b>Number of Hits</td>
<td style="text-align: right"><b>Cost in Gas Units</b></td>
<td style="text-align: right"><b>Percentage</b></td>
</tr>
{{#each writes}}
<tr>
<td>{{name}}</td>
<td style="text-align: right">{{hits}}</td>
<td style="text-align: right">{{cost}}</td>
<td style="text-align: right">{{percentage}}</td>
</tr>
{{/each}}
</table>
{{else}}
(No writes to show.)
{{/if}}
<h3>Storage</h3>
The storage fees cover the extended-term storage of states and events and are assessed at a fixed price in APT.

Expand Down
6 changes: 6 additions & 0 deletions aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ crate::gas_schedule::macros::define_gas_parameters!(
[
// The flat minimum amount of gas required for any transaction.
// Charged at the start of execution.
// It is variable to charge more for more expensive authenticators, e.g., keyless
[
min_transaction_gas_units: InternalGas,
"min_transaction_gas_units",
Expand Down Expand Up @@ -230,6 +231,11 @@ crate::gas_schedule::macros::define_gas_parameters!(
{ 15.. => "max_total_dependency_size" },
1024 * 1024 * 12 / 10, // 1.2 MB
],
[
keyless_base_cost: InternalGas,
{ 17.. => "keyless.base" },
414_000_000,
]
]
);

Expand Down
4 changes: 3 additions & 1 deletion aptos-move/aptos-gas-schedule/src/ver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
/// - Changing how gas is calculated in any way
///
/// Change log:
/// - V17
/// - Gas for keyless
/// - V16
/// - IO Gas for the transaction itself and events in the transaction output
/// - V15
Expand Down Expand Up @@ -55,4 +57,4 @@
/// global operations.
/// - V1
/// - TBA
pub const LATEST_GAS_FEATURE_VERSION: u64 = 16;
pub const LATEST_GAS_FEATURE_VERSION: u64 = 17;
2 changes: 2 additions & 0 deletions aptos-move/aptos-memory-usage-tracker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,5 +489,7 @@ where
) -> PartialVMResult<()>;

fn charge_intrinsic_gas_for_transaction(&mut self, txn_size: NumBytes) -> VMResult<()>;

fn charge_keyless(&mut self) -> VMResult<()>;
}
}
7 changes: 7 additions & 0 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,9 @@ impl AptosVM {
});

gas_meter.charge_intrinsic_gas_for_transaction(txn_data.transaction_size())?;
if txn_data.is_keyless() {
gas_meter.charge_keyless()?;
}

match payload {
TransactionPayload::Script(script) => {
Expand Down Expand Up @@ -1001,6 +1004,9 @@ impl AptosVM {
});

gas_meter.charge_intrinsic_gas_for_transaction(txn_data.transaction_size())?;
if txn_data.is_keyless() {
gas_meter.charge_keyless()?;
}

// Step 1: Obtain the payload. If any errors happen here, the entire transaction should fail
let invariant_violation_error = || {
Expand Down Expand Up @@ -1568,6 +1574,7 @@ impl AptosVM {
keyless_validation::validate_authenticators(
&authenticators,
self.features(),
self.gas_feature_version,
resolver,
)?;
}
Expand Down
18 changes: 12 additions & 6 deletions aptos-move/aptos-vm/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{move_vm_ext::AptosMoveResolver, transaction_metadata::TransactionMetadata};
use aptos_gas_algebra::GasExpression;
use aptos_gas_algebra::{Gas, GasExpression, InternalGas};
use aptos_gas_schedule::{
AptosGasParameters, FromOnChainGasSchedule, MiscGasParameters, NativeGasParameters,
gas_params::txn::KEYLESS_BASE_COST, AptosGasParameters, FromOnChainGasSchedule,
MiscGasParameters, NativeGasParameters,
};
use aptos_logger::{enabled, Level};
use aptos_types::on_chain_config::{
Expand Down Expand Up @@ -186,17 +187,22 @@ pub(crate) fn check_gas(
// The submitted transactions max gas units needs to be at least enough to cover the
// intrinsic cost of the transaction as calculated against the size of the
// underlying `RawTransaction`.
let keyless = if txn_metadata.is_keyless() {
KEYLESS_BASE_COST.evaluate(gas_feature_version, &gas_params.vm)
} else {
InternalGas::zero()
};
let intrinsic_gas = txn_gas_params
.calculate_intrinsic_gas(raw_bytes_len)
.evaluate(gas_feature_version, &gas_params.vm)
.to_unit_round_up_with_params(txn_gas_params);
.evaluate(gas_feature_version, &gas_params.vm);
let total_rounded: Gas = (intrinsic_gas + keyless).to_unit_round_up_with_params(txn_gas_params);

if txn_metadata.max_gas_amount() < intrinsic_gas {
if txn_metadata.max_gas_amount() < total_rounded {
speculative_warn!(
log_context,
format!(
"[VM] Gas unit error; min {}, submitted {}",
intrinsic_gas,
total_rounded,
txn_metadata.max_gas_amount()
),
);
Expand Down
7 changes: 4 additions & 3 deletions aptos-move/aptos-vm/src/keyless_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,23 +121,24 @@ fn get_jwk_for_authenticator(
pub(crate) fn validate_authenticators(
authenticators: &Vec<(KeylessPublicKey, KeylessSignature)>,
features: &Features,
gas_feature_version: u64,
resolver: &impl AptosMoveResolver,
) -> Result<(), VMStatus> {
for (_, sig) in authenticators {
// Feature-gating for keyless TXNs (whether ZK or ZKless, whether passkey-based or not)
if matches!(sig.cert, EphemeralCertificate::ZeroKnowledgeSig { .. })
&& !features.is_zk_keyless_enabled()
&& !(features.is_zk_keyless_enabled() && gas_feature_version >= 17)
{
return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None));
}

if matches!(sig.cert, EphemeralCertificate::OpenIdSig { .. })
&& !features.is_zkless_keyless_enabled()
&& !(features.is_zkless_keyless_enabled() && gas_feature_version >= 17)
{
return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None));
}
if matches!(sig.ephemeral_signature, EphemeralSignature::WebAuthn { .. })
&& !features.is_keyless_with_passkeys_enabled()
&& !(features.is_keyless_with_passkeys_enabled() && gas_feature_version >= 17)
{
return Err(VMStatus::error(StatusCode::FEATURE_UNDER_GATING, None));
}
Expand Down
Loading

0 comments on commit 553f171

Please sign in to comment.