From d22937137deb9a26365b6eec4a8a082fae2e9abb Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Mon, 31 Jul 2023 13:43:52 -0400 Subject: [PATCH 1/3] Add helpers for container bulk init; auth metering --- soroban-env-host/src/auth.rs | 217 +++++++++++++----- soroban-env-host/src/e2e_invoke.rs | 11 +- soroban-env-host/src/events/internal.rs | 60 +++-- soroban-env-host/src/host.rs | 42 ++-- soroban-env-host/src/host/conversion.rs | 28 +-- soroban-env-host/src/host/declared_size.rs | 70 +++++- soroban-env-host/src/host/metered_clone.rs | 68 ++++-- .../src/native_contract/account_contract.rs | 3 + .../src/native_contract/base_types.rs | 2 + .../native_contract/invoker_contract_auth.rs | 26 ++- soroban-env-host/src/test/budget_metering.rs | 18 +- 11 files changed, 375 insertions(+), 170 deletions(-) diff --git a/soroban-env-host/src/auth.rs b/soroban-env-host/src/auth.rs index 5cb999f57..16b4cfd7f 100644 --- a/soroban-env-host/src/auth.rs +++ b/soroban-env-host/src/auth.rs @@ -11,8 +11,9 @@ use soroban_env_common::xdr::{ }; use soroban_env_common::{AddressObject, Compare, Symbol, TryFromVal, TryIntoVal, Val, VecObject}; +use crate::budget::{AsBudget, Budget}; use crate::host::error::TryBorrowOrErr; -use crate::host::metered_clone::{charge_container_bulk_init_with_elts, MeteredClone}; +use crate::host::metered_clone::{MeteredClone, MeteredCollect, MeteredContainer}; use crate::host::Frame; use crate::host_object::HostVec; use crate::native_contract::account_contract::{ @@ -202,7 +203,7 @@ struct InvocationTracker { // In the recording mode this will record the invocations that are authorized // on behalf of the address. #[derive(Clone)] -struct AccountAuthorizationTracker { +pub(crate) struct AccountAuthorizationTracker { // Tracked address. address: AddressObject, // Helper for matching the tree that address authorized to the invocation @@ -225,7 +226,7 @@ struct AccountAuthorizationTracker { nonce: Option<(i64, u32)>, } -struct AccountAuthorizationTrackerSnapshot { +pub(crate) struct AccountAuthorizationTrackerSnapshot { invocation_tracker_root_snapshot: AuthorizedInvocationSnapshot, authenticated: bool, need_nonce: bool, @@ -233,19 +234,19 @@ struct AccountAuthorizationTrackerSnapshot { // Stores all the authorizations peformed by contracts at runtime. #[derive(Clone)] -struct InvokerContractAuthorizationTracker { +pub(crate) struct InvokerContractAuthorizationTracker { contract_address: AddressObject, invocation_tracker: InvocationTracker, } #[derive(Clone)] -enum AuthStackFrame { +pub(crate) enum AuthStackFrame { Contract(ContractInvocation), CreateContractHostFn(CreateContractArgs), } #[derive(Clone)] -struct ContractInvocation { +pub(crate) struct ContractInvocation { pub(crate) contract_address: AddressObject, pub(crate) function_name: Symbol, } @@ -277,7 +278,7 @@ pub(crate) struct AuthorizedInvocation { } // Snapshot of `AuthorizedInvocation` that contains only mutable fields. -struct AuthorizedInvocationSnapshot { +pub(crate) struct AuthorizedInvocationSnapshot { is_exhausted: bool, sub_invocations: Vec, } @@ -285,6 +286,7 @@ struct AuthorizedInvocationSnapshot { impl Compare for Host { type Error = HostError; + // metering: covered by host fn compare( &self, a: &ContractFunction, @@ -305,6 +307,7 @@ impl Compare for Host { impl Compare for Host { type Error = HostError; + // metering: covered by components fn compare( &self, a: &AuthorizedFunction, @@ -329,6 +332,7 @@ impl Compare for Host { } impl AuthStackFrame { + // metering: covered fn to_authorized_function( &self, host: &Host, @@ -352,6 +356,7 @@ impl AuthStackFrame { } impl AuthorizedFunction { + // metering: covered by the host fn from_xdr(host: &Host, xdr_fn: SorobanAuthorizedFunction) -> Result { Ok(match xdr_fn { SorobanAuthorizedFunction::ContractFn(xdr_contract_fn) => { @@ -367,6 +372,7 @@ impl AuthorizedFunction { }) } + // metering: covered by the host fn to_xdr(&self, host: &Host) -> Result { match self { AuthorizedFunction::ContractFn(contract_fn) => { @@ -395,6 +401,7 @@ impl AuthorizedFunction { } } + // metering: free fn to_xdr_non_metered(&self, host: &Host) -> Result { match self { AuthorizedFunction::ContractFn(contract_fn) => { @@ -404,6 +411,7 @@ impl AuthorizedFunction { Ok(addr.clone()) })?, function_name: contract_fn.function_name.try_into_val(host)?, + // why is this intentionally non-metered? the visit_obj above is metered. args: host.rawvals_to_sc_val_vec_non_metered(contract_fn.args.as_slice())?, })) } @@ -415,6 +423,7 @@ impl AuthorizedFunction { } impl AuthorizedInvocation { + // metering: covered fn from_xdr( host: &Host, xdr_invocation: xdr::SorobanAuthorizedInvocation, @@ -423,7 +432,7 @@ impl AuthorizedInvocation { let sub_invocations = sub_invocations_xdr .into_iter() .map(|a| AuthorizedInvocation::from_xdr(host, a)) - .collect::, _>>()?; + .metered_collect::, _>>(host.as_budget())??; Ok(Self { function: AuthorizedFunction::from_xdr(host, xdr_invocation.function)?, sub_invocations, @@ -431,6 +440,7 @@ impl AuthorizedInvocation { }) } + // metering: free pub(crate) fn new( function: AuthorizedFunction, sub_invocations: Vec, @@ -442,6 +452,7 @@ impl AuthorizedInvocation { } } + // metering: free fn new_recording(function: AuthorizedFunction) -> Self { Self { function, @@ -450,18 +461,17 @@ impl AuthorizedInvocation { } } + // metering: covered fn to_xdr(&self, host: &Host) -> Result { - charge_container_bulk_init_with_elts::< - Vec, - xdr::SorobanAuthorizedInvocation, - >(self.sub_invocations.len() as u64, host.budget_ref())?; Ok(xdr::SorobanAuthorizedInvocation { function: self.function.to_xdr(host)?, sub_invocations: self .sub_invocations .iter() .map(|i| i.to_xdr(host)) - .collect::, HostError>>()? + .metered_collect::, HostError>>( + host.as_budget(), + )?? .try_into() .map_err(|_| HostError::from((ScErrorType::Auth, ScErrorCode::InternalError)))?, }) @@ -469,6 +479,7 @@ impl AuthorizedInvocation { // Non-metered conversion should only be used for the recording preflight // runs or testing. + // metering: free fn to_xdr_non_metered( &self, host: &Host, @@ -489,6 +500,7 @@ impl AuthorizedInvocation { // Walks a path in the tree defined by `invocation_id_in_call_stack` and // returns the last visited authorized node. + // metering: free fn last_authorized_invocation_mut( &mut self, invocation_id_in_call_stack: &Vec>, @@ -498,11 +510,15 @@ impl AuthorizedInvocation { // hold the invariant that `invocation_id_in_call_stack[call_stack_id - 1]` // corresponds to this invocation tree, so that the next non-`None` child // corresponds to the child of the current tree. - for i in call_stack_id..invocation_id_in_call_stack.len() { - if let Some(id) = invocation_id_in_call_stack[i] { + for (i, id) in invocation_id_in_call_stack + .iter() + .enumerate() + .skip(call_stack_id) + { + if let Some(id) = id { // We trust the caller to have the correct sub-invocation // indices. - return self.sub_invocations[id] + return self.sub_invocations[*id] .last_authorized_invocation_mut(invocation_id_in_call_stack, i + 1); } // Skip `None` invocations as they don't require authorization. @@ -510,21 +526,29 @@ impl AuthorizedInvocation { self } - fn snapshot(&self) -> AuthorizedInvocationSnapshot { - AuthorizedInvocationSnapshot { + // metering: covered + fn snapshot(&self, budget: &Budget) -> Result { + Ok(AuthorizedInvocationSnapshot { is_exhausted: self.is_exhausted, - sub_invocations: self.sub_invocations.iter().map(|i| i.snapshot()).collect(), - } + sub_invocations: self + .sub_invocations + .iter() + .map(|i| i.snapshot(budget)) + .metered_collect::, HostError>>( + budget, + )??, + }) } + // metering: free fn rollback(&mut self, snapshot: &AuthorizedInvocationSnapshot) -> Result<(), HostError> { self.is_exhausted = snapshot.is_exhausted; if self.sub_invocations.len() != snapshot.sub_invocations.len() { // This would be a bug. return Err((ScErrorType::Auth, ScErrorCode::InternalError).into()); } - for i in 0..self.sub_invocations.len() { - self.sub_invocations[i].rollback(&snapshot.sub_invocations[i])?; + for (i, sub_invocation) in self.sub_invocations.iter_mut().enumerate() { + sub_invocation.rollback(&snapshot.sub_invocations[i])?; } Ok(()) } @@ -540,11 +564,16 @@ impl AuthorizationManager { // Creates a new enforcing `AuthorizationManager` from the given // authorization entries. // This should be created once per top-level invocation. + // metering: covered pub(crate) fn new_enforcing( host: &Host, auth_entries: Vec, ) -> Result { - let mut trackers = vec![]; + Vec::::charge_bulk_init( + auth_entries.len() as u64, + host.as_budget(), + )?; + let mut trackers = Vec::with_capacity(auth_entries.len()); for auth_entry in auth_entries { trackers.push(RefCell::new( AccountAuthorizationTracker::from_authorization_entry(host, auth_entry)?, @@ -561,6 +590,7 @@ impl AuthorizationManager { // Creates a new enforcing `AuthorizationManager` that doesn't allow any // authorizations. // This is useful as a safe default mode. + // metering: free pub(crate) fn new_enforcing_without_authorizations() -> Self { Self { mode: AuthorizationMode::Enforcing, @@ -573,6 +603,7 @@ impl AuthorizationManager { // Creates a new recording `AuthorizationManager`. // All the authorization requirements will be recorded and can then be // retrieved using `get_recorded_auth_payloads`. + // metering: free pub(crate) fn new_recording() -> Self { Self { mode: AuthorizationMode::Recording(RecordingAuthInfo { @@ -589,6 +620,7 @@ impl AuthorizationManager { // authorized call stack and for the current network). // In the recording mode this stores the auth requirement instead of // verifying it. + // metering: covered pub(crate) fn require_auth( &self, host: &Host, @@ -612,6 +644,7 @@ impl AuthorizationManager { self.require_auth_internal(host, address, authorized_function) } + // metering: covered pub(crate) fn add_invoker_contract_auth( &self, host: &Host, @@ -620,12 +653,15 @@ impl AuthorizationManager { let auth_entries = host.visit_obj(auth_entries, |e: &HostVec| e.to_vec(host.budget_ref()))?; let mut trackers = self.try_borrow_invoker_contract_trackers_mut(host)?; + Vec::::charge_bulk_init(auth_entries.len() as u64, host.as_budget())?; + trackers.reserve(auth_entries.len()); for e in auth_entries { trackers.push(InvokerContractAuthorizationTracker::new(host, e)?) } Ok(()) } + // metering: covered by components fn verify_contract_invoker_auth( &self, host: &Host, @@ -668,6 +704,7 @@ impl AuthorizationManager { return Ok(false); } + // metering: covered by components fn require_auth_enforcing( &self, host: &Host, @@ -735,6 +772,7 @@ impl AuthorizationManager { )) } + // metering: covered fn require_auth_internal( &self, host: &Host, @@ -751,6 +789,7 @@ impl AuthorizationManager { match &self.mode { AuthorizationMode::Enforcing => self.require_auth_enforcing(host, address, &function), + // metering: free for recording AuthorizationMode::Recording(recording_info) => { let address_obj_handle = address.get_handle(); let existing_tracker_id = recording_info @@ -806,38 +845,52 @@ impl AuthorizationManager { } // Returns a snapshot of `AuthorizationManager` to use for rollback. + // metering: covered pub(crate) fn snapshot(&self, host: &Host) -> Result { let _span = tracy_span!("snapshot auth"); let account_trackers_snapshot = match &self.mode { - AuthorizationMode::Enforcing => AccountTrackersSnapshot::Enforcing( - self.try_borrow_account_trackers(host)? - .iter() - .map(|t| { - if let Ok(tracker) = t.try_borrow() { - Some(tracker.snapshot()) - } else { - // If tracker is borrowed, snapshotting it is a no-op - // (it can't change until we release it higher up the - // stack). - None - } - }) - .collect(), - ), + AuthorizationMode::Enforcing => { + let len = self.try_borrow_account_trackers(host)?.len(); + let mut snapshots = Vec::with_capacity(len); + Vec::>::charge_bulk_init( + len as u64, + host.as_budget(), + )?; + for t in self.try_borrow_account_trackers(host)?.iter() { + let sp = if let Ok(tracker) = t.try_borrow() { + Some(tracker.snapshot(host.as_budget())?) + } else { + // If tracker is borrowed, snapshotting it is a no-op + // (it can't change until we release it higher up the + // stack). + None + }; + snapshots.push(sp); + } + AccountTrackersSnapshot::Enforcing(snapshots) + } AuthorizationMode::Recording(_) => { // All trackers should be avaialable to borrow for copy as in // recording mode we can't have recursive authorization. + // metering: free for recording AccountTrackersSnapshot::Recording(self.try_borrow_account_trackers(host)?.clone()) } }; let invoker_contract_tracker_root_snapshots = self .try_borrow_invoker_contract_trackers(host)? .iter() - .map(|t| t.invocation_tracker.root_authorized_invocation.snapshot()) - .collect(); + .map(|t| { + t.invocation_tracker + .root_authorized_invocation + .snapshot(host.as_budget()) + }) + .metered_collect::, HostError>>( + host.as_budget(), + )??; let tracker_by_address_handle = match &self.mode { AuthorizationMode::Enforcing => None, AuthorizationMode::Recording(recording_info) => Some( + // metering: free for recording recording_info .try_borrow_tracker_by_address_handle(host)? .clone(), @@ -851,6 +904,7 @@ impl AuthorizationManager { } // Rolls back this `AuthorizationManager` to the snapshot state. + // metering: covered pub(crate) fn rollback( &self, host: &Host, @@ -868,9 +922,9 @@ impl AuthorizationManager { &[], )); } - for i in 0..trackers.len() { + for (i, tracker) in trackers.iter().enumerate() { if let Some(tracker_snapshot) = &trackers_snapshot[i] { - trackers[i] + tracker .try_borrow_mut() .map_err(|_| { host.err( @@ -911,28 +965,31 @@ impl AuthorizationManager { Ok(()) } + // metering: covered fn push_tracker_frame(&self, host: &Host) -> Result<(), HostError> { for tracker in self.try_borrow_account_trackers(host)?.iter() { // Skip already borrowed trackers, these must be in the middle of // authentication and hence don't need stack to be updated. if let Ok(mut tracker) = tracker.try_borrow_mut() { - tracker.push_frame(); + tracker.push_frame(host.as_budget())?; } } for tracker in self .try_borrow_invoker_contract_trackers_mut(host)? .iter_mut() { - tracker.push_frame(); + tracker.push_frame(host.as_budget())?; } Ok(()) } + // metering: covered pub(crate) fn push_create_contract_host_fn_frame( &self, host: &Host, args: CreateContractArgs, ) -> Result<(), HostError> { + Vec::::charge_bulk_init(1, host.as_budget())?; self.try_borrow_call_stack_mut(host)? .push(AuthStackFrame::CreateContractHostFn(args)); self.push_tracker_frame(host) @@ -940,6 +997,7 @@ impl AuthorizationManager { // Records a new call stack frame. // This should be called for every `Host` `push_frame`. + // metering: covered pub(crate) fn push_frame(&self, host: &Host, frame: &Frame) -> Result<(), HostError> { let _span = tracy_span!("push auth frame"); let (contract_id, function_name) = match frame { @@ -958,6 +1016,7 @@ impl AuthorizationManager { }; // Currently only contracts might appear in the call stack. let contract_address = host.add_host_object(ScAddress::Contract(contract_id))?; + Vec::::charge_bulk_init(1, host.as_budget())?; self.try_borrow_call_stack_mut(host)? .push(AuthStackFrame::Contract(ContractInvocation { contract_address, @@ -969,6 +1028,7 @@ impl AuthorizationManager { // Pops a call stack frame. // This should be called for every `Host` `pop_frame`. + // metering: covered pub(crate) fn pop_frame(&self, host: &Host) -> Result<(), HostError> { let _span = tracy_span!("pop auth frame"); { @@ -1010,6 +1070,7 @@ impl AuthorizationManager { // Returns the recorded per-address authorization payloads that would cover the // top-level contract function invocation in the enforcing mode. // Should only be called in the recording mode. + // metering: free, recording mode pub(crate) fn get_recorded_auth_payloads( &self, host: &Host, @@ -1032,6 +1093,7 @@ impl AuthorizationManager { // This helps to build a more realistic footprint and produce more correct // meterting data for the recording mode. // No-op in the enforcing mode. + // metering: covered pub(crate) fn maybe_emulate_authentication(&self, host: &Host) -> Result<(), HostError> { match &self.mode { AuthorizationMode::Enforcing => Ok(()), @@ -1048,6 +1110,7 @@ impl AuthorizationManager { // Returns a 'reset' instance of `AuthorizationManager` that has the same // mode, but no data. + // metering: free, testutils #[cfg(any(test, feature = "testutils"))] pub(crate) fn reset(&mut self) { *self = match self.mode { @@ -1060,6 +1123,7 @@ impl AuthorizationManager { // Returns all authorizations that have been authenticated for the // last contract invocation. + // metering: free, testutils #[cfg(any(test, feature = "testutils"))] pub(crate) fn get_authenticated_authorizations( &self, @@ -1084,6 +1148,7 @@ impl AuthorizationManager { } impl InvocationTracker { + // metering: covered by components fn from_xdr( host: &Host, root_invocation: xdr::SorobanAuthorizedInvocation, @@ -1096,6 +1161,7 @@ impl InvocationTracker { }) } + // metering: free fn new(root_authorized_invocation: AuthorizedInvocation) -> Self { Self { root_authorized_invocation, @@ -1105,6 +1171,7 @@ impl InvocationTracker { } } + // metering: free for recording fn new_recording(function: AuthorizedFunction, current_stack_len: usize) -> Self { // Create the stack of `None` leading to the current invocation to // represent invocations that didn't need authorization on behalf of @@ -1123,6 +1190,7 @@ impl InvocationTracker { // Walks a path in the tree defined by `invocation_id_in_call_stack` and // returns the last visited authorized node. + // metering: free fn last_authorized_invocation_mut(&mut self) -> Option<&mut AuthorizedInvocation> { for i in 0..self.invocation_id_in_call_stack.len() { if self.invocation_id_in_call_stack[i].is_some() { @@ -1135,10 +1203,14 @@ impl InvocationTracker { None } - fn push_frame(&mut self) { + // metering: covered + fn push_frame(&mut self, budget: &Budget) -> Result<(), HostError> { + Vec::::charge_bulk_init(1, budget)?; self.invocation_id_in_call_stack.push(None); + Ok(()) } + // metering: free fn pop_frame(&mut self) { self.invocation_id_in_call_stack.pop(); if let Some(root_exhausted_frame) = self.root_exhausted_frame { @@ -1148,14 +1220,17 @@ impl InvocationTracker { } } + // metering: free fn is_empty(&self) -> bool { self.invocation_id_in_call_stack.is_empty() } + // metering: free fn is_active(&self) -> bool { self.root_authorized_invocation.is_exhausted && !self.is_fully_processed } + // metering: free fn current_frame_is_already_matched(&self) -> bool { match self.invocation_id_in_call_stack.last() { Some(Some(_)) => true, @@ -1167,6 +1242,7 @@ impl InvocationTracker { // of the current tree and push it to the call stack. // Returns `true` if the match has been found for the first time per current // frame. + // Metering: covered by components fn maybe_push_matching_invocation_frame( &mut self, host: &Host, @@ -1178,8 +1254,7 @@ impl InvocationTracker { } let mut frame_index = None; if let Some(curr_invocation) = self.last_authorized_invocation_mut() { - for i in 0..curr_invocation.sub_invocations.len() { - let sub_invocation = &mut curr_invocation.sub_invocations[i]; + for (i, sub_invocation) in curr_invocation.sub_invocations.iter_mut().enumerate() { if !sub_invocation.is_exhausted && host.compare(&sub_invocation.function, function)?.is_eq() { @@ -1208,6 +1283,7 @@ impl InvocationTracker { // This is needed for the recording mode only. // This assumes that the address matching is correctly performed before // calling this. + // metering: free for recording fn record_invocation( &mut self, host: &Host, @@ -1243,14 +1319,17 @@ impl InvocationTracker { Ok(()) } + // metering: free fn has_matched_invocations_in_stack(&self) -> bool { self.invocation_id_in_call_stack.iter().any(|i| i.is_some()) } - fn snapshot(&self) -> AuthorizedInvocationSnapshot { - self.root_authorized_invocation.snapshot() + // metering: covered + fn snapshot(&self, budget: &Budget) -> Result { + self.root_authorized_invocation.snapshot(budget) } + // metering: covered fn rollback(&mut self, snapshot: &AuthorizedInvocationSnapshot) -> Result<(), HostError> { self.root_authorized_invocation.rollback(snapshot)?; // Invocation can only be rolled back from 'exhausted' to @@ -1265,6 +1344,7 @@ impl InvocationTracker { } impl AccountAuthorizationTracker { + // Metering: covered by the host and components fn from_authorization_entry( host: &Host, auth_entry: SorobanAuthorizationEntry, @@ -1304,6 +1384,7 @@ impl AccountAuthorizationTracker { }) } + // metering: free, since this is recording mode only fn new_recording( host: &Host, address: AddressObject, @@ -1360,6 +1441,7 @@ impl AccountAuthorizationTracker { // Returns true/false based on whether the invocation is found in the // tracker. Returns error if invocation has been found, but the tracker // itself is not valid (failed authentication or nonce check). + // metering: covered fn maybe_authorize_invocation( &mut self, host: &Host, @@ -1411,6 +1493,7 @@ impl AccountAuthorizationTracker { // This is needed for the recording mode only. // This assumes that the address matching is correctly performed before // calling this. + // metering: free for recording fn record_invocation( &mut self, host: &Host, @@ -1421,6 +1504,7 @@ impl AccountAuthorizationTracker { // Build the authorization payload from the invocations recorded in this // tracker. + // metering: free for recording fn get_recorded_auth_payload(&self, host: &Host) -> Result { Ok(RecordedAuthPayload { address: if !self.is_invoker { @@ -1438,10 +1522,12 @@ impl AccountAuthorizationTracker { // Checks if there is at least one authorized invocation in the current call // stack. + // metering: free fn has_authorized_invocations_in_stack(&self) -> bool { self.invocation_tracker.has_matched_invocations_in_stack() } + // metering: covered fn root_invocation_to_xdr( &self, host: &Host, @@ -1451,14 +1537,17 @@ impl AccountAuthorizationTracker { .to_xdr(host) } - fn push_frame(&mut self) { - self.invocation_tracker.push_frame(); + // metering: covered + fn push_frame(&mut self, budget: &Budget) -> Result<(), HostError> { + self.invocation_tracker.push_frame(budget) } + // metering: covered fn pop_frame(&mut self) { self.invocation_tracker.pop_frame(); } + // metering: covered fn verify_and_consume_nonce(&mut self, host: &Host) -> Result<(), HostError> { if !self.need_nonce { return Ok(()); @@ -1502,6 +1591,7 @@ impl AccountAuthorizationTracker { // Computes the payload that has to be signed in order to authenticate // the authorized invocation tree corresponding to this tracker. + // metering: covered by components fn get_signature_payload(&self, host: &Host) -> Result<[u8; 32], HostError> { let (nonce, expiration_ledger) = self.nonce.ok_or_else(|| { host.err( @@ -1524,6 +1614,7 @@ impl AccountAuthorizationTracker { host.metered_hash_xdr(&payload_preimage) } + // metering: covered by the hsot fn authenticate(&self, host: &Host) -> Result<(), HostError> { if self.is_invoker { return Ok(()); @@ -1551,6 +1642,7 @@ impl AccountAuthorizationTracker { } // Emulates authentication for the recording mode. + // metering: covered fn emulate_authentication(&self, host: &Host) -> Result<(), HostError> { if self.is_invoker { return Ok(()); @@ -1569,9 +1661,10 @@ impl AccountAuthorizationTracker { Ok(()) } - fn snapshot(&self) -> AccountAuthorizationTrackerSnapshot { - AccountAuthorizationTrackerSnapshot { - invocation_tracker_root_snapshot: self.invocation_tracker.snapshot(), + // metering: covered + fn snapshot(&self, budget: &Budget) -> Result { + Ok(AccountAuthorizationTrackerSnapshot { + invocation_tracker_root_snapshot: self.invocation_tracker.snapshot(budget)?, // We need to snapshot authenctioncation and nocne-related fields as // during the rollback the nonce might be 'un-consumed' during // the storage rollback. We don't snapshot `is_valid` since this is @@ -1579,9 +1672,10 @@ impl AccountAuthorizationTracker { // authorizations. authenticated: self.authenticated, need_nonce: self.need_nonce, - } + }) } + // metering: covered fn rollback( &mut self, snapshot: &AccountAuthorizationTrackerSnapshot, @@ -1593,16 +1687,19 @@ impl AccountAuthorizationTracker { Ok(()) } + // metering: free fn is_active(&self) -> bool { self.is_valid && self.invocation_tracker.is_active() } + // metering: free fn current_frame_is_already_matched(&self) -> bool { self.invocation_tracker.current_frame_is_already_matched() } } impl InvokerContractAuthorizationTracker { + // metering: covered by components fn new(host: &Host, invoker_auth_entry: Val) -> Result { let invoker_sc_addr = ScAddress::Contract(host.get_current_contract_id_internal()?); let authorized_invocation = invoker_contract_auth_to_authorized_invocation( @@ -1617,18 +1714,22 @@ impl InvokerContractAuthorizationTracker { }) } - fn push_frame(&mut self) { - self.invocation_tracker.push_frame(); + // metering: covered + fn push_frame(&mut self, budget: &Budget) -> Result<(), HostError> { + self.invocation_tracker.push_frame(budget) } + // metering: covered fn pop_frame(&mut self) { self.invocation_tracker.pop_frame(); } + // metering: free fn is_out_of_scope(&self) -> bool { self.invocation_tracker.is_empty() } + // metering: covered fn maybe_authorize_invocation( &mut self, host: &Host, @@ -1642,6 +1743,7 @@ impl InvokerContractAuthorizationTracker { } impl Host { + // metering: covered by components fn consume_nonce( &self, address: AddressObject, @@ -1687,6 +1789,7 @@ impl Host { } } +// metering: free for testutils #[cfg(any(test, feature = "testutils"))] impl PartialEq for RecordedAuthPayload { fn eq(&self, other: &Self) -> bool { diff --git a/soroban-env-host/src/e2e_invoke.rs b/soroban-env-host/src/e2e_invoke.rs index 33ba3cb6e..deec9e12a 100644 --- a/soroban-env-host/src/e2e_invoke.rs +++ b/soroban-env-host/src/e2e_invoke.rs @@ -16,12 +16,12 @@ use soroban_env_common::{ }; use crate::{ - budget::Budget, + budget::{AsBudget, Budget}, events::Events, fees::LedgerEntryRentChange, host::{ ledger_info_helper::{get_entry_expiration, get_key_durability, set_entry_expiration}, - metered_clone::{charge_container_bulk_init_with_elts, MeteredClone}, + metered_clone::{MeteredClone, MeteredContainer}, metered_xdr::{metered_from_xdr_with_budget, metered_write_xdr}, }, storage::{AccessType, Footprint, FootprintMap, SnapshotSource, Storage, StorageMap}, @@ -450,12 +450,9 @@ impl Host { &self, encoded_contract_auth_entries: I, ) -> Result, HostError> { - charge_container_bulk_init_with_elts::< - Vec, - SorobanAuthorizationEntry, - >( + Vec::::charge_bulk_init( encoded_contract_auth_entries.len() as u64, - self.budget_ref(), + self.as_budget(), )?; encoded_contract_auth_entries .map(|buf| self.metered_from_xdr::(buf.as_ref())) diff --git a/soroban-env-host/src/events/internal.rs b/soroban-env-host/src/events/internal.rs index ac25e17ac..65da27b20 100644 --- a/soroban-env-host/src/events/internal.rs +++ b/soroban-env-host/src/events/internal.rs @@ -5,7 +5,7 @@ use soroban_env_common::{BytesObject, VecObject}; use super::{Events, HostEvent}; use crate::{ budget::{AsBudget, Budget}, - host::metered_clone, + host::metered_clone::MeteredContainer, xdr, xdr::ScVal, Host, HostError, Val, @@ -134,13 +134,10 @@ impl InternalEventsBuffer { // `Vec.push(event)`. Because the buffer length may be different on different instances // due to diagnostic events and we need a deterministic cost across all instances, // the cost needs to be amortized and buffer size-independent. + if let InternalEvent::Contract(_) = e { - metered_clone::charge_container_bulk_init_with_elts::< - Vec<(InternalEvent, EventError)>, - (InternalEvent, EventError), - >(1, budget)?; + Vec::<(InternalEvent, EventError)>::charge_bulk_init(1, budget)?; } - self.vec.push((e, EventError::FromSuccessfulCall)); Ok(()) } @@ -172,33 +169,30 @@ impl InternalEventsBuffer { /// Converts the internal events into their external representation. This should only be called /// either when the host is finished (via `try_finish`), or when an error occurs. pub fn externalize(&self, host: &Host) -> Result { - let vec: Result, HostError> = - self.vec - .iter() - .map(|e| match &e.0 { - InternalEvent::Contract(c) => { - // Metering: we use the cost of instantiating a size=1 `Vec` as an estimate - // for the cost collecting 1 `HostEvent` into the events buffer. Because - // the resulting buffer length may be different on different instances - // (due to diagnostic events) and we need a deterministic cost across all - // instances, the cost needs to be amortized and buffer size-independent. - metered_clone::charge_container_bulk_init_with_elts::< - Vec, - HostEvent, - >(1, host.as_budget())?; - Ok(HostEvent { - event: c.to_xdr(host)?, - failed_call: e.1 == EventError::FromFailedCall, - }) - } - InternalEvent::Diagnostic(c) => host.as_budget().with_free_budget(|| { - Ok(HostEvent { - event: c.to_xdr(host)?, - failed_call: e.1 == EventError::FromFailedCall, - }) - }), - }) - .collect(); + let vec: Result, HostError> = self + .vec + .iter() + .map(|e| match &e.0 { + InternalEvent::Contract(c) => { + // Metering: we use the cost of instantiating a size=1 `Vec` as an estimate + // for the cost collecting 1 `HostEvent` into the events buffer. Because + // the resulting buffer length may be different on different instances + // (due to diagnostic events) and we need a deterministic cost across all + // instances, the cost needs to be amortized and buffer size-independent. + Vec::::charge_bulk_init(1, host.as_budget())?; + Ok(HostEvent { + event: c.to_xdr(host)?, + failed_call: e.1 == EventError::FromFailedCall, + }) + } + InternalEvent::Diagnostic(c) => host.as_budget().with_free_budget(|| { + Ok(HostEvent { + event: c.to_xdr(host)?, + failed_call: e.1 == EventError::FromFailedCall, + }) + }), + }) + .collect(); Ok(Events(vec?)) } } diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 6be5f3467..7375c3f70 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -57,7 +57,10 @@ use self::{ metered_vector::MeteredVector, prng::Prng, }; -use self::{metered_clone::MeteredClone, metered_xdr::metered_write_xdr}; +use self::{ + metered_clone::{MeteredClone, MeteredContainer}, + metered_xdr::metered_write_xdr, +}; use crate::impl_bignum_host_fns; use crate::Compare; #[cfg(any(test, feature = "testutils"))] @@ -1017,21 +1020,22 @@ impl EnvBase for Host { } fn string_new_from_slice(&self, s: &str) -> Result { - self.add_host_object(ScString(s.as_bytes().to_vec().try_into()?)) + self.add_host_object(ScString( + self.metered_slice_to_vec(s.as_bytes())?.try_into()?, + )) } fn symbol_new_from_slice(&self, s: &str) -> Result { for ch in s.chars() { SymbolSmall::validate_char(ch)?; } - self.add_host_object(ScSymbol(s.as_bytes().to_vec().try_into()?)) + self.add_host_object(ScSymbol( + self.metered_slice_to_vec(s.as_bytes())?.try_into()?, + )) } fn map_new_from_slices(&self, keys: &[&str], vals: &[Val]) -> Result { - metered_clone::charge_container_bulk_init_with_elts::, Symbol>( - keys.len() as u64, - self.as_budget(), - )?; + Vec::::charge_bulk_init(keys.len() as u64, self.as_budget())?; // If only fallible iterators worked better in Rust, we would not need this Vec<...>. let mut key_syms: Vec = Vec::with_capacity(keys.len()); for k in keys.iter() { @@ -1670,11 +1674,7 @@ impl VmCallerEnv for Host { pos: keys_pos, len, } = self.decode_vmslice(keys_pos, len)?; - // covers `Vec::with_capacity` and `len` pushes - metered_clone::charge_container_bulk_init_with_elts::, Symbol>( - len as u64, - self.as_budget(), - )?; + Vec::::charge_bulk_init(len as u64, self.as_budget())?; let mut key_syms: Vec = Vec::with_capacity(len as usize); self.metered_vm_scan_slices_in_linear_memory( vmcaller, @@ -1692,10 +1692,7 @@ impl VmCallerEnv for Host { // Step 2: extract all val Vals. let vals_pos: u32 = vals_pos.into(); - metered_clone::charge_container_bulk_init_with_elts::, Symbol>( - len as u64, - self.as_budget(), - )?; + Vec::::charge_bulk_init(len as u64, self.as_budget())?; let mut vals: Vec = vec![Val::VOID.into(); len as usize]; self.metered_vm_read_vals_from_linear_memory::<8, Val>( vmcaller, @@ -1986,10 +1983,7 @@ impl VmCallerEnv for Host { len: U32Val, ) -> Result { let VmSlice { vm, pos, len } = self.decode_vmslice(vals_pos, len)?; - metered_clone::charge_container_bulk_init_with_elts::, Symbol>( - len as u64, - self.as_budget(), - )?; + Vec::::charge_bulk_init(len as u64, self.as_budget())?; let mut vals: Vec = vec![Val::VOID.to_val(); len as usize]; self.metered_vm_read_vals_from_linear_memory::<8, Val>( vmcaller, @@ -2767,9 +2761,7 @@ impl VmCallerEnv for Host { let end: u32 = end.into(); let vnew = self.visit_obj(b, move |hv: &ScBytes| { let range = self.valid_range_from_start_end_bound(start, end, hv.len())?; - metered_clone::charge_heap_alloc::(range.len() as u64, self.as_budget())?; - metered_clone::charge_shallow_copy::(range.len() as u64, self.as_budget())?; - Ok(hv.as_slice()[range].to_vec()) + self.metered_slice_to_vec(&hv.as_slice()[range]) })?; self.add_host_object(self.scbytes_from_vec(vnew)?) } @@ -2979,7 +2971,7 @@ impl VmCallerEnv for Host { let addr = self.visit_obj(address, |addr: &ScAddress| Ok(addr.clone()))?; match addr { ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(pk))) => Ok(self - .add_host_object(ScBytes(pk.0.to_vec().try_into()?))? + .add_host_object(ScBytes(self.metered_slice_to_vec(&pk.0)?.try_into()?))? .into()), ScAddress::Contract(_) => Ok(().into()), } @@ -2994,7 +2986,7 @@ impl VmCallerEnv for Host { match addr { ScAddress::Account(_) => Ok(().into()), ScAddress::Contract(Hash(h)) => Ok(self - .add_host_object(ScBytes(h.to_vec().try_into()?))? + .add_host_object(ScBytes(self.metered_slice_to_vec(&h)?.try_into()?))? .into()), } } diff --git a/soroban-env-host/src/host/conversion.rs b/soroban-env-host/src/host/conversion.rs index d010d0cb7..baf0f99ce 100644 --- a/soroban-env-host/src/host/conversion.rs +++ b/soroban-env-host/src/host/conversion.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use super::metered_clone::{self, charge_container_bulk_init_with_elts, MeteredClone}; +use super::metered_clone::{self, MeteredClone, MeteredCollect, MeteredContainer}; use crate::budget::AsBudget; use crate::err; use crate::host_object::{HostMap, HostObject, HostVec}; @@ -242,14 +242,10 @@ impl Host { } pub(crate) fn rawvals_to_sc_val_vec(&self, raw_vals: &[Val]) -> Result, HostError> { - charge_container_bulk_init_with_elts::, Val>( - raw_vals.len() as u64, - self.as_budget(), - )?; raw_vals .iter() .map(|v| self.from_host_val(*v)) - .collect::, HostError>>()? + .metered_collect::, HostError>>(self.as_budget())?? .try_into() .map_err(|_| { err!( @@ -281,21 +277,19 @@ impl Host { } pub(crate) fn scvals_to_rawvals(&self, sc_vals: &[ScVal]) -> Result, HostError> { - charge_container_bulk_init_with_elts::, Val>( - sc_vals.len() as u64, - self.as_budget(), - )?; sc_vals .iter() .map(|scv| self.to_host_val(scv)) - .collect::, HostError>>() + .metered_collect::, HostError>>(self.as_budget())? } pub(crate) fn bytesobj_from_internal_contract_id( &self, ) -> Result, HostError> { if let Some(id) = self.get_current_contract_id_opt_internal()? { - let obj = self.add_host_object::(id.as_slice().to_vec().try_into()?)?; + let obj = self.add_host_object::( + self.metered_slice_to_vec(id.as_slice())?.try_into()?, + )?; Ok(Some(obj)) } else { Ok(None) @@ -306,8 +300,14 @@ impl Host { Ok(ScBytes(v.try_into()?)) } - pub(crate) fn scbytes_from_slice(&self, slice: &[u8]) -> Result { - self.scbytes_from_vec(slice.to_vec()) + pub(crate) fn metered_slice_to_vec(&self, s: &[u8]) -> Result, HostError> { + Vec::::charge_bulk_init(s.len() as u64, self.as_budget())?; + Ok(s.to_vec()) + } + + // metering: covered + pub(crate) fn scbytes_from_slice(&self, s: &[u8]) -> Result { + self.scbytes_from_vec(self.metered_slice_to_vec(s)?) } pub(crate) fn scbytes_from_hash(&self, hash: &Hash) -> Result { diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 47903756a..27ec9f328 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -3,6 +3,10 @@ use std::rc::Rc; use soroban_env_common::xdr::SorobanAuthorizationEntry; use crate::{ + auth::{ + AccountAuthorizationTracker, AccountAuthorizationTrackerSnapshot, AuthorizedInvocation, + AuthorizedInvocationSnapshot, ContractInvocation, InvokerContractAuthorizationTracker, + }, events::{EventError, HostEvent, InternalContractEvent, InternalEvent}, host::Events, host_object::HostObject, @@ -18,7 +22,7 @@ use crate::{ ScSymbol, ScVal, ScVec, SorobanAuthorizedInvocation, StringM, TimePoint, TrustLineAsset, TrustLineEntry, Uint256, SCSYMBOL_LIMIT, }, - AddressObject, Bool, BytesObject, DurationObject, DurationSmall, DurationVal, Error, + AddressObject, Bool, BytesObject, DurationObject, DurationSmall, DurationVal, Error, HostError, I128Object, I128Small, I128Val, I256Object, I256Small, I256Val, I32Val, I64Object, I64Small, I64Val, MapObject, Object, ScValObject, StringObject, Symbol, SymbolObject, SymbolSmall, SymbolSmallIter, SymbolStr, TimepointObject, TimepointSmall, TimepointVal, U128Object, @@ -51,6 +55,7 @@ impl_declared_size_type!(u64, 8); impl_declared_size_type!(i64, 8); impl_declared_size_type!(u128, 16); impl_declared_size_type!(i128, 16); +impl_declared_size_type!(usize, 8); // Val-wrapping types impl_declared_size_type!(Val, 8); impl_declared_size_type!(Void, 8); @@ -97,6 +102,7 @@ impl_declared_size_type!(SymbolSmallIter, 8); impl_declared_size_type!(U256, 32); impl_declared_size_type!(I256, 32); impl_declared_size_type!(HostObject, 48); +impl_declared_size_type!(HostError, 16); // xdr types impl_declared_size_type!(TimePoint, 8); impl_declared_size_type!(Duration, 8); @@ -129,9 +135,9 @@ impl_declared_size_type!(DataEntry, 80); impl_declared_size_type!(ClaimableBalanceEntry, 120); impl_declared_size_type!(LiquidityPoolEntry, 160); impl_declared_size_type!(ContractCodeEntry, 64); -impl_declared_size_type!(ConfigSettingEntry, 104); +impl_declared_size_type!(ConfigSettingEntry, 96); impl_declared_size_type!(LedgerKey, 120); -impl_declared_size_type!(LedgerEntry, 264); +impl_declared_size_type!(LedgerEntry, 256); impl_declared_size_type!(AccessType, 1); impl_declared_size_type!(InternalContractEvent, 40); impl_declared_size_type!(ContractEvent, 128); @@ -142,14 +148,21 @@ impl_declared_size_type!(EventError, 1); impl_declared_size_type!(ScBytes, 24); impl_declared_size_type!(ScString, 24); impl_declared_size_type!(ScSymbol, 24); -impl_declared_size_type!(CreateContractArgs, 99); -impl_declared_size_type!(ContractIdPreimage, 66); +impl_declared_size_type!(CreateContractArgs, 98); +impl_declared_size_type!(ContractIdPreimage, 65); impl_declared_size_type!(ContractDataDurability, 4); impl_declared_size_type!(ContractEntryBodyType, 4); impl_declared_size_type!(ExtensionPoint, 0); -impl_declared_size_type!(SorobanAuthorizedInvocation, 128); impl_declared_size_type!(ScContractInstance, 64); impl_declared_size_type!(SorobanAuthorizationEntry, 240); +impl_declared_size_type!(SorobanAuthorizedInvocation, 128); +impl_declared_size_type!(AuthorizedInvocation, 136); +impl_declared_size_type!(AuthorizedInvocationSnapshot, 32); +impl_declared_size_type!(AccountAuthorizationTracker, 232); +impl_declared_size_type!(InvokerContractAuthorizationTracker, 192); +impl_declared_size_type!(AccountAuthorizationTrackerSnapshot, 40); +impl_declared_size_type!(ContractInvocation, 16); + // composite types // Rc is an exception, nothing is being cloned. We approximate ref counter bump with the cost of @@ -203,6 +216,12 @@ impl DeclaredSizeForMetering for Option { const DECLARED_SIZE: u64 = C::DECLARED_SIZE.saturating_add(8); } +impl DeclaredSizeForMetering + for Result +{ + const DECLARED_SIZE: u64 = C::DECLARED_SIZE + E::DECLARED_SIZE; +} + mod test { #[allow(unused)] use super::*; @@ -225,6 +244,7 @@ mod test { expect!["8"].assert_eq(size_of::().to_string().as_str()); expect!["16"].assert_eq(size_of::().to_string().as_str()); expect!["16"].assert_eq(size_of::().to_string().as_str()); + expect!["8"].assert_eq(size_of::().to_string().as_str()); // Val-wrapping types expect!["8"].assert_eq(size_of::().to_string().as_str()); expect!["8"].assert_eq(size_of::().to_string().as_str()); @@ -274,6 +294,7 @@ mod test { expect!["40"].assert_eq(size_of::().to_string().as_str()); #[cfg(target_arch = "aarch64")] expect!["48"].assert_eq(size_of::().to_string().as_str()); + expect!["16"].assert_eq(size_of::().to_string().as_str()); // xdr types expect!["8"].assert_eq(size_of::().to_string().as_str()); expect!["8"].assert_eq(size_of::().to_string().as_str()); @@ -306,9 +327,9 @@ mod test { expect!["120"].assert_eq(size_of::().to_string().as_str()); expect!["160"].assert_eq(size_of::().to_string().as_str()); expect!["64"].assert_eq(size_of::().to_string().as_str()); - expect!["104"].assert_eq(size_of::().to_string().as_str()); + expect!["96"].assert_eq(size_of::().to_string().as_str()); expect!["120"].assert_eq(size_of::().to_string().as_str()); - expect!["264"].assert_eq(size_of::().to_string().as_str()); + expect!["256"].assert_eq(size_of::().to_string().as_str()); expect!["1"].assert_eq(size_of::().to_string().as_str()); expect!["40"].assert_eq(size_of::().to_string().as_str()); expect!["128"].assert_eq(size_of::().to_string().as_str()); @@ -319,8 +340,8 @@ mod test { expect!["24"].assert_eq(size_of::().to_string().as_str()); expect!["24"].assert_eq(size_of::().to_string().as_str()); expect!["24"].assert_eq(size_of::().to_string().as_str()); - expect!["99"].assert_eq(size_of::().to_string().as_str()); - expect!["66"].assert_eq(size_of::().to_string().as_str()); + expect!["98"].assert_eq(size_of::().to_string().as_str()); + expect!["65"].assert_eq(size_of::().to_string().as_str()); expect!["4"].assert_eq(size_of::().to_string().as_str()); expect!["4"].assert_eq(size_of::().to_string().as_str()); expect!["0"].assert_eq(size_of::().to_string().as_str()); @@ -329,6 +350,28 @@ mod test { .to_string() .as_str(), ); + expect!["136"].assert_eq(size_of::().to_string().as_str()); + expect!["32"].assert_eq( + size_of::() + .to_string() + .as_str(), + ); + expect!["232"].assert_eq( + size_of::() + .to_string() + .as_str(), + ); + expect!["192"].assert_eq( + size_of::() + .to_string() + .as_str(), + ); + expect!["40"].assert_eq( + size_of::() + .to_string() + .as_str(), + ); + expect!["16"].assert_eq(size_of::().to_string().as_str()); // composite types expect!["16"].assert_eq(size_of::<&[ScVal]>().to_string().as_str()); expect!["72"].assert_eq(size_of::<(Val, ScVal)>().to_string().as_str()); @@ -372,6 +415,7 @@ mod test { assert_mem_size_le_declared_size!(i64); assert_mem_size_le_declared_size!(u128); assert_mem_size_le_declared_size!(i128); + assert_mem_size_le_declared_size!(usize); // Val-wrapping types assert_mem_size_le_declared_size!(Val); assert_mem_size_le_declared_size!(Void); @@ -468,6 +512,12 @@ mod test { assert_mem_size_le_declared_size!(ExtensionPoint); assert_mem_size_le_declared_size!(SorobanAuthorizedInvocation); assert_mem_size_le_declared_size!(SorobanAuthorizationEntry); + assert_mem_size_le_declared_size!(AuthorizedInvocation); + assert_mem_size_le_declared_size!(AuthorizedInvocationSnapshot); + assert_mem_size_le_declared_size!(AccountAuthorizationTracker); + assert_mem_size_le_declared_size!(InvokerContractAuthorizationTracker); + assert_mem_size_le_declared_size!(AccountAuthorizationTrackerSnapshot); + assert_mem_size_le_declared_size!(ContractInvocation); // composite types assert_mem_size_le_declared_size!(&[ScVal]); assert_mem_size_le_declared_size!((Val, ScVal)); diff --git a/soroban-env-host/src/host/metered_clone.rs b/soroban-env-host/src/host/metered_clone.rs index 839c2176e..5e631c2b5 100644 --- a/soroban-env-host/src/host/metered_clone.rs +++ b/soroban-env-host/src/host/metered_clone.rs @@ -1,4 +1,4 @@ -use std::{mem, rc::Rc}; +use std::{iter::FromIterator, mem, rc::Rc}; use soroban_env_common::xdr::{DepthLimiter, SorobanAuthorizationEntry}; @@ -34,7 +34,7 @@ use super::declared_size::DeclaredSizeForMetering; // unit is number of elements `n_elts` multiplied by a declared size of each element. In a better // world we would multiply by `size_of` instead but that's not guaranteed to be stable, which // might cause metering to differ across compilations, causing problems in concensus and replay. -pub(crate) fn charge_shallow_copy( +pub(crate) fn charge_shallow_copy( n_elts: u64, budget: &Budget, ) -> Result<(), HostError> { @@ -57,7 +57,7 @@ pub(crate) fn charge_shallow_copy( // Let it be a free function instead of a trait because charge_heap_alloc maybe called elsewhere, // not just metered clone, e.g. Box::::new(). -pub(crate) fn charge_heap_alloc( +pub(crate) fn charge_heap_alloc( n_elts: u64, budget: &Budget, ) -> Result<(), HostError> { @@ -75,16 +75,58 @@ pub(crate) fn charge_heap_alloc( ) } -// A convenience method for a container bulk initialization with elements, e.g. a -// `Vec::with_capacity(N)` immediately followed by N element pushes. -pub(crate) fn charge_container_bulk_init_with_elts( - n_elts: u64, - budget: &Budget, -) -> Result<(), HostError> { - debug_assert!(!C::IS_SHALLOW); - charge_shallow_copy::(1, budget)?; - charge_heap_alloc::(n_elts, budget)?; - charge_shallow_copy::(n_elts, budget) +/// Represents a collection type which can be created from an iterator, and provides +/// a method for bulk charging its initalization cost. +pub trait MeteredContainer: FromIterator + DeclaredSizeForMetering +where + Self: Sized, +{ + type Item: DeclaredSizeForMetering; + + fn charge_bulk_init(len: u64, budget: &Budget) -> Result<(), HostError> { + charge_shallow_copy::(1, budget)?; + charge_heap_alloc::(len, budget)?; + charge_shallow_copy::(len, budget) + } +} + +impl MeteredContainer for Vec { + type Item = T; +} + +impl MeteredContainer + for Result, E> +{ + type Item = Result; +} + +/// Represents an iterator which can collect its elements into a `MeteredContainer` +/// and automatically account for the metering cost. +/// +/// Returns: +/// Err - if either the budget was exceeded or the iterator lacks the size hint +/// Ok - otherwise +pub trait MeteredCollect: Iterator +where + Self::Item: DeclaredSizeForMetering, + Self: Sized, +{ + fn metered_collect>( + self, + budget: &Budget, + ) -> Result { + if let (_, Some(ub)) = self.size_hint() { + B::charge_bulk_init(ub as u64, budget)?; + Ok(self.collect()) + } else { + Err((ScErrorType::Object, ScErrorCode::InternalError).into()) + } + } +} + +impl MeteredCollect for std::iter::Map where + F: FnMut(I::Item) -> T +{ } pub trait MeteredClone: Clone + DeclaredSizeForMetering { diff --git a/soroban-env-host/src/native_contract/account_contract.rs b/soroban-env-host/src/native_contract/account_contract.rs index b8c780547..57cf15f07 100644 --- a/soroban-env-host/src/native_contract/account_contract.rs +++ b/soroban-env-host/src/native_contract/account_contract.rs @@ -104,6 +104,7 @@ impl AuthorizationContext { } } +// metering: covered fn invocation_tree_to_auth_contexts( host: &Host, invocation: &AuthorizedInvocation, @@ -119,6 +120,7 @@ fn invocation_tree_to_auth_contexts( Ok(()) } +// metering: covered pub(crate) fn check_account_contract_auth( host: &Host, account_contract: &Hash, @@ -142,6 +144,7 @@ pub(crate) fn check_account_contract_auth( .try_into()?) } +// metering: covered pub(crate) fn check_account_authentication( host: &Host, account_id: AccountId, diff --git a/soroban-env-host/src/native_contract/base_types.rs b/soroban-env-host/src/native_contract/base_types.rs index 5c23381e5..e711e9ddd 100644 --- a/soroban-env-host/src/native_contract/base_types.rs +++ b/soroban-env-host/src/native_contract/base_types.rs @@ -57,6 +57,7 @@ impl From for StringObject { } } +// TODO: check metering impl String { pub fn copy_to_rust_string(&self, env: &Host) -> Result { let len: u32 = env @@ -147,6 +148,7 @@ impl From> for Bytes { } } +// TODO: check metering impl Bytes { pub fn push(&mut self, x: u8) -> Result<(), HostError> { let x32: u32 = x.into(); diff --git a/soroban-env-host/src/native_contract/invoker_contract_auth.rs b/soroban-env-host/src/native_contract/invoker_contract_auth.rs index 6ed085a2b..434b7353f 100644 --- a/soroban-env-host/src/native_contract/invoker_contract_auth.rs +++ b/soroban-env-host/src/native_contract/invoker_contract_auth.rs @@ -1,6 +1,7 @@ use super::account_contract::{ContractAuthorizationContext, CreateContractHostFnContext}; use super::common_types::ContractExecutable; -use crate::host::metered_clone::MeteredClone; +use crate::budget::AsBudget; +use crate::host::metered_clone::{MeteredClone, MeteredContainer}; use crate::host_object::HostVec; use crate::{ auth::{AuthorizedFunction, AuthorizedInvocation, ContractFunction}, @@ -32,6 +33,7 @@ pub enum InvokerContractAuthEntry { CreateContractHostFn(CreateContractHostFnContext), } +// metering: covered impl InvokerContractAuthEntry { fn to_authorized_invocation( &self, @@ -48,18 +50,23 @@ impl InvokerContractAuthEntry { |v: &HostVec| v.to_vec(host.budget_ref()), )?, }); - let sub_invocations: Vec = host.visit_obj( + let mut sub_invocations: Vec = vec![]; + host.visit_obj( contract_invocation.sub_invocations.as_object(), |v: &HostVec| { - v.iter() - .map(|val| InvokerContractAuthEntry::try_from_val(host, val)) - .collect::, HostError>>() + Vec::::charge_bulk_init( + v.len() as u64, + host.as_budget(), + )?; + sub_invocations.reserve(v.len()); + for val in v.iter() { + let entry = InvokerContractAuthEntry::try_from_val(host, val)?; + sub_invocations + .push(entry.to_authorized_invocation(host, invoker_contract_addr)?); + } + Ok(()) }, )?; - let sub_invocations = sub_invocations - .into_iter() - .map(|i| i.to_authorized_invocation(host, invoker_contract_addr)) - .collect::, HostError>>()?; Ok(AuthorizedInvocation::new(function, sub_invocations)) } InvokerContractAuthEntry::CreateContractHostFn(create_contract_fn) => { @@ -94,6 +101,7 @@ impl InvokerContractAuthEntry { } } +// metering: covered pub(crate) fn invoker_contract_auth_to_authorized_invocation( host: &Host, invoker_contract_addr: &ScAddress, diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index e6c5a6bf5..66bf19bb2 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -1,6 +1,7 @@ use crate::{ - budget::AsBudget, - host::{metered_clone::MeteredClone, metered_xdr::metered_write_xdr}, + budget::{AsBudget, Budget}, + host::metered_clone::{MeteredClone, MeteredCollect}, + host::metered_xdr::metered_write_xdr, xdr::{ContractCostType, ScMap, ScMapEntry, ScVal}, Env, Host, HostError, Symbol, Val, }; @@ -221,6 +222,19 @@ fn test_recursive_type_clone() -> Result<(), HostError> { Ok(()) } +#[test] +fn test_metered_collection() -> Result<(), HostError> { + let budget = Budget::default(); + let v: Vec = vec![1, 2, -3, 4, -6, -11]; + let res = v + .iter() + .filter(|i| i.abs() > 3) + .map(|i| Ok(i.abs() as u64)) + .metered_collect::, HostError>>(&budget)??; + assert_eq!(res, vec![4, 6, 11]); + Ok(()) +} + // This test is a sanity check to make sure we didn't accidentally change the cost schedule. // If the cost schedule have changed, need to update this test by running // `UPDATE_EXPECT=true cargo test` From 4bccd21e6990aa7ae1c97559a7d6d5ca50ae8ce6 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Tue, 1 Aug 2023 14:54:18 -0400 Subject: [PATCH 2/3] Clean up frame metering, add depth limit check --- .../benches/common/cost_types/guard_frame.rs | 33 ------------------ .../benches/common/cost_types/mod.rs | 2 -- soroban-env-host/benches/common/mod.rs | 1 - .../src/cost_runner/cost_types/guard_frame.rs | 34 ------------------- .../src/cost_runner/cost_types/mod.rs | 2 -- soroban-env-host/src/host/declared_size.rs | 14 ++++---- soroban-env-host/src/host/frame.rs | 17 ++++++---- 7 files changed, 19 insertions(+), 84 deletions(-) delete mode 100644 soroban-env-host/benches/common/cost_types/guard_frame.rs delete mode 100644 soroban-env-host/src/cost_runner/cost_types/guard_frame.rs diff --git a/soroban-env-host/benches/common/cost_types/guard_frame.rs b/soroban-env-host/benches/common/cost_types/guard_frame.rs deleted file mode 100644 index 86e67222e..000000000 --- a/soroban-env-host/benches/common/cost_types/guard_frame.rs +++ /dev/null @@ -1,33 +0,0 @@ -use crate::common::HostCostMeasurement; -use soroban_env_host::{ - cost_runner::GuardFrameRun, - xdr::{ContractExecutable, Hash, ScContractInstance}, - Symbol, -}; - -pub(crate) struct GuardFrameMeasure; - -impl HostCostMeasurement for GuardFrameMeasure { - type Runner = GuardFrameRun; - - fn new_best_case( - _host: &soroban_env_host::Host, - _rng: &mut rand::prelude::StdRng, - ) -> (Hash, Symbol, ScContractInstance) { - let id: Hash = [0; 32].into(); - let fun: Symbol = Symbol::try_from_small_str("add").unwrap(); - let empty_instance = ScContractInstance { - executable: ContractExecutable::Token, - storage: None, - }; - (id, fun, empty_instance) - } - - fn new_random_case( - host: &soroban_env_host::Host, - rng: &mut rand::prelude::StdRng, - _input: u64, - ) -> (Hash, Symbol, ScContractInstance) { - Self::new_best_case(host, rng) - } -} diff --git a/soroban-env-host/benches/common/cost_types/mod.rs b/soroban-env-host/benches/common/cost_types/mod.rs index 3629389a8..59d814f7a 100644 --- a/soroban-env-host/benches/common/cost_types/mod.rs +++ b/soroban-env-host/benches/common/cost_types/mod.rs @@ -4,7 +4,6 @@ mod compute_ecdsa_secp256k1_sig; mod compute_ed25519_pubkey; mod compute_keccak256_hash; mod compute_sha256_hash; -mod guard_frame; mod host_mem_alloc; mod host_mem_cmp; mod host_mem_cpy; @@ -27,7 +26,6 @@ pub(crate) use compute_ecdsa_secp256k1_sig::*; pub(crate) use compute_ed25519_pubkey::*; pub(crate) use compute_keccak256_hash::*; pub(crate) use compute_sha256_hash::*; -pub(crate) use guard_frame::*; pub(crate) use host_mem_alloc::*; pub(crate) use host_mem_cmp::*; pub(crate) use host_mem_cpy::*; diff --git a/soroban-env-host/benches/common/mod.rs b/soroban-env-host/benches/common/mod.rs index 38cc51370..f6484fa92 100644 --- a/soroban-env-host/benches/common/mod.rs +++ b/soroban-env-host/benches/common/mod.rs @@ -73,7 +73,6 @@ pub(crate) fn for_each_host_cost_measurement( call_bench::(&mut params)?; call_bench::(&mut params)?; call_bench::(&mut params)?; - call_bench::(&mut params)?; call_bench::(&mut params)?; call_bench::(&mut params)?; call_bench::(&mut params)?; diff --git a/soroban-env-host/src/cost_runner/cost_types/guard_frame.rs b/soroban-env-host/src/cost_runner/cost_types/guard_frame.rs deleted file mode 100644 index 48116e6b8..000000000 --- a/soroban-env-host/src/cost_runner/cost_types/guard_frame.rs +++ /dev/null @@ -1,34 +0,0 @@ -use std::hint::black_box; - -use soroban_env_common::xdr::ScContractInstance; - -use crate::{cost_runner::CostRunner, host::Frame, xdr::ContractCostType, xdr::Hash, Symbol, Val}; - -pub struct GuardFrameRun; - -impl CostRunner for GuardFrameRun { - const COST_TYPE: ContractCostType = ContractCostType::GuardFrame; - - type SampleType = (Hash, Symbol, ScContractInstance); - - type RecycledType = Option; - - fn run_iter(host: &crate::Host, _iter: u64, sample: Self::SampleType) -> Self::RecycledType { - black_box( - host.with_frame(Frame::Token(sample.0, sample.1, vec![], sample.2), || { - Ok(Val::VOID.to_val()) - }) - .unwrap(), - ); - black_box(None) - } - - fn run_baseline_iter( - host: &crate::Host, - _iter: u64, - sample: Self::SampleType, - ) -> Self::RecycledType { - black_box(host.charge_budget(Self::COST_TYPE, None).unwrap()); - black_box(Some(sample)) - } -} diff --git a/soroban-env-host/src/cost_runner/cost_types/mod.rs b/soroban-env-host/src/cost_runner/cost_types/mod.rs index 246c053c7..1e994c386 100644 --- a/soroban-env-host/src/cost_runner/cost_types/mod.rs +++ b/soroban-env-host/src/cost_runner/cost_types/mod.rs @@ -4,7 +4,6 @@ mod compute_ecdsa_secp256k1_sig; mod compute_ed25519_pubkey; mod compute_keccak256_hash; mod compute_sha256_hash; -mod guard_frame; mod host_mem_alloc; mod host_mem_cmp; mod host_mem_cpy; @@ -27,7 +26,6 @@ pub use compute_ecdsa_secp256k1_sig::*; pub use compute_ed25519_pubkey::*; pub use compute_keccak256_hash::*; pub use compute_sha256_hash::*; -pub use guard_frame::*; pub use host_mem_alloc::*; pub use host_mem_cmp::*; pub use host_mem_cpy::*; diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 27ec9f328..60ce2f320 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -8,7 +8,7 @@ use crate::{ AuthorizedInvocationSnapshot, ContractInvocation, InvokerContractAuthorizationTracker, }, events::{EventError, HostEvent, InternalContractEvent, InternalEvent}, - host::Events, + host::{frame::Context, Events}, host_object::HostObject, storage::AccessType, xdr::{ @@ -103,6 +103,7 @@ impl_declared_size_type!(U256, 32); impl_declared_size_type!(I256, 32); impl_declared_size_type!(HostObject, 48); impl_declared_size_type!(HostError, 16); +impl_declared_size_type!(Context, 512); // xdr types impl_declared_size_type!(TimePoint, 8); impl_declared_size_type!(Duration, 8); @@ -295,6 +296,10 @@ mod test { #[cfg(target_arch = "aarch64")] expect!["48"].assert_eq(size_of::().to_string().as_str()); expect!["16"].assert_eq(size_of::().to_string().as_str()); + #[cfg(target_arch = "x86_64")] + expect!["512"].assert_eq(size_of::().to_string().as_str()); + #[cfg(target_arch = "aarch64")] + expect!["488"].assert_eq(size_of::().to_string().as_str()); // xdr types expect!["8"].assert_eq(size_of::().to_string().as_str()); expect!["8"].assert_eq(size_of::().to_string().as_str()); @@ -383,11 +388,6 @@ mod test { expect!["8"].assert_eq(size_of::>().to_string().as_str()); expect!["64"].assert_eq(size_of::>().to_string().as_str()); expect!["64"].assert_eq(size_of::().to_string().as_str()); - expect!["240"].assert_eq( - size_of::() - .to_string() - .as_str(), - ); } // This is the actual test. @@ -462,6 +462,8 @@ mod test { assert_mem_size_le_declared_size!(U256); assert_mem_size_le_declared_size!(I256); assert_mem_size_le_declared_size!(HostObject); + assert_mem_size_le_declared_size!(HostError); + assert_mem_size_le_declared_size!(Context); // xdr types assert_mem_size_le_declared_size!(TimePoint); assert_mem_size_le_declared_size!(Duration); diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index 70a767b04..de2bea6ed 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -7,8 +7,9 @@ use crate::{ auth::AuthorizationManagerSnapshot, budget::AsBudget, storage::{InstanceStorageMap, StorageMap}, - xdr::{ContractCostType, ContractExecutable, Hash, HostFunction, HostFunctionType, ScVal}, + xdr::{ContractExecutable, Hash, HostFunction, HostFunctionType, ScVal}, Error, Host, HostError, Object, Symbol, SymbolStr, TryFromVal, TryIntoVal, Val, + DEFAULT_HOST_DEPTH_LIMIT, }; #[cfg(any(test, feature = "testutils"))] @@ -23,7 +24,7 @@ use crate::Vm; use super::{ invoker_type::InvokerType, - metered_clone::{self, MeteredClone}, + metered_clone::{self, MeteredClone, MeteredContainer}, prng::Prng, }; @@ -132,6 +133,7 @@ impl Host { prng: None, storage: None, }; + Vec::::charge_bulk_init(1, self.as_budget())?; self.try_borrow_context_mut()?.push(ctx); Ok(RollbackPoint { storage: self.try_borrow_storage()?.map.clone(), @@ -320,15 +322,18 @@ impl Host { /// Pushes a [`Frame`], runs a closure, and then pops the frame, rolling back /// if the closure returned an error. Returns the result that the closure /// returned (or any error caused during the frame push/pop). - // Notes on metering: `GuardFrame` charges on the work done on protecting the `context`. - // It does not cover the cost of the actual closure call. The closure needs to be - // metered separately. pub(crate) fn with_frame(&self, frame: Frame, f: F) -> Result where F: FnOnce() -> Result, { - self.charge_budget(ContractCostType::GuardFrame, None)?; let start_depth = self.try_borrow_context()?.len(); + if start_depth as u32 == DEFAULT_HOST_DEPTH_LIMIT { + return Err(Error::from_type_and_code( + ScErrorType::Context, + ScErrorCode::ExceededLimit, + ) + .into()); + } let rp = self.push_frame(frame)?; let res = f(); let res = if let Ok(v) = res { From aeeb92f8f5f7c2c011000b767b0765a9e6391c46 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Tue, 1 Aug 2023 15:23:55 -0400 Subject: [PATCH 3/3] Enable `test_expected_size` test to catch divergence on x86 --- soroban-env-host/src/host/declared_size.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 60ce2f320..408d1ce40 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -230,7 +230,6 @@ mod test { // This section is for outputting the actual size of types. They are for informational use. // They might become outdated due to Rust type changes, and numbers may differ between // platforms. Run `UPDATE_EXPECT=true cargo test` to update this. - #[ignore] #[test] fn test_expected_size() { use expect_test::expect;