From a2cecd683bbe68227b07a5d211893f1220a057c4 Mon Sep 17 00:00:00 2001 From: Victor Gao Date: Thu, 21 Dec 2023 11:14:38 +0000 Subject: [PATCH] [gas] add gas charges for dependencies --- ...ansaction_with_entry_function_payload.json | 4 +- ...e_when_start_version_is_not_specified.json | 32 ++-- .../aptos-abstract-gas-usage/src/algebra.rs | 7 +- aptos-move/aptos-gas-algebra/src/algebra.rs | 5 + aptos-move/aptos-gas-meter/src/algebra.rs | 24 ++- aptos-move/aptos-gas-meter/src/meter.rs | 11 ++ aptos-move/aptos-gas-meter/src/traits.rs | 3 + aptos-move/aptos-gas-profiling/src/erased.rs | 20 ++- .../aptos-gas-profiling/src/flamegraph.rs | 4 + aptos-move/aptos-gas-profiling/src/log.rs | 18 +- .../aptos-gas-profiling/src/profiler.rs | 22 ++- aptos-move/aptos-gas-profiling/src/report.rs | 26 +++ .../aptos-gas-profiling/templates/index.html | 21 +++ .../src/gas_schedule/transaction.rs | 22 ++- aptos-move/aptos-gas-schedule/src/ver.rs | 4 +- .../aptos-memory-usage-tracker/src/lib.rs | 2 + aptos-move/aptos-vm/src/aptos_vm.rs | 63 ++++++- .../src/tests/dependencies.data/p1/Move.toml | 5 + .../dependencies.data/p1/sources/test.move | 3 + .../src/tests/dependencies.data/p2/Move.toml | 6 + .../dependencies.data/p2/sources/test.move | 7 + .../src/tests/dependencies.data/p3/Move.toml | 6 + .../dependencies.data/p3/sources/test.move | 7 + aptos-move/e2e-move-tests/src/tests/gas.rs | 66 ++++++- .../src/tests/storage_refund.rs | 4 +- ..._tests__create_account__create_account.exp | 4 +- .../async/move-async-vm/src/async_vm.rs | 3 +- .../move/move-core/types/src/vm_status.rs | 11 +- .../move/move-vm/runtime/src/data_cache.rs | 125 ++++++++++++-- .../move/move-vm/runtime/src/loader/mod.rs | 161 ++++++++++++------ .../move-vm/runtime/src/loader/modules.rs | 8 +- .../move/move-vm/runtime/src/loader/script.rs | 4 +- .../move/move-vm/runtime/src/move_vm.rs | 27 ++- .../move/move-vm/runtime/src/session.rs | 57 +++++-- .../move-vm/test-utils/src/gas_schedule.rs | 4 + third_party/move/move-vm/types/src/gas.rs | 6 + 36 files changed, 670 insertions(+), 132 deletions(-) create mode 100644 aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/Move.toml create mode 100644 aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/sources/test.move create mode 100644 aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/Move.toml create mode 100644 aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/sources/test.move create mode 100644 aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/Move.toml create mode 100644 aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/sources/test.move diff --git a/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_output_user_transaction_with_entry_function_payload.json b/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_output_user_transaction_with_entry_function_payload.json index 7c63230eee6720..f70d2719812075 100644 --- a/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_output_user_transaction_with_entry_function_payload.json +++ b/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_output_user_transaction_with_entry_function_payload.json @@ -114,7 +114,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -280,7 +280,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], diff --git a/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_returns_last_page_when_start_version_is_not_specified.json b/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_returns_last_page_when_start_version_is_not_specified.json index f530527c9ee588..3640e24a65ac83 100644 --- a/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_returns_last_page_when_start_version_is_not_specified.json +++ b/api/goldens/aptos_api__tests__transactions_test__test_get_transactions_returns_last_page_when_start_version_is_not_specified.json @@ -119,7 +119,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -285,7 +285,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -412,7 +412,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -578,7 +578,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -705,7 +705,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -871,7 +871,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -998,7 +998,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -1164,7 +1164,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -1291,7 +1291,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -1457,7 +1457,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -1584,7 +1584,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -1750,7 +1750,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -1877,7 +1877,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -2043,7 +2043,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], @@ -2170,7 +2170,7 @@ "state_change_hash": "", "event_root_hash": "", "state_checkpoint_hash": null, - "gas_used": "4", + "gas_used": "5", "success": true, "vm_status": "Executed successfully", "accumulator_root_hash": "", @@ -2336,7 +2336,7 @@ "io_gas_units": "1", "storage_fee_octas": "0", "storage_fee_refund_octas": "0", - "total_charge_gas_units": "4" + "total_charge_gas_units": "5" } } ], diff --git a/aptos-move/aptos-abstract-gas-usage/src/algebra.rs b/aptos-move/aptos-abstract-gas-usage/src/algebra.rs index b5ae2a0b0586b1..654cb9c97e97f7 100644 --- a/aptos-move/aptos-abstract-gas-usage/src/algebra.rs +++ b/aptos-move/aptos-abstract-gas-usage/src/algebra.rs @@ -2,7 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use aptos_gas_algebra::{ - DynamicExpression, Fee, FeePerGasUnit, GasExpression, InternalGas, InternalGasUnit, Octa, + DynamicExpression, Fee, FeePerGasUnit, GasExpression, InternalGas, InternalGasUnit, NumBytes, + Octa, }; use aptos_gas_meter::GasAlgebra; use aptos_gas_schedule::VMGasParameters; @@ -80,6 +81,10 @@ impl GasAlgebra for CalibrationAlgebra { .charge_storage_fee(abstract_amount, gas_unit_price) } + fn count_dependency(&mut self, size: NumBytes) -> PartialVMResult<()> { + self.base.count_dependency(size) + } + fn execution_gas_used(&self) -> InternalGas { self.base.execution_gas_used() } diff --git a/aptos-move/aptos-gas-algebra/src/algebra.rs b/aptos-move/aptos-gas-algebra/src/algebra.rs index 73f8ea75c6bd10..f4f49faa1186b9 100644 --- a/aptos-move/aptos-gas-algebra/src/algebra.rs +++ b/aptos-move/aptos-gas-algebra/src/algebra.rs @@ -42,6 +42,11 @@ pub type FeePerSlot = GasQuantity>; pub type FeePerByte = GasQuantity>; +/// Unit of module +pub enum Module {} + +pub type NumModules = GasQuantity; + /*************************************************************************************************** * Unit Conversion * diff --git a/aptos-move/aptos-gas-meter/src/algebra.rs b/aptos-move/aptos-gas-meter/src/algebra.rs index ad5085ea07bf13..5a2cda02d76e7a 100644 --- a/aptos-move/aptos-gas-meter/src/algebra.rs +++ b/aptos-move/aptos-gas-meter/src/algebra.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::traits::GasAlgebra; -use aptos_gas_algebra::{Fee, FeePerGasUnit, Gas, GasExpression, Octa}; +use aptos_gas_algebra::{Fee, FeePerGasUnit, Gas, GasExpression, NumBytes, NumModules, Octa}; use aptos_gas_schedule::VMGasParameters; use aptos_logger::error; use aptos_vm_types::storage::{ @@ -32,6 +32,9 @@ pub struct StandardGasAlgebra { storage_fee_in_internal_units: InternalGas, // The storage fee consumed by the storage operations. storage_fee_used: Fee, + + num_dependencies: NumModules, + total_dependency_size: NumBytes, } impl StandardGasAlgebra { @@ -53,6 +56,8 @@ impl StandardGasAlgebra { io_gas_used: 0.into(), storage_fee_in_internal_units: 0.into(), storage_fee_used: 0.into(), + num_dependencies: 0.into(), + total_dependency_size: 0.into(), } } } @@ -234,6 +239,23 @@ impl GasAlgebra for StandardGasAlgebra { Ok(()) } + fn count_dependency(&mut self, size: NumBytes) -> PartialVMResult<()> { + if self.feature_version >= 14 { + self.num_dependencies += 1.into(); + self.total_dependency_size += size; + + if self.num_dependencies > self.vm_gas_params.txn.max_num_dependencies { + return Err(PartialVMError::new(StatusCode::TOO_MANY_DEPENDENCIES)); + } + if self.total_dependency_size > self.vm_gas_params.txn.max_total_dependency_size { + return Err(PartialVMError::new( + StatusCode::TOTAL_DEPENDENCY_SIZE_TOO_BIG, + )); + } + } + Ok(()) + } + fn execution_gas_used(&self) -> InternalGas { self.execution_gas_used } diff --git a/aptos-move/aptos-gas-meter/src/meter.rs b/aptos-move/aptos-gas-meter/src/meter.rs index b9fe99f8e001f7..d9ed742cfc9ca8 100644 --- a/aptos-move/aptos-gas-meter/src/meter.rs +++ b/aptos-move/aptos-gas-meter/src/meter.rs @@ -469,6 +469,17 @@ where self.algebra.charge_execution(cost) } + + #[inline] + fn charge_dependency(&mut self, module_id: &ModuleId, size: NumBytes) -> PartialVMResult<()> { + // TODO: How about 0xA550C18? + if self.feature_version() >= 14 && !module_id.address().is_special() { + self.algebra + .charge_execution(DEPENDENCY_PER_MODULE + DEPENDENCY_PER_BYTE * size)?; + self.algebra.count_dependency(size)?; + } + Ok(()) + } } impl AptosGasMeter for StandardGasMeter diff --git a/aptos-move/aptos-gas-meter/src/traits.rs b/aptos-move/aptos-gas-meter/src/traits.rs index 5a0146516e64cb..e5718af91be741 100644 --- a/aptos-move/aptos-gas-meter/src/traits.rs +++ b/aptos-move/aptos-gas-meter/src/traits.rs @@ -66,6 +66,9 @@ pub trait GasAlgebra { gas_unit_price: FeePerGasUnit, ) -> PartialVMResult<()>; + /// Counts a dependency against the limits. + fn count_dependency(&mut self, size: NumBytes) -> PartialVMResult<()>; + /// Returns the amount of gas used under the execution category. fn execution_gas_used(&self) -> InternalGas; diff --git a/aptos-move/aptos-gas-profiling/src/erased.rs b/aptos-move/aptos-gas-profiling/src/erased.rs index b9a107e3a23f16..3580cdeb2a9e79 100644 --- a/aptos-move/aptos-gas-profiling/src/erased.rs +++ b/aptos-move/aptos-gas-profiling/src/erased.rs @@ -3,8 +3,8 @@ use crate::{ log::{ - CallFrame, EventStorage, ExecutionAndIOCosts, ExecutionGasEvent, StorageFees, WriteStorage, - WriteTransient, + CallFrame, Dependency, EventStorage, ExecutionAndIOCosts, ExecutionGasEvent, StorageFees, + WriteStorage, WriteTransient, }, render::Render, FrameName, TransactionGasLog, @@ -191,12 +191,28 @@ impl WriteTransient { } } +impl Dependency { + fn to_erased(&self) -> Node { + Node::new(format!("{}", Render(&self.id)), self.cost) + } +} + impl ExecutionAndIOCosts { /// Convert the gas log into a type-erased representation. pub fn to_erased(&self) -> TypeErasedExecutionAndIoCosts { let mut nodes = vec![]; nodes.push(Node::new("intrinsic", self.intrinsic_cost)); + + if !self.dependencies.is_empty() { + let deps = Node::new_with_children( + "dependencies", + 0, + self.dependencies.iter().map(|dep| dep.to_erased()), + ); + nodes.push(deps); + } + nodes.push(self.call_graph.to_erased()); let writes = Node::new_with_children( diff --git a/aptos-move/aptos-gas-profiling/src/flamegraph.rs b/aptos-move/aptos-gas-profiling/src/flamegraph.rs index 76753a402ab4d5..52e11864c85186 100644 --- a/aptos-move/aptos-gas-profiling/src/flamegraph.rs +++ b/aptos-move/aptos-gas-profiling/src/flamegraph.rs @@ -108,6 +108,10 @@ impl ExecutionAndIOCosts { path: &'a mut Vec, } + for dep in &self.dependencies { + lines.push(format!("dependencies;{}", Render(&dep.id)), dep.cost) + } + impl<'a> Rec<'a> { fn visit(&mut self, frame: &CallFrame) { self.path.push(format!("{}", frame.name)); diff --git a/aptos-move/aptos-gas-profiling/src/log.rs b/aptos-move/aptos-gas-profiling/src/log.rs index 2bde37a2fe3009..e5d83eb046ffe2 100644 --- a/aptos-move/aptos-gas-profiling/src/log.rs +++ b/aptos-move/aptos-gas-profiling/src/log.rs @@ -1,7 +1,7 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use aptos_gas_algebra::{Fee, GasScalingFactor, InternalGas}; +use aptos_gas_algebra::{Fee, GasScalingFactor, InternalGas, NumBytes}; use aptos_types::state_store::state_key::StateKey; use move_binary_format::{file_format::CodeOffset, file_format_common::Opcodes}; use move_core_types::{ @@ -94,18 +94,28 @@ pub struct EventStorage { pub cost: Fee, } +#[derive(Debug, Clone)] +/// Struct representing the cost of a dependency. +pub struct Dependency { + pub id: ModuleId, + pub size: NumBytes, + pub cost: InternalGas, +} + +/// Struct containing all execution and io costs. #[derive(Debug)] pub struct ExecutionAndIOCosts { pub gas_scaling_factor: GasScalingFactor, pub total: InternalGas, pub intrinsic_cost: InternalGas, + pub dependencies: Vec, pub call_graph: CallFrame, pub write_set_transient: Vec, } #[derive(Debug)] -// Struct containing all types of storage fees. +/// Struct containing all types of storage fees. pub struct StorageFees { pub total: Fee, pub total_refund: Fee, @@ -219,6 +229,10 @@ impl ExecutionAndIOCosts { total += self.intrinsic_cost; + for dep in &self.dependencies { + total += dep.cost; + } + for op in self.gas_events() { match op { Loc(..) | Call(..) => (), diff --git a/aptos-move/aptos-gas-profiling/src/profiler.rs b/aptos-move/aptos-gas-profiling/src/profiler.rs index 7b833d922aec9e..bdd130985d9f07 100644 --- a/aptos-move/aptos-gas-profiling/src/profiler.rs +++ b/aptos-move/aptos-gas-profiling/src/profiler.rs @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::log::{ - CallFrame, EventStorage, ExecutionAndIOCosts, ExecutionGasEvent, FrameName, StorageFees, - TransactionGasLog, WriteOpType, WriteStorage, WriteTransient, + CallFrame, Dependency, EventStorage, ExecutionAndIOCosts, ExecutionGasEvent, FrameName, + StorageFees, TransactionGasLog, WriteOpType, WriteStorage, WriteTransient, }; use aptos_gas_algebra::{Fee, FeePerGasUnit, InternalGas, NumArgs, NumBytes, NumTypeNodes}; use aptos_gas_meter::{AptosGasMeter, GasAlgebra}; @@ -32,6 +32,7 @@ pub struct GasProfiler { base: G, intrinsic_cost: Option, + dependencies: Vec, frames: Vec, write_set_transient: Vec, storage_fees: Option, @@ -85,6 +86,7 @@ impl GasProfiler { base, intrinsic_cost: None, + dependencies: vec![], frames: vec![CallFrame::new_script()], write_set_transient: vec![], storage_fees: None, @@ -101,6 +103,7 @@ impl GasProfiler { base, intrinsic_cost: None, + dependencies: vec![], frames: vec![CallFrame::new_function(module_id, func_name, ty_args)], write_set_transient: vec![], storage_fees: None, @@ -466,6 +469,20 @@ where res } + + fn charge_dependency(&mut self, module_id: &ModuleId, size: NumBytes) -> PartialVMResult<()> { + let (cost, res) = self.delegate_charge(|base| base.charge_dependency(module_id, size)); + + if !cost.is_zero() { + self.dependencies.push(Dependency { + id: module_id.clone(), + size, + cost, + }); + } + + res + } } fn write_op_type(op: &WriteOpSize) -> WriteOpType { @@ -615,6 +632,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()), + dependencies: self.dependencies, call_graph: self.frames.pop().expect("frame must exist"), write_set_transient: self.write_set_transient, }; diff --git a/aptos-move/aptos-gas-profiling/src/report.rs b/aptos-move/aptos-gas-profiling/src/report.rs index 94efa3c8c47195..f6f4341e595864 100644 --- a/aptos-move/aptos-gas-profiling/src/report.rs +++ b/aptos-move/aptos-gas-profiling/src/report.rs @@ -109,6 +109,32 @@ impl TransactionGasLog { data.insert("intrinsic-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( + "deps".to_string(), + Value::Array( + deps.iter() + .map(|dep| { + let name = format!("{}", Render(&dep.id)); + let cost_scaled = + format!("{:.8}", (u64::from(dep.cost) as f64 / scaling_factor)); + let cost_scaled = + crate::misc::strip_trailing_zeros_and_decimal_point(&cost_scaled); + let percentage = + format!("{:.2}%", u64::from(dep.cost) as f64 / total_exec_io * 100.0); + + json!({ + "name": name, + "size": u64::from(dep.size), + "cost": cost_scaled, + "percentage": percentage, + }) + }) + .collect(), + ), + ); + // Execution & IO (aggregated) let aggregated: crate::aggregate::AggregatedExecutionGasEvents = self.exec_io.aggregate_gas_events(); diff --git a/aptos-move/aptos-gas-profiling/templates/index.html b/aptos-move/aptos-gas-profiling/templates/index.html index 74f4b7eae0be8c..f1f4929a944f95 100644 --- a/aptos-move/aptos-gas-profiling/templates/index.html +++ b/aptos-move/aptos-gas-profiling/templates/index.html @@ -82,6 +82,27 @@

Intrinsic Cost

{{#if intrinsic-percentage}} , {{intrinsic-percentage}} of the total cost for execution & IO. {{/if}} +

Dependencies

+ {{#if deps}} + + + + + + + + {{#each deps}} + + + + + + + {{/each}} +
NameSize in BytesCost in Gas UnitsPercentage
{{name}}{{size}}{{cost}}{{percentage}}
+ {{else}} + (No dependencies to show. System dependencies are excluded.) + {{/if}}

Execution

{{#if ops}} diff --git a/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs b/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs index cdc53b4a356ccc..f0b7bc6c3ea234 100644 --- a/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs +++ b/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs @@ -7,7 +7,7 @@ use crate::gas_schedule::VMGasParameters; use aptos_gas_algebra::{ AbstractValueSize, Fee, FeePerByte, FeePerGasUnit, FeePerSlot, Gas, GasExpression, - GasScalingFactor, GasUnit, NumSlots, + GasScalingFactor, GasUnit, NumModules, NumSlots, }; use move_core_types::gas_algebra::{ InternalGas, InternalGasPerArg, InternalGasPerByte, InternalGasUnit, NumBytes, ToUnitWithParams, @@ -200,6 +200,26 @@ crate::gas_schedule::macros::define_gas_parameters!( { 7.. => "max_storage_fee" }, 2_0000_0000, // 2 APT ], + [ + dependency_per_module: InternalGas, + { 14.. => "dependency_per_module" }, + 4_000, + ], + [ + dependency_per_byte: InternalGasPerByte, + { 14.. => "dependency_per_byte" }, + 100, + ], + [ + max_num_dependencies: NumModules, + { 14.. => "max_num_dependencies" }, + 420, + ], + [ + max_total_dependency_size: NumBytes, + { 14.. => "max_total_dependency_size" }, + 1024 * 1024 * 12 / 10, // 1.2 MB + ], ] ); diff --git a/aptos-move/aptos-gas-schedule/src/ver.rs b/aptos-move/aptos-gas-schedule/src/ver.rs index d8dd1bddaa1df8..f4249c701290ef 100644 --- a/aptos-move/aptos-gas-schedule/src/ver.rs +++ b/aptos-move/aptos-gas-schedule/src/ver.rs @@ -8,6 +8,8 @@ /// - Changing how gas is calculated in any way /// /// Change log: +/// - V14 +/// - Gas & limits for dependencies /// - V13 /// - Storage Fee: Make state bytes refundable and remove the per slot free quota, gated by flag REFUNDABLE_BYTES /// - V12 @@ -47,4 +49,4 @@ /// global operations. /// - V1 /// - TBA -pub const LATEST_GAS_FEATURE_VERSION: u64 = 13; +pub const LATEST_GAS_FEATURE_VERSION: u64 = 14; diff --git a/aptos-move/aptos-memory-usage-tracker/src/lib.rs b/aptos-move/aptos-memory-usage-tracker/src/lib.rs index 8b4a4b7e353949..3f7ef0a9a19195 100644 --- a/aptos-move/aptos-memory-usage-tracker/src/lib.rs +++ b/aptos-move/aptos-memory-usage-tracker/src/lib.rs @@ -160,6 +160,8 @@ where fn charge_vec_swap(&mut self, ty: impl TypeView) -> PartialVMResult<()>; fn charge_create_ty(&mut self, num_nodes: NumTypeNodes) -> PartialVMResult<()>; + + fn charge_dependency(&mut self, module_id: &ModuleId, size: NumBytes) -> PartialVMResult<()>; } #[inline] diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index c1e4d684338eb5..e95c08926685ce 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -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, GasQuantity, Octa}; +use aptos_gas_algebra::{Gas, GasQuantity, NumBytes, Octa}; use aptos_gas_meter::{AptosGasMeter, GasAlgebra, StandardGasAlgebra, StandardGasMeter}; use aptos_gas_schedule::{AptosGasParameters, VMGasParameters}; use aptos_logger::{enabled, prelude::*, Level}; @@ -657,6 +657,13 @@ impl AptosVM { senders: Vec, script_fn: &EntryFunction, ) -> Result { + // Note: Feature gating is needed here because the traversal of the dependencies could + // result in shallow-loading of the modules and therefore subtle changes in + // the error semantics. + if self.gas_feature_version >= 14 { + session.check_dependencies_and_charge_gas(gas_meter, [script_fn.module().clone()])?; + } + let function = session.load_function( script_fn.module(), script_fn.function(), @@ -704,6 +711,14 @@ impl AptosVM { match payload { TransactionPayload::Script(script) => { + // Note: Feature gating is needed here because the traversal of the dependencies could + // result in shallow-loading of the modules and therefore subtle changes in + // the error semantics. + if self.gas_feature_version >= 14 { + session + .check_script_dependencies_and_check_gas(gas_meter, script.code())?; + } + let loaded_func = session.load_script(script.code(), script.ty_args().to_vec())?; // Gerardo: consolidate the extended validation to verifier. @@ -1002,6 +1017,13 @@ impl AptosVM { payload: &EntryFunction, new_published_modules_loaded: &mut bool, ) -> Result<(), VMStatus> { + // Note: Feature gating is needed here because the traversal of the dependencies could + // result in shallow-loading of the modules and therefore subtle changes in + // the error semantics. + if self.gas_feature_version >= 14 { + session.check_dependencies_and_charge_gas(gas_meter, [payload.module().clone()])?; + } + // If txn args are not valid, we'd still consider the transaction as executed but // failed. This is primarily because it's unrecoverable at this point. self.validate_and_execute_entry_function( @@ -1182,6 +1204,45 @@ impl AptosVM { // directly pass CompiledModule. let modules = self.deserialize_module_bundle(&bundle)?; + // Note: Feature gating is needed here because the traversal of the dependencies could + // result in shallow-loading of the modules and therefore subtle changes in + // the error semantics. + if self.gas_feature_version >= 14 { + // Charge all modules in the bundle that is about to be published. + for (module, blob) in modules.iter().zip(bundle.iter()) { + gas_meter + .charge_dependency( + &module.self_id(), + NumBytes::new(blob.code().len() as u64), + ) + .map_err(|err| err.finish(Location::Undefined))?; + } + + // Charge all dependencies. + // + // Must exclude the ones that are in the current bundle because they have not + // been published yet. + let module_ids_in_bundle = modules + .iter() + .map(|module| module.self_id()) + .collect::>(); + + session.check_dependencies_and_charge_gas( + gas_meter, + modules + .iter() + .flat_map(|module| { + module + .immediate_dependencies() + .into_iter() + .chain(module.immediate_friends()) + }) + .filter(|module_id| !module_ids_in_bundle.contains(module_id)), + )?; + + // TODO: Revisit the order of traversal. Consider switching to alphabetical order. + } + // Validate the module bundle self.validate_publish_request(session, &modules, expected_modules, allowed_deps)?; diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/Move.toml b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/Move.toml new file mode 100644 index 00000000000000..1e9dfd9af4ae2a --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/Move.toml @@ -0,0 +1,5 @@ +[package] +name = "p1" +version = "0.0.0" + +[dependencies] diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/sources/test.move b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/sources/test.move new file mode 100644 index 00000000000000..a53c5d4a63c44d --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p1/sources/test.move @@ -0,0 +1,3 @@ +module 0xcafe::m1 { + public entry fun run() {} +} diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/Move.toml b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/Move.toml new file mode 100644 index 00000000000000..fac3a9cf0b9adf --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "p2" +version = "0.0.0" + +[dependencies] +p1 = { local = "../p1" } \ No newline at end of file diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/sources/test.move b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/sources/test.move new file mode 100644 index 00000000000000..a462b34008b12b --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p2/sources/test.move @@ -0,0 +1,7 @@ +module 0xcafe::m2 { + use 0xcafe::m1; + + public entry fun run() { + m1::run(); + } +} diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/Move.toml b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/Move.toml new file mode 100644 index 00000000000000..38fa51a1497470 --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "p3" +version = "0.0.0" + +[dependencies] +p2 = { local = "../p2" } \ No newline at end of file diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/sources/test.move b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/sources/test.move new file mode 100644 index 00000000000000..084633cfe0cfcc --- /dev/null +++ b/aptos-move/e2e-move-tests/src/tests/dependencies.data/p3/sources/test.move @@ -0,0 +1,7 @@ +module 0xcafe::m3 { + use 0xcafe::m2; + + public entry fun run() { + m2::run(); + } +} diff --git a/aptos-move/e2e-move-tests/src/tests/gas.rs b/aptos-move/e2e-move-tests/src/tests/gas.rs index 6a0697af4152d6..3b542a2614c97a 100644 --- a/aptos-move/e2e-move-tests/src/tests/gas.rs +++ b/aptos-move/e2e-move-tests/src/tests/gas.rs @@ -14,8 +14,12 @@ use crate::{ use aptos_cached_packages::{aptos_stdlib, aptos_token_sdk_builder}; use aptos_crypto::{bls12381, PrivateKey, Uniform}; use aptos_gas_profiling::TransactionGasLog; -use aptos_types::account_address::{default_stake_pool_address, AccountAddress}; +use aptos_types::{ + account_address::{default_stake_pool_address, AccountAddress}, + transaction::{EntryFunction, TransactionPayload}, +}; use aptos_vm::AptosVM; +use move_core_types::{identifier::Identifier, language_storage::ModuleId}; use std::path::Path; fn save_profiling_results(name: &str, log: &TransactionGasLog) { @@ -332,6 +336,66 @@ fn test_gas() { publisher, &test_dir_path("code_publishing.data/pack_large_upgrade"), ); + publish( + &mut harness, + "PublishDependencyChain-1", + publisher, + &test_dir_path("dependencies.data/p1"), + ); + publish( + &mut harness, + "PublishDependencyChain-2", + publisher, + &test_dir_path("dependencies.data/p2"), + ); + publish( + &mut harness, + "PublishDependencyChain-3", + publisher, + &test_dir_path("dependencies.data/p3"), + ); + run( + &mut harness, + "UseDependencyChain-1", + publisher, + TransactionPayload::EntryFunction(EntryFunction::new( + ModuleId::new( + AccountAddress::from_hex_literal("0xcafe").unwrap(), + Identifier::new("m1").unwrap(), + ), + Identifier::new("run").unwrap(), + vec![], + vec![], + )), + ); + run( + &mut harness, + "UseDependencyChain-2", + publisher, + TransactionPayload::EntryFunction(EntryFunction::new( + ModuleId::new( + AccountAddress::from_hex_literal("0xcafe").unwrap(), + Identifier::new("m2").unwrap(), + ), + Identifier::new("run").unwrap(), + vec![], + vec![], + )), + ); + run( + &mut harness, + "UseDependencyChain-3", + publisher, + TransactionPayload::EntryFunction(EntryFunction::new( + ModuleId::new( + AccountAddress::from_hex_literal("0xcafe").unwrap(), + Identifier::new("m3").unwrap(), + ), + Identifier::new("run").unwrap(), + vec![], + vec![], + )), + ); } fn dollar_cost(gas_units: u64, price: u64) -> f64 { diff --git a/aptos-move/e2e-move-tests/src/tests/storage_refund.rs b/aptos-move/e2e-move-tests/src/tests/storage_refund.rs index de40ddf9f7860a..c0925645e88ea7 100644 --- a/aptos-move/e2e-move-tests/src/tests/storage_refund.rs +++ b/aptos-move/e2e-move-tests/src/tests/storage_refund.rs @@ -24,8 +24,8 @@ fn test_refunds() { vec![], ); // Note: This test uses a lot of execution gas so we need to bump the limit in order for it - // to pass. -h.modify_gas_schedule(|params| { + // to pass. + h.modify_gas_schedule(|params| { params.vm.txn.max_execution_gas = 40_000_000_000.into(); params.vm.txn.storage_fee_per_state_byte = 0.into(); // tested in DiskSpacePricing. }); diff --git a/aptos-move/e2e-tests/goldens/language_e2e_testsuite__tests__create_account__create_account.exp b/aptos-move/e2e-tests/goldens/language_e2e_testsuite__tests__create_account__create_account.exp index a8a3c51458f67a..a0dc53ff7e01d3 100644 --- a/aptos-move/e2e-tests/goldens/language_e2e_testsuite__tests__create_account__create_account.exp +++ b/aptos-move/e2e-tests/goldens/language_e2e_testsuite__tests__create_account__create_account.exp @@ -23,9 +23,9 @@ Ok( ), events: [ ContractEvent { key: EventKey { creation_number: 0, account_address: f5b9d6f01a99e74c790e2f330c092fa05455a8193f1dfc1b113ecc54d067afe1 }, index: 0, type: Struct(StructTag { address: 0000000000000000000000000000000000000000000000000000000000000001, module: Identifier("account"), name: Identifier("CoinRegisterEvent"), type_params: [] }), event_data: "00000000000000000000000000000000000000000000000000000000000000010a6170746f735f636f696e094170746f73436f696e" }, - ModuleEvent { type: Struct(StructTag { address: 0000000000000000000000000000000000000000000000000000000000000001, module: Identifier("transaction_fee"), name: Identifier("FeeStatement"), type_params: [] }), event_data: "04000000000000000400000000000000010000000000000000000000000000000000000000000000" }, + ModuleEvent { type: Struct(StructTag { address: 0000000000000000000000000000000000000000000000000000000000000001, module: Identifier("transaction_fee"), name: Identifier("FeeStatement"), type_params: [] }), event_data: "05000000000000000400000000000000010000000000000000000000000000000000000000000000" }, ], - gas_used: 4, + gas_used: 5, status: Keep( Success, ), diff --git a/third_party/move/extensions/async/move-async-vm/src/async_vm.rs b/third_party/move/extensions/async/move-async-vm/src/async_vm.rs index 532f88d39e7706..cfebc3fbbaa543 100644 --- a/third_party/move/extensions/async/move-async-vm/src/async_vm.rs +++ b/third_party/move/extensions/async/move-async-vm/src/async_vm.rs @@ -340,7 +340,8 @@ impl<'r, 'l> AsyncSession<'r, 'l> { } } - fn to_bcs(&self, value: Value, tag: &TypeTag) -> PartialVMResult> { + #[allow(clippy::wrong_self_convention)] + fn to_bcs(&mut self, value: Value, tag: &TypeTag) -> PartialVMResult> { let type_layout = self .vm_session .get_type_layout(tag) diff --git a/third_party/move/move-core/types/src/vm_status.rs b/third_party/move/move-core/types/src/vm_status.rs index e27ba2e28a3e9f..f0067ab3c2f874 100644 --- a/third_party/move/move-core/types/src/vm_status.rs +++ b/third_party/move/move-core/types/src/vm_status.rs @@ -703,10 +703,13 @@ pub enum StatusCode { // Reserved error code for future use TOO_MANY_BACK_EDGES = 1122, EVENT_METADATA_VALIDATION_ERROR = 1123, - RESERVED_VERIFICATION_ERROR_2 = 1124, - RESERVED_VERIFICATION_ERROR_3 = 1125, - RESERVED_VERIFICATION_ERROR_4 = 1126, - RESERVED_VERIFICATION_ERROR_5 = 1127, + TOO_MANY_DEPENDENCIES = 1124, + TOTAL_DEPENDENCY_SIZE_TOO_BIG = 1125, + RESERVED_VERIFICATION_ERROR_1 = 1126, + RESERVED_VERIFICATION_ERROR_2 = 1127, + RESERVED_VERIFICATION_ERROR_3 = 1128, + RESERVED_VERIFICATION_ERROR_4 = 1129, + RESERVED_VERIFICATION_ERROR_5 = 1130, // These are errors that the VM might raise if a violation of internal // invariants takes place. diff --git a/third_party/move/move-vm/runtime/src/data_cache.rs b/third_party/move/move-vm/runtime/src/data_cache.rs index 9ed368dc490e38..10a582ec3e60ed 100644 --- a/third_party/move/move-vm/runtime/src/data_cache.rs +++ b/third_party/move/move-vm/runtime/src/data_cache.rs @@ -2,9 +2,16 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use crate::loader::{Loader, ModuleStorageAdapter}; +use crate::{ + loader::{Loader, ModuleStorageAdapter}, + logging::expect_no_verification_errors, +}; use bytes::Bytes; -use move_binary_format::errors::*; +use move_binary_format::{ + deserializer::DeserializerConfig, + errors::*, + file_format::{CompiledModule, CompiledScript}, +}; use move_core_types::{ account_address::AccountAddress, effects::{AccountChanges, ChangeSet, Changes, Op}, @@ -21,7 +28,10 @@ use move_vm_types::{ value_serde::deserialize_and_allow_delayed_values, values::{GlobalValue, Value}, }; -use std::collections::btree_map::BTreeMap; +use std::{ + collections::btree_map::{self, BTreeMap}, + sync::Arc, +}; pub struct AccountDataCache { // The bool flag in the `data_map` indicates whether the resource contains @@ -39,6 +49,27 @@ impl AccountDataCache { } } +fn load_module_impl( + remote: &dyn MoveResolver, + account_map: &BTreeMap, + module_id: &ModuleId, +) -> PartialVMResult { + if let Some(account_cache) = account_map.get(module_id.address()) { + if let Some((blob, _is_republishing)) = account_cache.module_map.get(module_id.name()) { + return Ok(blob.clone()); + } + } + match remote.get_module(module_id)? { + Some(bytes) => Ok(bytes), + None => Err( + PartialVMError::new(StatusCode::LINKER_ERROR).with_message(format!( + "Linker Error: Cannot find {:?} in data cache", + module_id + )), + ), + } +} + /// Transaction data cache. Keep updates within a transaction so they can all be published at /// once when the transaction succeeds. /// @@ -55,15 +86,27 @@ impl AccountDataCache { pub(crate) struct TransactionDataCache<'r> { remote: &'r dyn MoveResolver, account_map: BTreeMap, + + deserializer_config: DeserializerConfig, + + // Caches to help avoid duplicate deserialization calls. + compiled_scripts: BTreeMap<[u8; 32], Arc>, + compiled_modules: BTreeMap, usize)>, } impl<'r> TransactionDataCache<'r> { /// Create a `TransactionDataCache` with a `RemoteCache` that provides access to data /// not updated in the transaction. - pub(crate) fn new(remote: &'r impl MoveResolver) -> Self { + pub(crate) fn new( + deserializer_config: DeserializerConfig, + remote: &'r impl MoveResolver, + ) -> Self { TransactionDataCache { remote, account_map: BTreeMap::new(), + deserializer_config, + compiled_scripts: BTreeMap::new(), + compiled_modules: BTreeMap::new(), } } @@ -239,19 +282,69 @@ impl<'r> TransactionDataCache<'r> { } pub(crate) fn load_module(&self, module_id: &ModuleId) -> PartialVMResult { - if let Some(account_cache) = self.account_map.get(module_id.address()) { - if let Some((blob, _is_republishing)) = account_cache.module_map.get(module_id.name()) { - return Ok(blob.clone()); - } + load_module_impl(self.remote, &self.account_map, module_id) + } + + pub(crate) fn load_compiled_script_to_cache( + &mut self, + script_blob: &[u8], + hash_value: [u8; 32], + ) -> VMResult> { + let cache = &mut self.compiled_scripts; + match cache.entry(hash_value) { + btree_map::Entry::Occupied(entry) => Ok(entry.get().clone()), + btree_map::Entry::Vacant(entry) => { + let script = match CompiledScript::deserialize_with_config( + script_blob, + &self.deserializer_config, + ) { + Ok(script) => script, + Err(err) => { + let msg = format!("[VM] deserializer for script returned error: {:?}", err); + return Err(PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) + .with_message(msg) + .finish(Location::Script)); + }, + }; + Ok(entry.insert(Arc::new(script)).clone()) + }, } - match self.remote.get_module(module_id)? { - Some(bytes) => Ok(bytes), - None => Err( - PartialVMError::new(StatusCode::LINKER_ERROR).with_message(format!( - "Linker Error: Cannot find {:?} in data cache", - module_id - )), - ), + } + + pub(crate) fn load_compiled_module_to_cache( + &mut self, + id: ModuleId, + allow_loading_failure: bool, + ) -> VMResult<(Arc, usize)> { + let cache = &mut self.compiled_modules; + match cache.entry(id) { + btree_map::Entry::Occupied(entry) => Ok(entry.get().clone()), + btree_map::Entry::Vacant(entry) => { + // bytes fetching, allow loading to fail if the flag is set + let bytes = match load_module_impl(self.remote, &self.account_map, entry.key()) + .map_err(|err| err.finish(Location::Undefined)) + { + Ok(bytes) => bytes, + Err(err) if allow_loading_failure => return Err(err), + Err(err) => { + return Err(expect_no_verification_errors(err)); + }, + }; + + // for bytes obtained from the data store, they should always deserialize and verify. + // It is an invariant violation if they don't. + let module = + CompiledModule::deserialize_with_config(&bytes, &self.deserializer_config) + .map_err(|err| { + let msg = format!("Deserialization error: {:?}", err); + PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) + .with_message(msg) + .finish(Location::Module(entry.key().clone())) + }) + .map_err(expect_no_verification_errors)?; + + Ok(entry.insert((Arc::new(module), bytes.len())).clone()) + }, } } diff --git a/third_party/move/move-vm/runtime/src/loader/mod.rs b/third_party/move/move-vm/runtime/src/loader/mod.rs index 185779a510d32a..325e1246cd31b7 100644 --- a/third_party/move/move-vm/runtime/src/loader/mod.rs +++ b/third_party/move/move-vm/runtime/src/loader/mod.rs @@ -20,7 +20,7 @@ use move_binary_format::{ use move_bytecode_verifier::{self, cyclic_dependencies, dependencies}; use move_core_types::{ account_address::AccountAddress, - gas_algebra::NumTypeNodes, + gas_algebra::{NumBytes, NumTypeNodes}, ident_str, identifier::IdentStr, language_storage::{ModuleId, StructTag, TypeTag}, @@ -232,6 +232,28 @@ impl Loader { // Script verification and loading // + pub(crate) fn check_script_dependencies_and_check_gas( + &self, + data_store: &mut TransactionDataCache, + gas_meter: &mut impl GasMeter, + script_blob: &[u8], + ) -> VMResult<()> { + let mut sha3_256 = Sha3_256::new(); + sha3_256.update(script_blob); + let hash_value: [u8; 32] = sha3_256.finalize().into(); + + let script = data_store.load_compiled_script_to_cache(script_blob, hash_value)?; + + // TODO(Gas): the script it self? + self.check_dependencies_and_charge_gas( + data_store, + gas_meter, + script.immediate_dependencies(), + )?; + + Ok(()) + } + // Scripts are verified and dependencies are loaded. // Effectively that means modules are cached from leaf to root in the dependency DAG. // If a dependency error is found, loading stops and the error is returned. @@ -244,7 +266,7 @@ impl Loader { &self, script_blob: &[u8], ty_args: &[TypeTag], - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult<(Arc, LoadedFunctionInstantiation)> { // retrieve or load the script @@ -256,8 +278,12 @@ impl Loader { let (main, parameters, return_) = match scripts.get(&hash_value) { Some(cached) => cached, None => { - let ver_script = - self.deserialize_and_verify_script(script_blob, data_store, module_store)?; + let ver_script = self.deserialize_and_verify_script( + script_blob, + hash_value, + data_store, + module_store, + )?; let script = Script::new(ver_script, &hash_value, module_store, &self.name_cache)?; scripts.insert(hash_value, script) }, @@ -298,21 +324,11 @@ impl Loader { fn deserialize_and_verify_script( &self, script: &[u8], - data_store: &TransactionDataCache, + hash_value: [u8; 32], + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, - ) -> VMResult { - let script = match CompiledScript::deserialize_with_config( - script, - &self.vm_config.deserializer_config, - ) { - Ok(script) => script, - Err(err) => { - let msg = format!("[VM] deserializer for script returned error: {:?}", err); - return Err(PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) - .with_message(msg) - .finish(Location::Script)); - }, - }; + ) -> VMResult> { + let script = data_store.load_compiled_script_to_cache(script, hash_value)?; match self.verify_script(&script) { Ok(_) => { @@ -358,7 +374,7 @@ impl Loader { &self, module_id: &ModuleId, function_name: &IdentStr, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult<(Arc, Arc, Vec, Vec)> { let module = self.load_module(module_id, data_store, module_store)?; @@ -465,7 +481,7 @@ impl Loader { module_id: &ModuleId, function_name: &IdentStr, expected_return_type: &Type, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult<(LoadedFunction, LoadedFunctionInstantiation)> { let (module, func, parameters, return_vec) = self.load_function_without_type_args( @@ -532,7 +548,7 @@ impl Loader { module_id: &ModuleId, function_name: &IdentStr, ty_args: &[TypeTag], - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult<(Arc, Arc, LoadedFunctionInstantiation)> { let (module, func, parameters, return_) = self.load_function_without_type_args( @@ -612,7 +628,7 @@ impl Loader { module: &CompiledModule, bundle_verified: &BTreeMap, bundle_unverified: &BTreeSet, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult<()> { // Performs all verification steps to load the module without loading it, i.e., the new @@ -732,7 +748,7 @@ impl Loader { pub(crate) fn load_type( &self, type_tag: &TypeTag, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult { Ok(match type_tag { @@ -782,12 +798,66 @@ impl Loader { }) } + /// Traverses the whole transitive closure of dependencies, starting from the specified + /// modules and perform gas metering. + /// + /// The traversal follows a depth-first order, with the module itself being visited first, + /// followed by its dependencies, and finally its friends. + /// Changing this order without a valid reason may affect the gas semantics and must be + /// avoided. + /// + /// This will result in the shallow-loading of the modules -- they will be read from the + /// storage as bytes and then deserialized, but NOT converted into the runtime representation. + /// + /// TODO: Revisit the order of traversal. Consider switching to alphabetical order. + pub(crate) fn check_dependencies_and_charge_gas( + &self, + data_store: &mut TransactionDataCache, + gas_meter: &mut impl GasMeter, + ids: I, + ) -> VMResult<()> + where + I: IntoIterator, + I::IntoIter: DoubleEndedIterator, + { + let mut visited = BTreeSet::new(); + let mut stack = ids + .into_iter() + .rev() + .map(|id| (id, true)) + .collect::>(); + + while let Some((id, allow_loading_failure)) = stack.pop() { + if !visited.insert(id.clone()) { + continue; + } + + let (module, size) = + data_store.load_compiled_module_to_cache(id.clone(), allow_loading_failure)?; + + gas_meter + .charge_dependency(&id, NumBytes::new(size as u64)) + .map_err(|err| err.finish(Location::Module(id.clone())))?; + + stack.extend( + module + .immediate_dependencies() + .into_iter() + .chain(module.immediate_friends()) + .map(|id| (id, false)) + .rev(), + ); + } + + Ok(()) + } + // The interface for module loading. Aligned with `load_type` and `load_function`, this function // verifies that the module is OK instead of expect it. pub(crate) fn load_module( &self, id: &ModuleId, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult> { self.load_module_internal(id, data_store, module_store) @@ -798,7 +868,7 @@ impl Loader { fn load_module_internal( &self, id: &ModuleId, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, ) -> VMResult> { // if the module is already in the code cache, load the cached version @@ -833,32 +903,11 @@ impl Loader { fn load_and_verify_module( &self, id: &ModuleId, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, allow_loading_failure: bool, - ) -> VMResult { - // bytes fetching, allow loading to fail if the flag is set - let bytes = match data_store - .load_module(id) - .map_err(|e| e.finish(Location::Undefined)) - { - Ok(bytes) => bytes, - Err(err) if allow_loading_failure => return Err(err), - Err(err) => { - return Err(expect_no_verification_errors(err)); - }, - }; - - // for bytes obtained from the data store, they should always deserialize and verify. - // It is an invariant violation if they don't. - let module = - CompiledModule::deserialize_with_config(&bytes, &self.vm_config.deserializer_config) - .map_err(|err| { - let msg = format!("Deserialization error: {:?}", err); - PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) - .with_message(msg) - .finish(Location::Module(id.clone())) - }) - .map_err(expect_no_verification_errors)?; + ) -> VMResult> { + let (module, _) = + data_store.load_compiled_module_to_cache(id.clone(), allow_loading_failure)?; fail::fail_point!("verifier-failpoint-2", |_| { Ok(module.clone()) }); @@ -884,7 +933,7 @@ impl Loader { &self, id: &ModuleId, bundle_verified: &BTreeMap, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, visited: &mut BTreeSet, friends_discovered: &mut BTreeSet, @@ -927,7 +976,7 @@ impl Loader { &self, module: &CompiledModule, bundle_verified: &BTreeMap, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, visited: &mut BTreeSet, friends_discovered: &mut BTreeSet, @@ -994,7 +1043,7 @@ impl Loader { id: &ModuleId, bundle_verified: &BTreeMap, bundle_unverified: &BTreeSet, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, allow_module_loading_failure: bool, dependencies_depth: usize, @@ -1034,7 +1083,7 @@ impl Loader { friends_discovered: BTreeSet, bundle_verified: &BTreeMap, bundle_unverified: &BTreeSet, - data_store: &TransactionDataCache, + data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, allow_friend_loading_failure: bool, dependencies_depth: usize, @@ -2150,7 +2199,7 @@ impl Loader { pub(crate) fn get_type_layout( &self, type_tag: &TypeTag, - move_storage: &TransactionDataCache, + move_storage: &mut TransactionDataCache, module_storage: &ModuleStorageAdapter, ) -> VMResult { let ty = self.load_type(type_tag, move_storage, module_storage)?; @@ -2161,7 +2210,7 @@ impl Loader { pub(crate) fn get_fully_annotated_type_layout( &self, type_tag: &TypeTag, - move_storage: &TransactionDataCache, + move_storage: &mut TransactionDataCache, module_storage: &ModuleStorageAdapter, ) -> VMResult { let ty = self.load_type(type_tag, move_storage, module_storage)?; diff --git a/third_party/move/move-vm/runtime/src/loader/modules.rs b/third_party/move/move-vm/runtime/src/loader/modules.rs index ea84bd28f2238f..a0ac9b777c8ff9 100644 --- a/third_party/move/move-vm/runtime/src/loader/modules.rs +++ b/third_party/move/move-vm/runtime/src/loader/modules.rs @@ -93,7 +93,7 @@ impl ModuleStorageAdapter { &self, natives: &NativeFunctions, id: ModuleId, - module: CompiledModule, + module: Arc, name_cache: &StructNameCache, ) -> VMResult> { if let Some(cached) = self.module_at(&id) { @@ -252,10 +252,10 @@ pub(crate) struct FieldInstantiation { impl Module { pub(crate) fn new( natives: &NativeFunctions, - module: CompiledModule, + module: Arc, cache: &ModuleStorageAdapter, name_cache: &StructNameCache, - ) -> Result { + ) -> Result)> { let id = module.self_id(); let mut structs = vec![]; @@ -431,7 +431,7 @@ impl Module { match create() { Ok(_) => Ok(Self { id, - module: Arc::new(module), + module, structs, struct_instantiations, function_refs, diff --git a/third_party/move/move-vm/runtime/src/loader/script.rs b/third_party/move/move-vm/runtime/src/loader/script.rs index b886e9f53be40e..f2aa2e05ff3b92 100644 --- a/third_party/move/move-vm/runtime/src/loader/script.rs +++ b/third_party/move/move-vm/runtime/src/loader/script.rs @@ -22,7 +22,7 @@ use std::{collections::BTreeMap, sync::Arc}; #[derive(Clone, Debug)] pub(crate) struct Script { // primitive pools - pub(crate) script: CompiledScript, + pub(crate) script: Arc, // functions as indexes into the Loader function list pub(crate) function_refs: Vec, @@ -44,7 +44,7 @@ pub(crate) struct Script { impl Script { pub(crate) fn new( - script: CompiledScript, + script: Arc, script_hash: &ScriptHash, cache: &ModuleStorageAdapter, name_cache: &StructNameCache, diff --git a/third_party/move/move-vm/runtime/src/move_vm.rs b/third_party/move/move-vm/runtime/src/move_vm.rs index 01cfe0d9d17f72..5fa7f549cb8b55 100644 --- a/third_party/move/move-vm/runtime/src/move_vm.rs +++ b/third_party/move/move-vm/runtime/src/move_vm.rs @@ -72,7 +72,14 @@ impl MoveVM { ) -> Session<'r, '_> { Session { move_vm: self, - data_cache: TransactionDataCache::new(remote), + data_cache: TransactionDataCache::new( + self.runtime + .loader() + .vm_config() + .deserializer_config + .clone(), + remote, + ), module_store: ModuleStorageAdapter::new(self.runtime.module_storage()), native_extensions, } @@ -87,7 +94,14 @@ impl MoveVM { ) -> Session<'r, '_> { Session { move_vm: self, - data_cache: TransactionDataCache::new(remote), + data_cache: TransactionDataCache::new( + self.runtime + .loader() + .vm_config() + .deserializer_config + .clone(), + remote, + ), module_store: ModuleStorageAdapter::new(module_storage), native_extensions, } @@ -103,7 +117,14 @@ impl MoveVM { .loader() .load_module( module_id, - &TransactionDataCache::new(remote), + &mut TransactionDataCache::new( + self.runtime + .loader() + .vm_config() + .deserializer_config + .clone(), + remote, + ), &ModuleStorageAdapter::new(self.runtime.module_storage()), ) .map(|arc_module| arc_module.arc_module()) diff --git a/third_party/move/move-vm/runtime/src/session.rs b/third_party/move/move-vm/runtime/src/session.rs index 312dc5d8048a91..ce0062c31df2fe 100644 --- a/third_party/move/move-vm/runtime/src/session.rs +++ b/third_party/move/move-vm/runtime/src/session.rs @@ -336,14 +336,14 @@ impl<'r, 'l> Session<'r, 'l> { /// Load a script and all of its types into cache pub fn load_script( - &self, + &mut self, script: impl Borrow<[u8]>, ty_args: Vec, ) -> VMResult { let (_, instantiation) = self.move_vm.runtime.loader().load_script( script.borrow(), &ty_args, - &self.data_cache, + &mut self.data_cache, &self.module_store, )?; Ok(instantiation) @@ -351,7 +351,7 @@ impl<'r, 'l> Session<'r, 'l> { /// Load a module, a function, and all of its types into cache pub fn load_function_with_type_arg_inference( - &self, + &mut self, module_id: &ModuleId, function_name: &IdentStr, expected_return_type: &Type, @@ -364,7 +364,7 @@ impl<'r, 'l> Session<'r, 'l> { module_id, function_name, expected_return_type, - &self.data_cache, + &mut self.data_cache, &self.module_store, )?; Ok((func, instantiation)) @@ -372,7 +372,7 @@ impl<'r, 'l> Session<'r, 'l> { /// Load a module, a function, and all of its types into cache pub fn load_function( - &self, + &mut self, module_id: &ModuleId, function_name: &IdentStr, type_arguments: &[TypeTag], @@ -381,32 +381,35 @@ impl<'r, 'l> Session<'r, 'l> { module_id, function_name, type_arguments, - &self.data_cache, + &mut self.data_cache, &self.module_store, )?; Ok(instantiation) } - pub fn load_type(&self, type_tag: &TypeTag) -> VMResult { + pub fn load_type(&mut self, type_tag: &TypeTag) -> VMResult { self.move_vm .runtime .loader() - .load_type(type_tag, &self.data_cache, &self.module_store) + .load_type(type_tag, &mut self.data_cache, &self.module_store) } - pub fn get_type_layout(&self, type_tag: &TypeTag) -> VMResult { + pub fn get_type_layout(&mut self, type_tag: &TypeTag) -> VMResult { self.move_vm.runtime.loader().get_type_layout( type_tag, - &self.data_cache, + &mut self.data_cache, &self.module_store, ) } - pub fn get_fully_annotated_type_layout(&self, type_tag: &TypeTag) -> VMResult { + pub fn get_fully_annotated_type_layout( + &mut self, + type_tag: &TypeTag, + ) -> VMResult { self.move_vm .runtime .loader() - .get_fully_annotated_type_layout(type_tag, &self.data_cache, &self.module_store) + .get_fully_annotated_type_layout(type_tag, &mut self.data_cache, &self.module_store) } pub fn get_type_tag(&self, ty: &Type) -> VMResult { @@ -446,6 +449,36 @@ impl<'r, 'l> Session<'r, 'l> { .get_struct_type_by_identifier(&name.name, &name.module) .ok() } + + pub fn check_dependencies_and_charge_gas( + &mut self, + gas_meter: &mut impl GasMeter, + ids: I, + ) -> VMResult<()> + where + I: IntoIterator, + I::IntoIter: DoubleEndedIterator, + { + self.move_vm + .runtime + .loader() + .check_dependencies_and_charge_gas(&mut self.data_cache, gas_meter, ids) + } + + pub fn check_script_dependencies_and_check_gas( + &mut self, + gas_meter: &mut impl GasMeter, + script: impl Borrow<[u8]>, + ) -> VMResult<()> { + self.move_vm + .runtime + .loader() + .check_script_dependencies_and_check_gas( + &mut self.data_cache, + gas_meter, + script.borrow(), + ) + } } pub struct LoadedFunctionInstantiation { diff --git a/third_party/move/move-vm/test-utils/src/gas_schedule.rs b/third_party/move/move-vm/test-utils/src/gas_schedule.rs index cbba922d3296ca..2bf900e91efbd7 100644 --- a/third_party/move/move-vm/test-utils/src/gas_schedule.rs +++ b/third_party/move/move-vm/test-utils/src/gas_schedule.rs @@ -509,6 +509,10 @@ impl<'b> GasMeter for GasStatus<'b> { fn charge_create_ty(&mut self, _num_nodes: NumTypeNodes) -> PartialVMResult<()> { Ok(()) } + + fn charge_dependency(&mut self, _module_id: &ModuleId, _size: NumBytes) -> PartialVMResult<()> { + Ok(()) + } } pub fn new_from_instructions(mut instrs: Vec<(Bytecode, GasCost)>) -> CostTable { diff --git a/third_party/move/move-vm/types/src/gas.rs b/third_party/move/move-vm/types/src/gas.rs index 38536102985f20..5ae6b5a70a8deb 100644 --- a/third_party/move/move-vm/types/src/gas.rs +++ b/third_party/move/move-vm/types/src/gas.rs @@ -299,6 +299,8 @@ pub trait GasMeter { ) -> PartialVMResult<()>; fn charge_create_ty(&mut self, num_nodes: NumTypeNodes) -> PartialVMResult<()>; + + fn charge_dependency(&mut self, module_id: &ModuleId, size: NumBytes) -> PartialVMResult<()>; } /// A dummy gas meter that does not meter anything. @@ -534,4 +536,8 @@ impl GasMeter for UnmeteredGasMeter { fn charge_create_ty(&mut self, _num_nodes: NumTypeNodes) -> PartialVMResult<()> { Ok(()) } + + fn charge_dependency(&mut self, _module_id: &ModuleId, _size: NumBytes) -> PartialVMResult<()> { + Ok(()) + } }