Skip to content

Commit

Permalink
Code cleanup around PRNG (#1064)
Browse files Browse the repository at this point in the history 
* Remove dead and move around some code

* [prng] avoid using core::mem:size_of

* fixup! [prng] avoid using core::mem:size_of
  • Loading branch information
@jayz22
jayz22 authored Sep 15, 2023
1 parent 6460cf6 commit 1dccd1c
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 136 deletions.
70 changes: 1 addition & 69 deletions soroban-env-host/src/host.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ mod data_helper;
pub(crate) mod declared_size;
pub(crate) mod error;
pub(crate) mod frame;
pub(crate) mod invoker_type;
pub(crate) mod ledger_info_helper;
mod mem_helper;
pub(crate) mod metered_clone;
Expand Down Expand Up @@ -939,73 +938,6 @@ impl Host {

Ok(())
}

fn decorate_contract_data_storage_error(&self, err: HostError, key: Val) -> HostError {
if !err.error.is_type(ScErrorType::Storage) {
return err;
}
if err.error.is_code(ScErrorCode::ExceededLimit) {
return self.err(
ScErrorType::Storage,
ScErrorCode::ExceededLimit,
"trying to access contract storage key outside of the correct footprint",
&[key],
);
}
if err.error.is_code(ScErrorCode::MissingValue) {
return self.err(
ScErrorType::Storage,
ScErrorCode::MissingValue,
"trying to get non-existing value for contract storage key",
&[key],
);
}
err
}

pub(crate) fn decorate_contract_instance_storage_error(
&self,
err: HostError,
contract_id: &Hash,
) -> HostError {
if err.error.is_type(ScErrorType::Storage) && err.error.is_code(ScErrorCode::ExceededLimit)
{
return self.err(
ScErrorType::Storage,
ScErrorCode::ExceededLimit,
"trying to access contract instance key outside of the correct footprint",
// No need for metered clone here as we are on the unrecoverable
// error path.
&[self
.add_host_object(ScAddress::Contract(contract_id.clone()))
.map(|a| a.into())
.unwrap_or(Val::VOID.into())],
);
}
err
}

pub(crate) fn decorate_contract_code_storage_error(
&self,
err: HostError,
wasm_hash: &Hash,
) -> HostError {
if err.error.is_type(ScErrorType::Storage) && err.error.is_code(ScErrorCode::ExceededLimit)
{
return self.err(
ScErrorType::Storage,
ScErrorCode::ExceededLimit,
"trying to access contract code key outside of the correct footprint",
// No need for metered clone here as we are on the unrecoverable
// error path.
&[self
.add_host_object(self.scbytes_from_hash(wasm_hash).unwrap_or_default())
.map(|a| a.into())
.unwrap_or(Val::VOID.into())],
);
}
err
}
}

// Notes on metering: these are called from the guest and thus charged on the VM instructions.
Expand Down Expand Up @@ -3105,7 +3037,7 @@ impl VmCallerEnv for Host {
) -> Result<Void, Self::Error> {
self.visit_obj(seed, |bytes: &ScBytes| {
let slice: &[u8] = bytes.as_ref();
self.charge_budget(ContractCostType::HostMemCpy, Some(prng::SEED_BYTES as u64))?;
self.charge_budget(ContractCostType::HostMemCpy, Some(prng::SEED_BYTES))?;
if let Ok(seed32) = slice.try_into() {
self.with_current_prng(|prng| {
*prng = Prng::new_from_seed(seed32);
Expand Down
79 changes: 73 additions & 6 deletions soroban-env-host/src/host/error.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
use crate::{
budget::AsBudget,
events::Events,
xdr::{self, ScError},
EnvBase, Error, Host,
xdr::{self, Hash, ScAddress, ScError, ScErrorCode, ScErrorType},
ConversionError, EnvBase, Error, Host, TryFromVal, U32Val, Val,
};
use backtrace::{Backtrace, BacktraceFrame};
use core::fmt::Debug;
use soroban_env_common::{
xdr::{ScErrorCode, ScErrorType},
ConversionError, TryFromVal, U32Val, Val,
};
use std::{
cell::{Ref, RefCell, RefMut},
ops::DerefMut,
Expand Down Expand Up @@ -326,6 +322,77 @@ impl Host {
}
})
}

pub(crate) fn decorate_contract_data_storage_error(
&self,
err: HostError,
key: Val,
) -> HostError {
if !err.error.is_type(ScErrorType::Storage) {
return err;
}
if err.error.is_code(ScErrorCode::ExceededLimit) {
return self.err(
ScErrorType::Storage,
ScErrorCode::ExceededLimit,
"trying to access contract storage key outside of the correct footprint",
&[key],
);
}
if err.error.is_code(ScErrorCode::MissingValue) {
return self.err(
ScErrorType::Storage,
ScErrorCode::MissingValue,
"trying to get non-existing value for contract storage key",
&[key],
);
}
err
}

pub(crate) fn decorate_contract_instance_storage_error(
&self,
err: HostError,
contract_id: &Hash,
) -> HostError {
if err.error.is_type(ScErrorType::Storage) && err.error.is_code(ScErrorCode::ExceededLimit)
{
return self.err(
ScErrorType::Storage,
ScErrorCode::ExceededLimit,
"trying to access contract instance key outside of the correct footprint",
// No need for metered clone here as we are on the unrecoverable
// error path.
&[self
.add_host_object(ScAddress::Contract(contract_id.clone()))
.map(|a| a.into())
.unwrap_or(Val::VOID.into())],
);
}
err
}

pub(crate) fn decorate_contract_code_storage_error(
&self,
err: HostError,
wasm_hash: &Hash,
) -> HostError {
if err.error.is_type(ScErrorType::Storage) && err.error.is_code(ScErrorCode::ExceededLimit)
{
return self.err(
ScErrorType::Storage,
ScErrorCode::ExceededLimit,
"trying to access contract code key outside of the correct footprint",
// No need for metered clone here as we are on the unrecoverable
// error path.
&[self
.add_host_object(self.scbytes_from_hash(wasm_hash).unwrap_or_default())
.map(|a| a.into())
.unwrap_or(Val::VOID.into())],
);
}
err
}
}

pub(crate) trait DebugArg {
Expand Down
28 changes: 0 additions & 28 deletions soroban-env-host/src/host/frame.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use std::rc::Rc;
use crate::Vm;

use super::{
invoker_type::InvokerType,
metered_clone::{MeteredClone, MeteredContainer, MeteredIterator},
prng::Prng,
};
Expand Down Expand Up @@ -426,33 +425,6 @@ impl Host {
Ok(hash)
}

// Metering: mostly free or already covered by components (e.g. err_general)
pub(super) fn get_invoker_type(&self) -> Result<u64, HostError> {
let frames = self.try_borrow_context()?;
// If the previous frame exists and is a contract, return its ID, otherwise return
// the account invoking.
let st = match frames.as_slice() {
// There are always two frames when WASM is executed in the VM.
[.., c2, _] => match &c2.frame {
Frame::ContractVM { .. } => Ok(InvokerType::Contract),
Frame::HostFunction(_) => Ok(InvokerType::Account),
Frame::Token(..) => Ok(InvokerType::Contract),
#[cfg(any(test, feature = "testutils"))]
Frame::TestContract(_) => Ok(InvokerType::Contract),
},
// In tests contracts are executed with a single frame.
// TODO: Investigate this discrepancy: https://github.com/stellar/rs-soroban-env/issues/485.
[_] => Ok(InvokerType::Account),
_ => Err(self.err(
ScErrorType::Context,
ScErrorCode::InternalError,
"no frames to derive the invoker from",
&[],
)),
}?;
Ok(st as u64)
}

/// Pushes a test contract [`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
Expand Down
28 changes: 0 additions & 28 deletions soroban-env-host/src/host/invoker_type.rs
View file

This file was deleted.

11 changes: 6 additions & 5 deletions soroban-env-host/src/host/prng.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::declared_size::DeclaredSizeForMetering;
use crate::{
budget::Budget,
host::metered_clone::MeteredClone,
Expand Down Expand Up @@ -80,7 +81,7 @@ use std::ops::RangeInclusive;
pub(crate) struct Prng(ChaCha20Rng);

pub type Seed = <rand_chacha::ChaCha20Rng as rand::SeedableRng>::Seed;
pub const SEED_BYTES: usize = core::mem::size_of::<Seed>();
pub const SEED_BYTES: u64 = <Seed as DeclaredSizeForMetering>::DECLARED_SIZE;
static_assertions::const_assert_eq!(SEED_BYTES, 32);

impl Prng {
Expand All @@ -101,7 +102,7 @@ impl Prng {
// We over-estimate the number of bytes drawn by a factor of 2, to
// account for the fact that a range sample is rejection-sampling which
// is expected to only do one draw but might do more than one.
self.charge_prng_bytes(budget, (2 * core::mem::size_of::<u64>()) as u64)?;
self.charge_prng_bytes(budget, 2 * <u64 as DeclaredSizeForMetering>::DECLARED_SIZE)?;
let u = Uniform::from(range);
Ok(u.sample(&mut self.0))
}
Expand Down Expand Up @@ -136,10 +137,10 @@ impl Prng {
}

pub(crate) fn sub_prng(&mut self, budget: &Budget) -> Result<Prng, HostError> {
let mut new_seed: Seed = [0; SEED_BYTES];
self.charge_prng_bytes(budget, SEED_BYTES as u64)?;
let mut new_seed: Seed = [0; SEED_BYTES as usize];
self.charge_prng_bytes(budget, SEED_BYTES)?;
self.0.fill_bytes(&mut new_seed);
budget.charge(ContractCostType::HostMemCpy, Some(SEED_BYTES as u64))?;
budget.charge(ContractCostType::HostMemCpy, Some(SEED_BYTES))?;
Ok(Self(ChaCha20Rng::from_seed(new_seed)))
}
}

0 comments on commit 1dccd1c

Please sign in to comment.