Skip to content

Commit

Permalink
Code review, cleanup, removal of dead/dangerous host functions (#1085)
Browse files Browse the repository at this point in the history
Most of this is fairly trivial cleanup _except_ the part where I'm
removing 2 host functions:

  - get_invoking_contract
  - get_current_call_stack

These are vestigial from the older auth scheme and are potential
footguns to leave exposed; the consensus seems to be that anything you
_should_ do with them you can do better with the new auth system, so
exposing them is just asking for trouble.
  • Loading branch information
graydon authored Sep 23, 2023
1 parent f593800 commit 511337e
Show file tree
Hide file tree
Showing 38 changed files with 127 additions and 191 deletions.
32 changes: 9 additions & 23 deletions soroban-env-common/env.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@
},
{
"export": "0",
"name": "get_invoking_contract",
"args": [],
"return": "AddressObject",
"docs": "Get the address object of the contract which invoked the running contract. Traps if the running contract was not invoked by a contract."
},
{
"export": "1",
"name": "obj_cmp",
"args": [
{
Expand All @@ -52,7 +45,7 @@
"docs": "Compare two objects, or at least one object to a non-object, structurally. Returns -1 if a<b, 1 if a>b, or 0 if a==b."
},
{
"export": "2",
"export": "1",
"name": "contract_event",
"args": [
{
Expand All @@ -68,35 +61,28 @@
"docs": "Records a contract event. `topics` is expected to be a `SCVec` with length <= 4 that cannot contain `Vec`, `Map`, or `Bytes` with length > 32."
},
{
"export": "3",
"export": "2",
"name": "get_ledger_version",
"args": [],
"return": "U32Val",
"docs": "Return the protocol version of the current ledger as a u32."
},
{
"export": "4",
"export": "3",
"name": "get_ledger_sequence",
"args": [],
"return": "U32Val",
"docs": "Return the sequence number of the current ledger as a u32."
},
{
"export": "5",
"export": "4",
"name": "get_ledger_timestamp",
"args": [],
"return": "U64Val",
"docs": "Return the timestamp number of the current ledger as a u64."
},
{
"export": "6",
"name": "get_current_call_stack",
"args": [],
"return": "VecObject",
"docs": "Returns the full call stack from the first contract call to the current one as a vector of vectors, where the inside vector contains the contract id as Hash, and a function as a Symbol."
},
{
"export": "7",
"export": "5",
"name": "fail_with_error",
"args": [
{
Expand All @@ -108,21 +94,21 @@
"docs": "Causes the currently executing contract to fail immediately with a provided error code, which must be of error-type `ScErrorType::Contract`. Does not actually return."
},
{
"export": "8",
"export": "6",
"name": "get_ledger_network_id",
"args": [],
"return": "BytesObject",
"docs": "Return the network id (sha256 hash of network passphrase) of the current ledger as `Bytes`. The value is always 32 bytes in length."
},
{
"export": "9",
"export": "7",
"name": "get_current_contract_address",
"args": [],
"return": "AddressObject",
"docs": "Get the Address object for the current contract."
},
{
"export": "a",
"export": "8",
"name": "get_max_expiration_ledger",
"args": [],
"return": "U32Val",
Expand Down Expand Up @@ -1178,7 +1164,7 @@
}
],
"return": "u64",
"docs": "Binary search a sorted vector for a given element. If it exists, the high-32 bits of the return value is 0x0001 and the low-32 bits contain the u32 index of the element. If it does not exist, the high-32 bits of the return value is 0x0000 and the low-32 bits contain the u32 index at which the element would need to be inserted into the vector to maintain sorted order."
"docs": "Binary search a sorted vector for a given element. If it exists, the high 32 bits of the return value is 0x0000_0001 and the low 32 bits contain the u32 index of the element. If it does not exist, the high 32 bits of the return value is 0x0000_0000 and the low-32 bits contain the u32 index at which the element would need to be inserted into the vector to maintain sorted order."
},
{
"export": "g",
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-common/src/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ soroban_env_macros::generate_env_meta_consts!(
#[cfg(not(feature = "next"))]
soroban_env_macros::generate_env_meta_consts!(
ledger_protocol_version: 20,
pre_release_version: 61,
pre_release_version: 62,
);

pub const fn get_ledger_protocol_version(interface_version: u64) -> u32 {
Expand Down
28 changes: 16 additions & 12 deletions soroban-env-common/src/vmcaller_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,17 @@ impl<'a, T> VmCaller<'a, T> {
match &self.0 {
Some(caller) => Ok(caller),
None => Err(Error::from_type_and_code(
ScErrorType::WasmVm,
ScErrorCode::MissingValue,
ScErrorType::Context,
ScErrorCode::InternalError,
)),
}
}
pub fn try_mut(&mut self) -> Result<&mut wasmi::Caller<'a, T>, Error> {
match &mut self.0 {
Some(caller) => Ok(caller),
None => Err(Error::from_type_and_code(
ScErrorType::WasmVm,
ScErrorCode::MissingValue,
ScErrorType::Context,
ScErrorCode::InternalError,
)),
}
}
Expand Down Expand Up @@ -122,15 +122,15 @@ macro_rules! generate_vmcaller_checked_env_trait {

/// This trait is a variant of the [Env](crate::Env) trait used to
/// define the interface implemented by Host. The wasmi VM dispatch
/// functions call methods on `VmCallerEnv`, passing a
/// [`VmCaller`] that wraps the wasmi Caller context, and then convert
/// any `Result::Err(...)` return value into a VM trap, halting VM
/// execution.
/// functions (in soroban_env_host::dispatch) call methods on
/// `VmCallerEnv`, passing a [`VmCaller`] that wraps the wasmi Caller
/// context, and then convert any `Result::Err(...)` return value into a
/// VM trap, halting VM execution.
///
/// There is also a blanket `impl<T:VmCallerEnv> Env for
/// T` that implements the `Env` for any `VmCallerEnv` by
/// passing [`VmCaller::none()`] for the first argument, allowing user
/// code such as the native contract to avoid writing `VmCaller::none()`
/// There is also a blanket `impl<T:VmCallerEnv> Env for T` that
/// implements the `Env` for any `VmCallerEnv` by passing
/// [`VmCaller::none()`] for the first argument, allowing user code such
/// as the native contract to avoid writing `VmCaller::none()`
/// everywhere.
pub trait VmCallerEnv: EnvBase
{
Expand Down Expand Up @@ -172,6 +172,10 @@ macro_rules! vmcaller_none_function_helper {
{fn $fn_id:ident($($arg:ident:$type:ty),*) -> $ret:ty}
=>
{
// We call `augment_err_result` here to give the Env a chance to attach
// context (eg. a backtrace) to any error that was generated by code
// that didn't have an Env on hand when creating the error. This will at
// least localize the error to a given Env call.
fn $fn_id(&self, $($arg:$type),*) -> Result<$ret, Self::Error> {
self.augment_err_result(<Self as VmCallerEnv>::$fn_id(self, &mut VmCaller::none(), $($arg),*))
}
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-guest/src/guest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use static_assertions as sa;

// Just the smallest possible version of a runtime assertion-or-panic.
#[inline(always)]
pub(crate) fn require(b: bool) {
fn require(b: bool) {
if !b {
core::arch::wasm32::unreachable()
}
Expand Down
Loading

0 comments on commit 511337e

Please sign in to comment.