Skip to content

Commit

Permalink
Remove DepthGuard; Remove EnvBase dependency on DepthLimiter; c…
Browse files Browse the repository at this point in the history
…lean-ups
  • Loading branch information
jayz22 committed Jul 12, 2023
1 parent bda0ff1 commit 1f4546f
Show file tree
Hide file tree
Showing 15 changed files with 158 additions and 205 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ soroban-native-sdk-macros = { version = "0.0.17", path = "soroban-native-sdk-mac
[workspace.dependencies.stellar-xdr]
version = "0.0.17"
git = "https://github.com/stellar/rs-stellar-xdr"
rev = "3f5fdb4c485d5539245474dd62df7b685f2258e0"
rev = "2000347692d6f07b8875225cbbd723656c3a00c1"
default-features = false

[workspace.dependencies.wasmi]
Expand Down
8 changes: 1 addition & 7 deletions soroban-env-common/src/compare.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[cfg(feature = "std")]
use std::rc::Rc;

use crate::{val::ValConvert, DepthGuard, Env, Error, Tag, Val};
use crate::{val::ValConvert, Env, Tag, Val};
use core::cmp::Ordering;

/// General trait representing the ability to compare two values of some type.
Expand Down Expand Up @@ -126,12 +126,6 @@ impl<E: Env> Compare<Val> for E {
type Error = E::Error;

fn compare(&self, a: &Val, b: &Val) -> Result<Ordering, Self::Error> {
let _dg = DepthGuard::new(self).map_err(|_| {
Error::from_type_and_code(
stellar_xdr::ScErrorType::Context,
stellar_xdr::ScErrorCode::ExceededLimit,
)
});
if a.get_payload() == b.get_payload() {
// Fast-path exactly-equal values.
return Ok(Ordering::Equal);
Expand Down
25 changes: 0 additions & 25 deletions soroban-env-common/src/depth_guard.rs

This file was deleted.

3 changes: 1 addition & 2 deletions soroban-env-common/src/env.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use soroban_env_macros::generate_call_macro_with_all_host_functions;
use stellar_xdr::DepthLimiter;

use super::Symbol;
use super::{
Expand All @@ -11,7 +10,7 @@ use core::any;

/// Base trait extended by the [Env](crate::Env) trait, providing various special-case
/// functions that do _not_ simply call across cross the guest/host interface.
pub trait EnvBase: Sized + Clone + DepthLimiter {
pub trait EnvBase: Sized + Clone {
/// The type of error returned from the environment when the environment
/// itself fails "unrecoverably", or at least in a way that the user is not
/// expected to be able to recover from, such as an internal logic error,
Expand Down
2 changes: 0 additions & 2 deletions soroban-env-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ mod arbitrary;
mod bytes;
mod compare;
mod convert;
mod depth_guard;
mod env;
mod error;
mod object;
Expand Down Expand Up @@ -84,7 +83,6 @@ pub use val::{ConversionError, Tag, Val};

pub use compare::Compare;
pub use convert::{Convert, TryFromVal, TryIntoVal};
pub use depth_guard::DepthGuard;
pub use env::{call_macro_with_all_host_functions, Env, EnvBase};
pub use unimplemented_env::UnimplementedEnv;
pub use vmcaller_env::{VmCaller, VmCallerEnv};
Expand Down
6 changes: 3 additions & 3 deletions soroban-env-common/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
TryFromVal, Val,
};
use core::{cmp::Ordering, fmt::Debug};
use stellar_xdr::{DepthLimiter, Duration, ScVal, TimePoint};
use stellar_xdr::{Duration, ScVal, TimePoint};

/// Wrapper for a [Val] that is tagged with one of the object types,
/// interpreting the [Val]'s body as containing a 32-bit object-code handle
Expand Down Expand Up @@ -61,7 +61,7 @@ impl From<ScValObject> for ScVal {

impl<E> TryFromVal<E, Object> for ScValObject
where
E: Env + Convert<Object, ScValObject> + DepthLimiter,
E: Env + Convert<Object, ScValObject>,
{
type Error = <E as Convert<Object, ScValObject>>::Error;
fn try_from_val(env: &E, val: &Object) -> Result<Self, Self::Error> {
Expand All @@ -71,7 +71,7 @@ where

impl<'a, E> TryFromVal<E, ScValObjRef<'a>> for Object
where
E: Env + Convert<ScValObjRef<'a>, Object> + DepthLimiter,
E: Env + Convert<ScValObjRef<'a>, Object>,
{
type Error = ConversionError;

Expand Down
12 changes: 0 additions & 12 deletions soroban-env-common/src/unimplemented_env.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use stellar_xdr::DepthLimiter;

use super::{call_macro_with_all_host_functions, Env, EnvBase, Symbol};
use super::{
AddressObject, Bool, BytesObject, DurationObject, Error, I128Object, I256Object, I256Val,
Expand All @@ -13,16 +11,6 @@ use core::{any, convert::Infallible};
#[derive(Clone, Default)]
pub struct UnimplementedEnv;

impl DepthLimiter for UnimplementedEnv {
type DepthError = <Self as EnvBase>::Error;

fn enter(&self) -> Result<(), Self::DepthError> {
Ok(())
}

fn leave(&self) {}
}

impl EnvBase for UnimplementedEnv {
type Error = Infallible;

Expand Down
4 changes: 2 additions & 2 deletions soroban-env-common/src/wrapper_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ macro_rules! impl_tryfroms_and_tryfromvals_delegating_to_rawvalconvertible {
Self::try_from(*v)
}
}
impl<E: $crate::Env + stellar_xdr::DepthLimiter> $crate::TryFromVal<E, $crate::Val> for $T {
impl<E: $crate::Env> $crate::TryFromVal<E, $crate::Val> for $T {
type Error = $crate::ConversionError;
#[inline(always)]
fn try_from_val(_env: &E, val: &$crate::Val) -> Result<Self, Self::Error> {
Self::try_from(*val)
}
}
impl<E: $crate::Env + stellar_xdr::DepthLimiter> $crate::TryFromVal<E, $T> for $crate::Val {
impl<E: $crate::Env> $crate::TryFromVal<E, $T> for $crate::Val {
type Error = $crate::ConversionError;
fn try_from_val(_env: &E, val: &$T) -> Result<Self, Self::Error> {
Ok((*val).into())
Expand Down
13 changes: 1 addition & 12 deletions soroban-env-guest/src/guest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use core::convert::Infallible;

use soroban_env_common::{call_macro_with_all_host_functions, xdr::DepthLimiter};
use soroban_env_common::call_macro_with_all_host_functions;

use super::{
AddressObject, Bool, BytesObject, DurationObject, Error, I128Object, I256Object, I256Val,
Expand All @@ -22,17 +22,6 @@ use static_assertions as sa;
#[derive(Copy, Clone, Default)]
pub struct Guest;

#[cfg(not(target_family = "wasm"))]
impl DepthLimiter for Guest {
type DepthError = <Self as EnvBase>::Error;

fn enter(&self) -> Result<(), Self::DepthError> {
Ok(())
}

fn leave(&self) {}
}

// The Guest struct is only meaningful when compiling for the WASM target. All
// these fns should not be called at all because the SDK's choice of Env should be
// Host for a non-WASM build.
Expand Down
48 changes: 30 additions & 18 deletions soroban-env-host/src/budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ use std::{
rc::Rc,
};

use soroban_env_common::xdr::{ScErrorCode, ScErrorType};

use crate::{
host::error::TryBorrowOrErr,
xdr::{
ContractCostParamEntry, ContractCostParams, ContractCostType, DepthLimiter, ExtensionPoint,
ScErrorCode, ScErrorType,
},
Error, Host, HostError, DEFAULT_HOST_DEPTH_LIMIT,
};
Expand Down Expand Up @@ -412,6 +411,30 @@ impl Display for BudgetImpl {
}
}

impl DepthLimiter for BudgetImpl {
type DepthLimiterError = HostError;

fn enter(&mut self) -> Result<(), HostError> {
if let Some(depth) = self.depth_limit.checked_sub(1) {
self.depth_limit = depth;
} else {
return Err(Error::from_type_and_code(
ScErrorType::Context,
ScErrorCode::ExceededLimit,
)
.into());
}
Ok(())
}

// `leave` should be called in tandem with `enter` such that the depth
// doesn't exceed the initial depth limit.
fn leave(&mut self) -> Result<(), HostError> {
self.depth_limit = self.depth_limit.saturating_add(1);
Ok(())
}
}

#[derive(Default, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub struct Budget(pub(crate) Rc<RefCell<BudgetImpl>>);

Expand Down Expand Up @@ -450,25 +473,14 @@ impl AsBudget for &Host {
}

impl DepthLimiter for Budget {
type DepthError = HostError;
type DepthLimiterError = HostError;

fn enter(&self) -> Result<(), HostError> {
let depth = self.0.borrow().depth_limit;
if depth == 0 {
return Err(Error::from_type_and_code(
ScErrorType::Context,
ScErrorCode::ExceededLimit,
)
.into());
}
self.0.borrow_mut().depth_limit -= 1;
Ok(())
fn enter(&mut self) -> Result<(), HostError> {
self.0.try_borrow_mut_or_err()?.enter()
}

fn leave(&self) {
// no need to do saturating_add because `leave` must be called after `enter`,
// thus the `depth_limit` cannot exceed the initial value.
self.0.borrow_mut().depth_limit += 1;
fn leave(&mut self) -> Result<(), HostError> {
self.0.try_borrow_mut_or_err()?.leave()
}
}

Expand Down
23 changes: 5 additions & 18 deletions soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ use crate::{
LedgerEntryData, LedgerKey, LedgerKeyContractCode, PublicKey, ScAddress, ScBytes,
ScErrorType, ScString, ScSymbol, ScVal, TimePoint,
},
AddressObject, Bool, BytesObject, ConversionError, DepthGuard, Error, I128Object, I256Object,
MapObject, StorageType, StringObject, SymbolObject, SymbolSmall, SymbolStr, TryFromVal,
U128Object, U256Object, U32Val, U64Val, VecObject, VmCaller, VmCallerEnv, Void, I256, U256,
AddressObject, Bool, BytesObject, ConversionError, Error, I128Object, I256Object, MapObject,
StorageType, StringObject, SymbolObject, SymbolSmall, SymbolStr, TryFromVal, U128Object,
U256Object, U32Val, U64Val, VecObject, VmCaller, VmCallerEnv, Void, I256, U256,
};

use crate::Vm;
Expand All @@ -49,8 +49,8 @@ mod validity;
pub use error::HostError;
use soroban_env_common::xdr::{
ContractCodeEntryBody, ContractDataDurability, ContractDataEntryBody, ContractDataEntryData,
ContractEntryBodyType, ContractIdPreimage, ContractIdPreimageFromAddress, DepthLimiter,
ScContractInstance, ScErrorCode, MASK_CONTRACT_DATA_FLAGS_V20,
ContractEntryBodyType, ContractIdPreimage, ContractIdPreimageFromAddress, ScContractInstance,
ScErrorCode, MASK_CONTRACT_DATA_FLAGS_V20,
};

use self::metered_clone::MeteredClone;
Expand Down Expand Up @@ -227,18 +227,6 @@ impl_checked_borrow_helpers!(
try_borrow_previous_authorization_manager_mut
);

impl DepthLimiter for Host {
type DepthError = HostError;

fn enter(&self) -> Result<(), HostError> {
self.0.budget.enter()
}

fn leave(&self) {
self.0.budget.leave()
}
}

impl Debug for HostImpl {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "HostImpl(...)")
Expand Down Expand Up @@ -1172,7 +1160,6 @@ impl VmCallerEnv for Host {

// Metered: covered by `visit` and `metered_cmp`.
fn obj_cmp(&self, _vmcaller: &mut VmCaller<Host>, a: Val, b: Val) -> Result<i64, HostError> {
let dg = DepthGuard::new(self);
let res = match unsafe {
match (Object::try_from(a), Object::try_from(b)) {
// We were given two objects: compare them.
Expand Down
Loading

0 comments on commit 1f4546f

Please sign in to comment.