From dc2542f4fd0e37bcc0c6abf9ce043cf87d0d1498 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Tue, 14 Mar 2023 18:20:38 -0400 Subject: [PATCH 01/19] chore: clippy --fix --- soroban-env-common/src/compare.rs | 4 ++-- soroban-env-common/src/env.rs | 6 ++--- soroban-env-common/src/env_val.rs | 12 +++++----- soroban-env-common/src/lib.rs | 10 ++++---- soroban-env-common/src/meta.rs | 2 +- soroban-env-common/src/num.rs | 20 ++++++++-------- soroban-env-common/src/object.rs | 16 ++++++------- soroban-env-common/src/raw_val.rs | 18 +++++++------- soroban-env-common/src/status.rs | 18 +++++++------- soroban-env-common/src/string.rs | 8 +++---- soroban-env-common/src/symbol.rs | 12 ++++------ soroban-env-common/src/vmcaller_env.rs | 4 ++-- soroban-env-guest/src/lib.rs | 2 +- .../src/call_macro_with_all_host_functions.rs | 4 ++-- soroban-test-wasms/src/lib.rs | 24 +++++++++---------- 15 files changed, 79 insertions(+), 81 deletions(-) diff --git a/soroban-env-common/src/compare.rs b/soroban-env-common/src/compare.rs index fc37aa9df..8e2921142 100644 --- a/soroban-env-common/src/compare.rs +++ b/soroban-env-common/src/compare.rs @@ -97,7 +97,7 @@ impl> Compare> for C { type Error = C::Error; fn compare(&self, a: &Box, b: &Box) -> Result { - >::compare(self, &*a, &*b) + >::compare(self, a, b) } } @@ -106,7 +106,7 @@ impl> Compare> for C { type Error = C::Error; fn compare(&self, a: &Rc, b: &Rc) -> Result { - >::compare(self, &*a, &*b) + >::compare(self, a, b) } } diff --git a/soroban-env-common/src/env.rs b/soroban-env-common/src/env.rs index 5a228e2d7..43498c264 100644 --- a/soroban-env-common/src/env.rs +++ b/soroban-env-common/src/env.rs @@ -160,7 +160,7 @@ pub trait EnvBase: Sized + Clone { /// Log a formatted debugging message to the debug log (if present), passing /// a simplified format string (supporting only positional `{}` markers) and - /// a single [RawVal] argument that will be inserted at the marker in the + /// a single [`RawVal`] argument that will be inserted at the marker in the /// format string. fn log_static_fmt_val(&self, fmt: &'static str, v: RawVal) -> Result<(), Self::Error>; @@ -176,7 +176,7 @@ pub trait EnvBase: Sized + Clone { /// Log a formatted debugging message to the debug log (if present), passing /// a simplified format string (supporting only positional `{}` markers) and - /// both a [RawVal] and a string-slice argument, that will each be inserted + /// both a [`RawVal`] and a string-slice argument, that will each be inserted /// at markers in the format string. fn log_static_fmt_val_static_str( &self, @@ -187,7 +187,7 @@ pub trait EnvBase: Sized + Clone { /// Log a formatted debugging message to the debug log (if present), passing /// a simplified format string (supporting only positional `{}` markers) and - /// both a slice of [RawVal]s and a slice of string-slice argument, that + /// both a slice of [`RawVal`]s and a slice of string-slice argument, that /// will be sequentially inserted at markers in the format string. fn log_static_fmt_general( &self, diff --git a/soroban-env-common/src/env_val.rs b/soroban-env-common/src/env_val.rs index 569f33e4a..f1aa87c76 100644 --- a/soroban-env-common/src/env_val.rs +++ b/soroban-env-common/src/env_val.rs @@ -143,7 +143,7 @@ impl TryFromVal for i128 { let obj = v.try_into()?; let lo = env.obj_to_i128_lo64(obj).map_err(|_| ConversionError)?; let hi = env.obj_to_i128_hi64(obj).map_err(|_| ConversionError)?; - let u: u128 = (lo as u128) | ((hi as u128) << 64); + let u: u128 = u128::from(lo) | (u128::from(hi) << 64); Ok(u as i128) } } @@ -177,7 +177,7 @@ impl TryFromVal for u128 { let obj = v.try_into()?; let lo = env.obj_to_u128_lo64(obj).map_err(|_| ConversionError)?; let hi = env.obj_to_u128_hi64(obj).map_err(|_| ConversionError)?; - let u: u128 = (lo as u128) | ((hi as u128) << 64); + let u: u128 = u128::from(lo) | (u128::from(hi) << 64); Ok(u) } } @@ -241,11 +241,11 @@ where lo: val.get_signed_body() as u64, })), Tag::U256Small => { - let val = U256::new(val.get_body() as u128); + let val = U256::new(u128::from(val.get_body())); Ok(ScVal::U256(Uint256(val.to_be_bytes()))) } Tag::I256Small => { - let val = I256::new(val.get_signed_body() as i128); + let val = I256::new(i128::from(val.get_signed_body())); Ok(ScVal::I256(Uint256(val.to_be_bytes()))) } Tag::SymbolSmall => { @@ -327,12 +327,12 @@ where unsafe { RawVal::from_body_and_tag((i as i64) as u64, Tag::I128Small) } } ScVal::U256(u) => { - let u: U256 = U256::from_be_bytes(u.0.clone()); + let u: U256 = U256::from_be_bytes(u.0); assert!(num::is_small_u256(&u)); unsafe { RawVal::from_body_and_tag(u.as_u64(), Tag::U256Small) } } ScVal::I256(i) => { - let i: I256 = I256::from_be_bytes(i.0.clone()); + let i: I256 = I256::from_be_bytes(i.0); assert!(num::is_small_i256(&i)); unsafe { RawVal::from_body_and_tag(i.as_i64() as u64, Tag::I256Small) } } diff --git a/soroban-env-common/src/lib.rs b/soroban-env-common/src/lib.rs index 373f8f1fc..af33941e7 100644 --- a/soroban-env-common/src/lib.rs +++ b/soroban-env-common/src/lib.rs @@ -1,22 +1,22 @@ #![cfg_attr(not(feature = "std"), no_std)] //! The environment-common crate contains three families of types: //! -//! - The [RawVal] type, a 64-bit value type that is a union between several +//! - The [`RawVal`] type, a 64-bit value type that is a union between several //! different types (numbers, booleans, symbols, object references), encoded //! via careful bit-packing. //! - Wrapper types ([Object], [Symbol], [Status]) that -//! contain [RawVal] in a specific, known union state. These are also 64-bit +//! contain [`RawVal`] in a specific, known union state. These are also 64-bit //! values, but offer methods specific to the union state (eg. [Symbol] will //! interconvert with Rust strings). //! - The [Env] trait, which describes the _interface_ between guest and host //! code. In other words, `Env` describes a set of _host functions_ that //! must be implemented in a contract host, and can be called from a guest //! (or by the SDK). Methods on the [Env] trait can only pass 64-bit values, -//! which are usually [RawVal] or one of the wrapper types. +//! which are usually [`RawVal`] or one of the wrapper types. //! //! The crate additionally contains functions for interconversion between the -//! [RawVal] type and XDR types, and re-exports the XDR definitions from -//! [stellar_xdr] under the module [xdr]. +//! [`RawVal`] type and XDR types, and re-exports the XDR definitions from +//! [`stellar_xdr`] under the module [xdr]. #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] diff --git a/soroban-env-common/src/meta.rs b/soroban-env-common/src/meta.rs index 1a03fac6b..d221e61dd 100644 --- a/soroban-env-common/src/meta.rs +++ b/soroban-env-common/src/meta.rs @@ -55,7 +55,7 @@ // a contract, since the same contract will be expected to run against multliple // implementations over a long period of time. -pub const ENV_META_V0_SECTION_NAME: &'static str = "contractenvmetav0"; +pub const ENV_META_V0_SECTION_NAME: &str = "contractenvmetav0"; soroban_env_macros::generate_env_meta_consts!( interface_version: 32, diff --git a/soroban-env-common/src/num.rs b/soroban-env-common/src/num.rs index d9fbf67d6..1c9270fa4 100644 --- a/soroban-env-common/src/num.rs +++ b/soroban-env-common/src/num.rs @@ -94,7 +94,7 @@ impl From for u64 { impl From for i64 { fn from(value: I64Small) -> Self { - value.0.get_signed_body() as i64 + value.0.get_signed_body() } } @@ -112,13 +112,13 @@ impl From for u64 { impl From for u128 { fn from(value: U128Small) -> Self { - value.0.get_body() as u128 + u128::from(value.0.get_body()) } } impl From for i128 { fn from(value: I128Small) -> Self { - (value.0.get_signed_body() as i64) as i128 + i128::from(value.0.get_signed_body()) } } @@ -130,7 +130,7 @@ impl From for U256 { impl From for I256 { fn from(value: I256Small) -> Self { - I256::from(value.0.get_signed_body() as i64) + I256::from(value.0.get_signed_body()) } } @@ -215,7 +215,7 @@ impl TryFrom for I256Small { impl TryFrom for ScVal { type Error = ConversionError; fn try_from(value: U256Small) -> Result { - let val = U256::new(value.as_raw().get_body() as u128); + let val = U256::new(u128::from(value.as_raw().get_body())); Ok(ScVal::U256(Uint256(val.to_be_bytes()))) } } @@ -223,14 +223,14 @@ impl TryFrom for ScVal { impl TryFrom<&U256Small> for ScVal { type Error = ConversionError; fn try_from(value: &U256Small) -> Result { - value.clone().try_into() + (*value).try_into() } } impl TryFrom for ScVal { type Error = ConversionError; fn try_from(value: I256Small) -> Result { - let val = I256::new(value.as_raw().get_signed_body() as i128); + let val = I256::new(i128::from(value.as_raw().get_signed_body())); Ok(ScVal::I256(Uint256(val.to_be_bytes()))) } } @@ -238,7 +238,7 @@ impl TryFrom for ScVal { impl TryFrom<&I256Small> for ScVal { type Error = ConversionError; fn try_from(value: &I256Small) -> Result { - value.clone().try_into() + (*value).try_into() } } @@ -252,12 +252,12 @@ pub const fn is_small_i64(i: i64) -> bool { pub fn is_small_u128(u: u128) -> bool { let word = u as u64; - is_small_u64(word) && u == (word as u128) + is_small_u64(word) && u == u128::from(word) } pub fn is_small_i128(i: i128) -> bool { let word = i as i64; - is_small_i64(word) && i == (word as i128) + is_small_i64(word) && i == i128::from(word) } pub fn is_small_u256(u: &U256) -> bool { diff --git a/soroban-env-common/src/object.rs b/soroban-env-common/src/object.rs index 237fea70e..246a516c2 100644 --- a/soroban-env-common/src/object.rs +++ b/soroban-env-common/src/object.rs @@ -6,8 +6,8 @@ use core::{cmp::Ordering, fmt::Debug}; use ethnum::{I256, U256}; use stellar_xdr::{Duration, ScVal, TimePoint}; -/// Wrapper for a [RawVal] that is tagged with one of the object types, -/// interpreting the [RawVal]'s body as containing a 32-bit object-code handle +/// Wrapper for a [`RawVal`] that is tagged with one of the object types, +/// interpreting the [`RawVal`]'s body as containing a 32-bit object-code handle /// to a host object of the object-type. #[repr(transparent)] #[derive(Copy, Clone)] @@ -88,7 +88,7 @@ impl ScValObject { /// Inspect the provided `value` and return `Ok(ScValObject(value))` if it /// is a value that should be represented as an object, else `Err(value)`. pub fn classify(value: ScVal) -> Result { - if let Some(_) = ScValObjRef::classify(&value) { + if ScValObjRef::classify(&value).is_some() { Ok(ScValObject(value)) } else { Err(value) @@ -111,9 +111,9 @@ impl AsRef for ScValObject { #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct ScValObjRef<'a>(&'a ScVal); -impl<'a> Into<&'a ScVal> for ScValObjRef<'a> { - fn into(self) -> &'a ScVal { - self.0 +impl<'a> From> for &'a ScVal { + fn from(val: ScValObjRef<'a>) -> Self { + val.0 } } @@ -176,7 +176,7 @@ impl<'a> ScValObjRef<'a> { } } ScVal::U256(bytes) => { - let u = U256::from_be_bytes(bytes.0.clone()); + let u = U256::from_be_bytes(bytes.0); if num::is_small_u256(&u) { None } else { @@ -184,7 +184,7 @@ impl<'a> ScValObjRef<'a> { } } ScVal::I256(bytes) => { - let i = I256::from_be_bytes(bytes.0.clone()); + let i = I256::from_be_bytes(bytes.0); if num::is_small_i256(&i) { None } else { diff --git a/soroban-env-common/src/raw_val.rs b/soroban-env-common/src/raw_val.rs index 62b1e2e3a..6a942e835 100644 --- a/soroban-env-common/src/raw_val.rs +++ b/soroban-env-common/src/raw_val.rs @@ -37,7 +37,7 @@ sa::const_assert!(MINOR_MASK == 0x00ff_ffff); sa::const_assert!(MAJOR_BITS + MINOR_BITS == BODY_BITS); /// Code values for the 8 `tag` bits in the bit-packed representation -/// of [RawVal]. These don't coincide with tag numbers in the SCVal XDR +/// of [`RawVal`]. These don't coincide with tag numbers in the `SCVal` XDR /// but cover all those cases as well as some optimized refinements for /// special cases (boolean true and false, small-value forms). #[repr(u8)] @@ -274,8 +274,8 @@ impl From for ConversionError { } } -/// Trait abstracting over types that can be converted into [RawVal], similar to -/// [TryFrom] but with a different signature that enables generating slightly +/// Trait abstracting over types that can be converted into [`RawVal`], similar to +/// [`TryFrom`] but with a different signature that enables generating slightly /// more efficient conversion code. An implementation of `TryFrom` is also /// provided for any type that implements `ValConvertible`. pub trait RawValConvertible: Into + TryFrom { @@ -370,7 +370,7 @@ impl RawValConvertible for u32 { } #[inline(always)] unsafe fn unchecked_from_val(v: RawVal) -> Self { - v.get_major() as u32 + v.get_major() } } @@ -623,8 +623,8 @@ impl Debug for RawVal { } match self.get_tag() { - Tag::U32Val => write!(f, "U32({})", self.get_major() as u32), - Tag::I32Val => write!(f, "I32({})", (self.get_major() as u32) as i32), + Tag::U32Val => write!(f, "U32({})", { self.get_major() }), + Tag::I32Val => write!(f, "I32({})", self.get_major() as i32), Tag::False => write!(f, "False"), Tag::True => write!(f, "True"), Tag::Void => write!(f, "Void"), @@ -637,15 +637,15 @@ impl Debug for RawVal { Tag::DurationSmall => write!(f, "Duration({})", self.get_body()), // These can't be bigger than u64/i64 so just cast to them. Tag::U128Small => write!(f, "U128({})", self.get_body()), - Tag::I128Small => write!(f, "I128({})", self.get_signed_body() as i64), + Tag::I128Small => write!(f, "I128({})", { self.get_signed_body() }), // These can't be bigger than u64/i64 so just cast to them. Tag::U256Small => write!(f, "U256({})", self.get_body()), - Tag::I256Small => write!(f, "I256({})", self.get_signed_body() as i64), + Tag::I256Small => write!(f, "I256({})", { self.get_signed_body() }), Tag::SymbolSmall => { let ss: SymbolStr = unsafe { ::unchecked_from_val(*self) }.into(); let s: &str = ss.as_ref(); - write!(f, "Symbol({})", s) + write!(f, "Symbol({s})") } Tag::LedgerKeyContractExecutable => write!(f, "LedgerKeyContractCode"), diff --git a/soroban-env-common/src/status.rs b/soroban-env-common/src/status.rs index ec99efdd0..2b08d4315 100644 --- a/soroban-env-common/src/status.rs +++ b/soroban-env-common/src/status.rs @@ -15,11 +15,11 @@ use stellar_xdr::{ ScVmErrorCode, }; -/// Wrapper for a [RawVal] that is tagged with [Tag::Status], interpreting the -/// [RawVal]'s body as a pair of a 28-bit status-type code and a 32-bit status +/// Wrapper for a [`RawVal`] that is tagged with [`Tag::Status`], interpreting the +/// [`RawVal`]'s body as a pair of a 28-bit status-type code and a 32-bit status /// code. The status-type codes correspond to the enumerated cases of -/// [ScStatusType], and the status codes correspond to the code values stored in -/// each variant of the [ScStatus] union. +/// [`ScStatusType`], and the status codes correspond to the code values stored in +/// each variant of the [`ScStatus`] union. #[repr(transparent)] #[derive(Copy, Clone)] pub struct Status(RawVal); @@ -70,7 +70,7 @@ impl Ord for Status { impl Compare for E { type Error = E::Error; fn compare(&self, a: &Status, b: &Status) -> Result { - Ok(a.cmp(&b)) + Ok(a.cmp(b)) } } @@ -146,8 +146,8 @@ impl Debug for Status { }; write!(f, "Status({}(", st.name())?; match st { - ScStatusType::Ok => write!(f, "{}", code), - ScStatusType::UnknownError => write!(f, "{}", code), + ScStatusType::Ok => write!(f, "{code}"), + ScStatusType::UnknownError => write!(f, "{code}"), ScStatusType::HostValueError => fmt_named_code::(code, f), ScStatusType::HostObjectError => fmt_named_code::(code, f), ScStatusType::HostFunctionError => fmt_named_code::(code, f), @@ -155,7 +155,7 @@ impl Debug for Status { ScStatusType::HostContextError => fmt_named_code::(code, f), ScStatusType::HostAuthError => fmt_named_code::(code, f), ScStatusType::VmError => fmt_named_code::(code, f), - ScStatusType::ContractError => write!(f, "{}", code), + ScStatusType::ContractError => write!(f, "{code}"), }?; write!(f, "))") } @@ -316,7 +316,7 @@ impl Status { ScStatus::HostAuthError(code) => code as i32 as u32, ScStatus::VmError(code) => code as i32 as u32, ScStatus::UnknownError(code) => code as i32 as u32, - ScStatus::ContractError(code) => code as u32, + ScStatus::ContractError(code) => code, }; Self::from_type_and_code(sc.discriminant(), code) } diff --git a/soroban-env-common/src/string.rs b/soroban-env-common/src/string.rs index e09ed9755..8f5473188 100644 --- a/soroban-env-common/src/string.rs +++ b/soroban-env-common/src/string.rs @@ -12,7 +12,7 @@ impl TryFromVal for String { fn try_from_val(env: &E, v: &StringObject) -> Result { let len: u32 = env.string_len(*v).map_err(|_| ConversionError)?.into(); let len = len as usize; - let mut vec = std::vec![0; len as usize]; + let mut vec = std::vec![0; len]; env.string_copy_to_slice(*v, RawVal::U32_ZERO, &mut vec) .map_err(|_| ConversionError)?; String::from_utf8(vec).map_err(|_| ConversionError) @@ -34,9 +34,9 @@ impl TryFromVal for StringObject { type Error = ConversionError; #[inline(always)] fn try_from_val(env: &E, val: &&str) -> Result { - Ok(env - .string_new_from_slice(*val) - .map_err(|_| ConversionError)?) + env + .string_new_from_slice(val) + .map_err(|_| ConversionError) } } diff --git a/soroban-env-common/src/symbol.rs b/soroban-env-common/src/symbol.rs index 6879e68ae..1a5ff5b1f 100644 --- a/soroban-env-common/src/symbol.rs +++ b/soroban-env-common/src/symbol.rs @@ -6,7 +6,7 @@ use core::{cmp::Ordering, fmt::Debug, hash::Hash, str}; declare_tag_based_small_and_object_wrappers!(Symbol, SymbolSmall, SymbolObject); -/// Errors related to operations on the [SymbolObject] and [SymbolSmall] types. +/// Errors related to operations on the [`SymbolObject`] and [`SymbolSmall`] types. #[derive(Debug)] pub enum SymbolError { /// Returned when attempting to form a [SymbolSmall] from a string with more @@ -21,12 +21,10 @@ impl core::fmt::Display for SymbolError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { SymbolError::TooLong(len) => f.write_fmt(format_args!( - "symbol too long: length {}, max {}", - len, MAX_SMALL_CHARS + "symbol too long: length {len}, max {MAX_SMALL_CHARS}" )), SymbolError::BadChar(char) => f.write_fmt(format_args!( - "symbol bad char: encountered {}, supported range [a-zA-Z0-9_]", - char + "symbol bad char: encountered {char}, supported range [a-zA-Z0-9_]" )), } } @@ -69,7 +67,7 @@ impl TryFromVal for Symbol { fn try_from_val(env: &E, v: &&[u8]) -> Result { // We don't know this byte-slice is actually utf-8 ... - let s: &str = unsafe { core::str::from_utf8_unchecked(*v) }; + let s: &str = unsafe { core::str::from_utf8_unchecked(v) }; // ... but this next conversion step will check that its // _bytes_ are in the symbol-char range, which is a subset // of utf-8, so we're only lying harmlessly. @@ -439,7 +437,7 @@ impl TryFrom for ScVal { impl TryFrom<&SymbolSmall> for ScVal { type Error = ConversionError; fn try_from(s: &SymbolSmall) -> Result { - s.clone().try_into() + (*s).try_into() } } diff --git a/soroban-env-common/src/vmcaller_env.rs b/soroban-env-common/src/vmcaller_env.rs index 7c7389c39..c85164566 100644 --- a/soroban-env-common/src/vmcaller_env.rs +++ b/soroban-env-common/src/vmcaller_env.rs @@ -59,7 +59,7 @@ impl<'a, T> VmCaller<'a, T> { } /////////////////////////////////////////////////////////////////////////////// -/// X-macro use: defining trait VmCallerEnv +/// X-macro use: defining trait `VmCallerEnv` /////////////////////////////////////////////////////////////////////////////// // This is a helper macro used only by generate_vmcaller_checked_env_trait @@ -150,7 +150,7 @@ macro_rules! generate_vmcaller_checked_env_trait { call_macro_with_all_host_functions! { generate_vmcaller_checked_env_trait } /////////////////////////////////////////////////////////////////////////////// -/// X-macro use: impl Env for VmCallerEnv +/// X-macro use: impl Env for `VmCallerEnv` /////////////////////////////////////////////////////////////////////////////// // This is a helper macro used only by diff --git a/soroban-env-guest/src/lib.rs b/soroban-env-guest/src/lib.rs index e28383642..c7dee0d3b 100644 --- a/soroban-env-guest/src/lib.rs +++ b/soroban-env-guest/src/lib.rs @@ -3,7 +3,7 @@ //! This crate provides the [Guest] type, the "stub" implementation of the [Env] //! interface used for communicating between a contract guest and its host. //! -//! It also re-exports all of the content of the [soroban_env_common] crate for +//! It also re-exports all of the content of the [`soroban_env_common`] crate for //! use by guest code. Most of the type and module definitions visible here are //! actually defined in the common crate. diff --git a/soroban-env-macros/src/call_macro_with_all_host_functions.rs b/soroban-env-macros/src/call_macro_with_all_host_functions.rs index 8f93f319d..66431a4ee 100644 --- a/soroban-env-macros/src/call_macro_with_all_host_functions.rs +++ b/soroban-env-macros/src/call_macro_with_all_host_functions.rs @@ -14,14 +14,14 @@ pub fn generate(file_lit: LitStr) -> Result { let file = File::open(&file_path).map_err(|e| { Error::new( file_lit.span(), - format!("error reading file '{}': {}", file_str, e), + format!("error reading file '{file_str}': {e}"), ) })?; let root: Root = serde_json::from_reader(file).map_err(|e| { Error::new( file_lit.span(), - format!("error parsing file '{}': {}", file_str, e), + format!("error parsing file '{file_str}': {e}"), ) })?; diff --git a/soroban-test-wasms/src/lib.rs b/soroban-test-wasms/src/lib.rs index 200b08f1c..61b583d44 100644 --- a/soroban-test-wasms/src/lib.rs +++ b/soroban-test-wasms/src/lib.rs @@ -42,25 +42,25 @@ //! documentation purpopses without having to worry about breaking tests in //! the host here. -pub const ADD_I32: &'static [u8] = +pub const ADD_I32: &[u8] = include_bytes!("../wasm-workspace/opt/example_add_i32.wasm").as_slice(); -pub const CREATE_CONTRACT: &'static [u8] = +pub const CREATE_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_create_contract.wasm").as_slice(); -pub const CONTRACT_DATA: &'static [u8] = +pub const CONTRACT_DATA: &[u8] = include_bytes!("../wasm-workspace/opt/example_contract_data.wasm").as_slice(); -pub const LINEAR_MEMORY: &'static [u8] = +pub const LINEAR_MEMORY: &[u8] = include_bytes!("../wasm-workspace/opt/example_linear_memory.wasm").as_slice(); -pub const VEC: &'static [u8] = include_bytes!("../wasm-workspace/opt/example_vec.wasm").as_slice(); -pub const INVOKE_CONTRACT: &'static [u8] = +pub const VEC: &[u8] = include_bytes!("../wasm-workspace/opt/example_vec.wasm").as_slice(); +pub const INVOKE_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_invoke_contract.wasm").as_slice(); -pub const HOSTILE: &'static [u8] = +pub const HOSTILE: &[u8] = include_bytes!("../wasm-workspace/opt/example_hostile.wasm").as_slice(); -pub const FIB: &'static [u8] = include_bytes!("../wasm-workspace/opt/example_fib.wasm").as_slice(); -pub const FANNKUCH: &'static [u8] = +pub const FIB: &[u8] = include_bytes!("../wasm-workspace/opt/example_fib.wasm").as_slice(); +pub const FANNKUCH: &[u8] = include_bytes!("../wasm-workspace/opt/example_fannkuch.wasm").as_slice(); -pub const COMPLEX: &'static [u8] = +pub const COMPLEX: &[u8] = include_bytes!("../wasm-workspace/opt/example_complex.wasm").as_slice(); -pub const SIMPLE_ACCOUNT_CONTRACT: &'static [u8] = +pub const SIMPLE_ACCOUNT_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_simple_account.wasm").as_slice(); -pub const AUTH_TEST_CONTRACT: &'static [u8] = +pub const AUTH_TEST_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/auth_test_contract.wasm").as_slice(); From 7dd8ac3594f6193e2420ecffc25738881eb6a303 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 09:09:39 -0400 Subject: [PATCH 02/19] chore: cargo fmt --- soroban-env-common/src/string.rs | 4 +--- soroban-test-wasms/src/lib.rs | 9 +++------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/soroban-env-common/src/string.rs b/soroban-env-common/src/string.rs index 8f5473188..82c8ae99c 100644 --- a/soroban-env-common/src/string.rs +++ b/soroban-env-common/src/string.rs @@ -34,9 +34,7 @@ impl TryFromVal for StringObject { type Error = ConversionError; #[inline(always)] fn try_from_val(env: &E, val: &&str) -> Result { - env - .string_new_from_slice(val) - .map_err(|_| ConversionError) + env.string_new_from_slice(val).map_err(|_| ConversionError) } } diff --git a/soroban-test-wasms/src/lib.rs b/soroban-test-wasms/src/lib.rs index 61b583d44..3334e869f 100644 --- a/soroban-test-wasms/src/lib.rs +++ b/soroban-test-wasms/src/lib.rs @@ -42,8 +42,7 @@ //! documentation purpopses without having to worry about breaking tests in //! the host here. -pub const ADD_I32: &[u8] = - include_bytes!("../wasm-workspace/opt/example_add_i32.wasm").as_slice(); +pub const ADD_I32: &[u8] = include_bytes!("../wasm-workspace/opt/example_add_i32.wasm").as_slice(); pub const CREATE_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_create_contract.wasm").as_slice(); pub const CONTRACT_DATA: &[u8] = @@ -53,13 +52,11 @@ pub const LINEAR_MEMORY: &[u8] = pub const VEC: &[u8] = include_bytes!("../wasm-workspace/opt/example_vec.wasm").as_slice(); pub const INVOKE_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_invoke_contract.wasm").as_slice(); -pub const HOSTILE: &[u8] = - include_bytes!("../wasm-workspace/opt/example_hostile.wasm").as_slice(); +pub const HOSTILE: &[u8] = include_bytes!("../wasm-workspace/opt/example_hostile.wasm").as_slice(); pub const FIB: &[u8] = include_bytes!("../wasm-workspace/opt/example_fib.wasm").as_slice(); pub const FANNKUCH: &[u8] = include_bytes!("../wasm-workspace/opt/example_fannkuch.wasm").as_slice(); -pub const COMPLEX: &[u8] = - include_bytes!("../wasm-workspace/opt/example_complex.wasm").as_slice(); +pub const COMPLEX: &[u8] = include_bytes!("../wasm-workspace/opt/example_complex.wasm").as_slice(); pub const SIMPLE_ACCOUNT_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_simple_account.wasm").as_slice(); pub const AUTH_TEST_CONTRACT: &[u8] = From d1dcc5fe150c5e91f394b462a9cd9f513ed20929 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 10:19:00 -0400 Subject: [PATCH 03/19] chore: more clippy --- soroban-env-common/src/compare.rs | 117 +++++++++---------- soroban-env-common/src/env.rs | 4 +- soroban-env-common/src/env_val.rs | 5 +- soroban-env-common/src/lib.rs | 1 + soroban-env-common/src/raw_val.rs | 3 +- soroban-env-common/src/status.rs | 11 +- soroban-env-common/src/string.rs | 4 +- soroban-env-common/src/symbol.rs | 4 +- soroban-env-common/src/unimplemented_env.rs | 2 + soroban-env-host/src/storage.rs | 3 +- soroban-native-sdk-macros/src/derive_fn.rs | 16 ++- soroban-native-sdk-macros/src/derive_type.rs | 8 +- soroban-synth-wasm/src/func_emitter.rs | 4 +- 13 files changed, 84 insertions(+), 98 deletions(-) diff --git a/soroban-env-common/src/compare.rs b/soroban-env-common/src/compare.rs index 8e2921142..ab568d224 100644 --- a/soroban-env-common/src/compare.rs +++ b/soroban-env-common/src/compare.rs @@ -135,74 +135,65 @@ impl Compare for E { if a.is_object() || b.is_object() { // Delegate any object-comparing to environment. let v = self.obj_cmp(*a, *b)?; - return if v == 0 { - Ok(Ordering::Equal) - } else if v < 0 { - Ok(Ordering::Less) - } else { - Ok(Ordering::Greater) - }; + return Ok(v.cmp(&0)); } let a_tag = a.get_tag(); let b_tag = b.get_tag(); - if a_tag < b_tag { - Ok(Ordering::Less) - } else if a_tag > b_tag { - Ok(Ordering::Greater) - } else { + match a_tag.cmp(&b_tag) { + Ordering::Equal => // Tags are equal so we only have to switch on one. - match a_tag { - Tag::False => Ok(Ordering::Equal), - Tag::True => Ok(Ordering::Equal), - Tag::Void => Ok(Ordering::Equal), - - Tag::Status => delegate_compare_to_wrapper!(Status, a, b, self), - - Tag::U32Val => delegate_compare_to_wrapper!(U32Val, a, b, self), - Tag::I32Val => delegate_compare_to_wrapper!(I32Val, a, b, self), - - Tag::U64Small => delegate_compare_to_wrapper!(U64Small, a, b, self), - Tag::I64Small => delegate_compare_to_wrapper!(I64Small, a, b, self), - - Tag::TimepointSmall => delegate_compare_to_wrapper!(TimepointSmall, a, b, self), - Tag::DurationSmall => delegate_compare_to_wrapper!(DurationSmall, a, b, self), - - Tag::U128Small => delegate_compare_to_wrapper!(U128Small, a, b, self), - Tag::I128Small => delegate_compare_to_wrapper!(I128Small, a, b, self), - - Tag::U256Small => delegate_compare_to_wrapper!(U256Small, a, b, self), - Tag::I256Small => delegate_compare_to_wrapper!(I256Small, a, b, self), - - Tag::SymbolSmall => delegate_compare_to_wrapper!(SymbolSmall, a, b, self), - - Tag::LedgerKeyContractExecutable => Ok(Ordering::Equal), - - Tag::SmallCodeUpperBound => Ok(Ordering::Equal), - Tag::ObjectCodeLowerBound => Ok(Ordering::Equal), - - // None of the object cases should be reachable, they - // should all have been handled by the is_object() branch - // above. - Tag::U64Object - | Tag::I64Object - | Tag::TimepointObject - | Tag::DurationObject - | Tag::U128Object - | Tag::I128Object - | Tag::U256Object - | Tag::I256Object - | Tag::BytesObject - | Tag::StringObject - | Tag::SymbolObject - | Tag::VecObject - | Tag::MapObject - | Tag::ContractExecutableObject - | Tag::AddressObject - | Tag::LedgerKeyNonceObject => unreachable!(), - - Tag::ObjectCodeUpperBound => Ok(Ordering::Equal), - Tag::Bad => Ok(Ordering::Equal), + { + match a_tag { + Tag::False + | Tag::True + | Tag::Void + | Tag::LedgerKeyContractExecutable + | Tag::SmallCodeUpperBound + | Tag::ObjectCodeLowerBound + | Tag::ObjectCodeUpperBound + | Tag::Bad => Ok(Ordering::Equal), + + Tag::Status => delegate_compare_to_wrapper!(Status, a, b, self), + + Tag::U32Val => delegate_compare_to_wrapper!(U32Val, a, b, self), + Tag::I32Val => delegate_compare_to_wrapper!(I32Val, a, b, self), + + Tag::U64Small => delegate_compare_to_wrapper!(U64Small, a, b, self), + Tag::I64Small => delegate_compare_to_wrapper!(I64Small, a, b, self), + + Tag::TimepointSmall => delegate_compare_to_wrapper!(TimepointSmall, a, b, self), + Tag::DurationSmall => delegate_compare_to_wrapper!(DurationSmall, a, b, self), + + Tag::U128Small => delegate_compare_to_wrapper!(U128Small, a, b, self), + Tag::I128Small => delegate_compare_to_wrapper!(I128Small, a, b, self), + + Tag::U256Small => delegate_compare_to_wrapper!(U256Small, a, b, self), + Tag::I256Small => delegate_compare_to_wrapper!(I256Small, a, b, self), + + Tag::SymbolSmall => delegate_compare_to_wrapper!(SymbolSmall, a, b, self), + + // None of the object cases should be reachable, they + // should all have been handled by the is_object() branch + // above. + Tag::U64Object + | Tag::I64Object + | Tag::TimepointObject + | Tag::DurationObject + | Tag::U128Object + | Tag::I128Object + | Tag::U256Object + | Tag::I256Object + | Tag::BytesObject + | Tag::StringObject + | Tag::SymbolObject + | Tag::VecObject + | Tag::MapObject + | Tag::ContractExecutableObject + | Tag::AddressObject + | Tag::LedgerKeyNonceObject => unreachable!(), + } } + greater_or_less => Ok(greater_or_less), } } } diff --git a/soroban-env-common/src/env.rs b/soroban-env-common/src/env.rs index 43498c264..381ce8bea 100644 --- a/soroban-env-common/src/env.rs +++ b/soroban-env-common/src/env.rs @@ -46,7 +46,9 @@ pub trait EnvBase: Sized + Clone { /// Used to check two environments are the same, trapping if not. fn check_same_env(&self, other: &Self); + /// Used to clone an environment deeply, not just a handle to it. + #[allow(clippy::return_self_not_must_use)] fn deep_clone(&self) -> Self; // Helpers for methods that wish to pass Rust lifetime-qualified _slices_ @@ -285,7 +287,7 @@ macro_rules! generate_env_trait { /// This trait represents the interface between Host and Guest, used by /// client contract code and implemented (via [Env](crate::Env)) by the host. /// It consists of functions that take or return only 64-bit values such - /// as [RawVal] or [u64]. + /// as [`RawVal`] or [`u64`]. pub trait Env: EnvBase { $( diff --git a/soroban-env-common/src/env_val.rs b/soroban-env-common/src/env_val.rs index f1aa87c76..121d7e6bb 100644 --- a/soroban-env-common/src/env_val.rs +++ b/soroban-env-common/src/env_val.rs @@ -337,10 +337,7 @@ where unsafe { RawVal::from_body_and_tag(i.as_i64() as u64, Tag::I256Small) } } ScVal::Symbol(bytes) => { - let ss = match std::str::from_utf8(bytes.as_slice()) { - Ok(ss) => ss, - Err(_) => return Err(ConversionError), - }; + let Ok(ss) = std::str::from_utf8(bytes.as_slice()) else { return Err(ConversionError) }; SymbolSmall::try_from_str(ss)?.into() } ScVal::LedgerKeyContractExecutable => unsafe { diff --git a/soroban-env-common/src/lib.rs b/soroban-env-common/src/lib.rs index af33941e7..cba3303eb 100644 --- a/soroban-env-common/src/lib.rs +++ b/soroban-env-common/src/lib.rs @@ -91,6 +91,7 @@ pub use status::Status; pub use string::StringObject; pub use symbol::{Symbol, SymbolError, SymbolObject, SymbolSmall, SymbolSmallIter, SymbolStr}; +#[allow(clippy::missing_panics_doc)] #[inline(always)] // Awkward: this is a free function rather than a trait call because // you can't have const trait calls. It calls panic! rather than diff --git a/soroban-env-common/src/raw_val.rs b/soroban-env-common/src/raw_val.rs index 6a942e835..f54be03c4 100644 --- a/soroban-env-common/src/raw_val.rs +++ b/soroban-env-common/src/raw_val.rs @@ -497,10 +497,10 @@ impl RawVal { #[inline(always)] pub const fn get_tag(self) -> Tag { - let tag = self.get_tag_u8(); const A: u8 = Tag::SmallCodeUpperBound as u8; const B: u8 = Tag::ObjectCodeLowerBound as u8; const C: u8 = Tag::ObjectCodeUpperBound as u8; + let tag = self.get_tag_u8(); if !((tag < A) || (B < tag && tag < C)) { return Tag::Bad; } @@ -618,6 +618,7 @@ impl RawVal { impl Debug for RawVal { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + #[allow(clippy::trivially_copy_pass_by_ref)] fn fmt_obj(name: &str, r: &RawVal, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "{}(obj#{})", name, r.get_major()) } diff --git a/soroban-env-common/src/status.rs b/soroban-env-common/src/status.rs index 2b08d4315..76042dd75 100644 --- a/soroban-env-common/src/status.rs +++ b/soroban-env-common/src/status.rs @@ -140,14 +140,12 @@ impl Debug for Status { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { let st_res: Result = (self.as_raw().get_minor() as i32).try_into(); let code = self.as_raw().get_major(); - let st = match st_res { - Ok(t) => t, - Err(_) => return write!(f, "Status(UnknownType)"), - }; + let Ok(st) = st_res else { return write!(f, "Status(UnknownType)") }; write!(f, "Status({}(", st.name())?; match st { - ScStatusType::Ok => write!(f, "{code}"), - ScStatusType::UnknownError => write!(f, "{code}"), + ScStatusType::Ok | ScStatusType::ContractError | ScStatusType::UnknownError => { + write!(f, "{code}") + } ScStatusType::HostValueError => fmt_named_code::(code, f), ScStatusType::HostObjectError => fmt_named_code::(code, f), ScStatusType::HostFunctionError => fmt_named_code::(code, f), @@ -155,7 +153,6 @@ impl Debug for Status { ScStatusType::HostContextError => fmt_named_code::(code, f), ScStatusType::HostAuthError => fmt_named_code::(code, f), ScStatusType::VmError => fmt_named_code::(code, f), - ScStatusType::ContractError => write!(f, "{code}"), }?; write!(f, "))") } diff --git a/soroban-env-common/src/string.rs b/soroban-env-common/src/string.rs index 82c8ae99c..5fc862667 100644 --- a/soroban-env-common/src/string.rs +++ b/soroban-env-common/src/string.rs @@ -47,7 +47,7 @@ impl TryFromVal for RawVal { } #[cfg(feature = "std")] -impl<'a, E: Env> TryFromVal for StringObject { +impl TryFromVal for StringObject { type Error = ConversionError; fn try_from_val(env: &E, v: &String) -> Result { @@ -56,7 +56,7 @@ impl<'a, E: Env> TryFromVal for StringObject { } #[cfg(feature = "std")] -impl<'a, E: Env> TryFromVal for RawVal { +impl TryFromVal for RawVal { type Error = ConversionError; fn try_from_val(env: &E, v: &String) -> Result { diff --git a/soroban-env-common/src/symbol.rs b/soroban-env-common/src/symbol.rs index 1a5ff5b1f..643c40e3e 100644 --- a/soroban-env-common/src/symbol.rs +++ b/soroban-env-common/src/symbol.rs @@ -341,11 +341,9 @@ impl Iterator for SymbolSmallIter { impl FromIterator for SymbolSmall { fn from_iter>(iter: T) -> Self { - let mut n = 0; let mut accum: u64 = 0; - for i in iter { + for (n, i) in iter.into_iter().enumerate() { require(n < MAX_SMALL_CHARS); - n += 1; accum <<= CODE_BITS; let v = match i { '_' => 1, diff --git a/soroban-env-common/src/unimplemented_env.rs b/soroban-env-common/src/unimplemented_env.rs index f496e61e3..a811d9f94 100644 --- a/soroban-env-common/src/unimplemented_env.rs +++ b/soroban-env-common/src/unimplemented_env.rs @@ -68,10 +68,12 @@ impl EnvBase for UnimplementedEnv { unimplemented!() } + #[allow(clippy::needless_lifetimes)] fn string_new_from_slice<'a>(&self, _s: &'a str) -> Result { unimplemented!() } + #[allow(clippy::needless_lifetimes)] fn symbol_new_from_slice<'a>(&self, _s: &'a str) -> Result { unimplemented!() } diff --git a/soroban-env-host/src/storage.rs b/soroban-env-host/src/storage.rs index 20a46f146..fcdf6a9b9 100644 --- a/soroban-env-host/src/storage.rs +++ b/soroban-env-host/src/storage.rs @@ -285,9 +285,8 @@ impl Storage { FootprintMode::Enforcing => { self.footprint.enforce_access(key, ty, budget)?; match self.map.get::>(key, budget)? { - Some(None) => Ok(false), Some(Some(_)) => Ok(true), - None => Ok(false), + Some(None) | None => Ok(false), } } } diff --git a/soroban-native-sdk-macros/src/derive_fn.rs b/soroban-native-sdk-macros/src/derive_fn.rs index 2ab086317..d00e3d84e 100644 --- a/soroban-native-sdk-macros/src/derive_fn.rs +++ b/soroban-native-sdk-macros/src/derive_fn.rs @@ -4,7 +4,7 @@ use quote::{format_ident, quote}; use syn::{spanned::Spanned, Error, FnArg, Type}; pub fn derive_contract_function_set<'a>( - ty: &Box, + ty: &Type, methods: impl Iterator, ) -> TokenStream2 { let mut errors = Vec::::new(); @@ -19,10 +19,8 @@ pub fn derive_contract_function_set<'a>( let arg = format_ident!("arg{}", i); match a { FnArg::Typed(t) => (i, arg, t.ty), - _ => { - errors.push(Error::new(a.span(), "invalid argument type")); - (i, arg, syn::parse_quote! { () }) - } + FnArg::Receiver(_) => { errors.push(Error::new(a.span(), "invalid argument type")); + (i, arg, syn::parse_quote! { () })}, } }).multiunzip(); let num_args = args.len(); @@ -40,10 +38,7 @@ pub fn derive_contract_function_set<'a>( }) .multiunzip(); - if !errors.is_empty() { - let compile_errors = errors.iter().map(Error::to_compile_error); - quote! { #(#compile_errors)* } - } else { + if errors.is_empty() { quote! { impl crate::native_contract::NativeContract for #ty { fn call( @@ -62,5 +57,8 @@ pub fn derive_contract_function_set<'a>( } } } + } else { + let compile_errors = errors.iter().map(Error::to_compile_error); + quote! { #(#compile_errors)* } } } diff --git a/soroban-native-sdk-macros/src/derive_type.rs b/soroban-native-sdk-macros/src/derive_type.rs index 5f96e1a90..a784beaa5 100644 --- a/soroban-native-sdk-macros/src/derive_type.rs +++ b/soroban-native-sdk-macros/src/derive_type.rs @@ -146,10 +146,7 @@ pub fn derive_type_enum(ident: &Ident, data: &DataEnum) -> TokenStream2 { }) .multiunzip(); - if !errors.is_empty() { - let compile_errors = errors.iter().map(Error::to_compile_error); - quote! { #(#compile_errors)* } - } else { + if errors.is_empty() { quote! { impl #ident { @@ -208,5 +205,8 @@ pub fn derive_type_enum(ident: &Ident, data: &DataEnum) -> TokenStream2 { } } } + } else { + let compile_errors = errors.iter().map(Error::to_compile_error); + quote! { #(#compile_errors)* } } } diff --git a/soroban-synth-wasm/src/func_emitter.rs b/soroban-synth-wasm/src/func_emitter.rs index aeaecb574..c1858c0f8 100644 --- a/soroban-synth-wasm/src/func_emitter.rs +++ b/soroban-synth-wasm/src/func_emitter.rs @@ -94,8 +94,8 @@ impl FuncEmitter { /// directly to get the corresponding [`LocalRef`]s. pub fn new(mod_emit: ModEmitter, arity: Arity, n_locals: u32) -> Self { let func = Function::new([(n_locals, ValType::I64)]); - let args = (0..arity.0).map(|n| LocalRef(n)).collect(); - let locals = (arity.0..arity.0 + n_locals).map(|n| LocalRef(n)).collect(); + let args = (0..arity.0).map(LocalRef).collect(); + let locals = (arity.0..arity.0 + n_locals).map(LocalRef).collect(); Self { mod_emit, arity, From 690c7ece23614bf0ee9ef92f81222ea0c4da2433 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 13:26:19 -0400 Subject: [PATCH 04/19] chore: update CI to use clippy --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c8209aa05..b09ab2b18 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -64,7 +64,7 @@ jobs: with: name: cargo-hack version: 0.5.16 - - run: cargo hack --feature-powerset check --locked --target ${{ matrix.sys.target }} + - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator - if: matrix.sys.test run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }} From f6dcac4e1e75126ab3bbcf008cfa05e9636d083d Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 13:33:47 -0400 Subject: [PATCH 05/19] chore: clear fixes --- soroban-env-host/src/auth.rs | 66 +++---- soroban-env-host/src/budget.rs | 19 +- soroban-env-host/src/events/debug.rs | 17 +- soroban-env-host/src/events/internal.rs | 2 +- soroban-env-host/src/host.rs | 186 ++++++++---------- soroban-env-host/src/host/comparison.rs | 114 ++++++----- soroban-env-host/src/host/conversion.rs | 4 +- soroban-env-host/src/host/data_helper.rs | 4 +- soroban-env-host/src/host/err_helper.rs | 4 +- soroban-env-host/src/host/error.rs | 16 +- soroban-env-host/src/host/mem_helper.rs | 2 +- soroban-env-host/src/host/metered_clone.rs | 1 + soroban-env-host/src/host/metered_map.rs | 7 +- soroban-env-host/src/host/metered_vector.rs | 13 +- soroban-env-host/src/host/metered_xdr.rs | 4 +- soroban-env-host/src/host_object.rs | 2 +- .../src/native_contract/account_contract.rs | 2 +- .../src/native_contract/base_types.rs | 41 ++-- .../src/native_contract/testutils.rs | 17 +- .../src/native_contract/token/admin.rs | 8 +- .../src/native_contract/token/balance.rs | 61 +++--- .../src/native_contract/token/contract.rs | 38 ++-- .../src/native_contract/token/event.rs | 16 +- .../src/native_contract/token/metadata.rs | 2 +- .../src/native_contract/token/test_token.rs | 10 +- soroban-synth-wasm/src/func_emitter.rs | 4 +- soroban-synth-wasm/src/mod_emitter.rs | 15 +- 27 files changed, 320 insertions(+), 355 deletions(-) diff --git a/soroban-env-host/src/auth.rs b/soroban-env-host/src/auth.rs index d3f101ddc..bd1aee179 100644 --- a/soroban-env-host/src/auth.rs +++ b/soroban-env-host/src/auth.rs @@ -141,12 +141,12 @@ impl AuthorizedInvocation { let sub_invocations_xdr = xdr_invocation.sub_invocations.to_vec(); let sub_invocations = sub_invocations_xdr .into_iter() - .map(|i| AuthorizedInvocation::from_xdr(i)) + .map(AuthorizedInvocation::from_xdr) .collect::, _>>()?; Ok(Self { contract_id: xdr_invocation.contract_id.clone(), function_name: xdr_invocation.function_name.clone(), - args: xdr_invocation.args.clone(), + args: xdr_invocation.args, sub_invocations, is_exhausted: false, }) @@ -189,7 +189,7 @@ impl AuthorizedInvocation { sub_invocations: self .sub_invocations .iter() - .map(|i| i.to_xdr_non_metered()) + .map(AuthorizedInvocation::to_xdr_non_metered) .collect::, HostError>>()? .try_into() .map_err(|_| HostError::from(ScUnknownErrorCode::General))?, @@ -355,11 +355,7 @@ impl AuthorizationManager { // that when their stack no longer has authorized // invocation, then we've popped frames past its root // and hence need to create a new tracker. - if !tracker.has_authorized_invocations_in_stack() { - recording_info - .tracker_by_address_handle - .remove(&address_obj_handle); - } else { + if tracker.has_authorized_invocations_in_stack() { return self.trackers[*tracker_id].record_invocation( host, &curr_invocation.contract_id, @@ -367,6 +363,9 @@ impl AuthorizationManager { args, ); } + recording_info + .tracker_by_address_handle + .remove(&address_obj_handle); } // If a tracker for the new tree doesn't exist yet, create // it and initialize with the current invocation. @@ -429,9 +428,9 @@ impl AuthorizationManager { // We could also make this included into the authorized stack to // generalize all the host function invocations. Frame::HostFunction(_) => return Ok(()), - Frame::Token(id, fn_name, _) => (id.metered_clone(&self.budget)?, fn_name.clone()), + Frame::Token(id, fn_name, _) => (id.metered_clone(&self.budget)?, *fn_name), #[cfg(any(test, feature = "testutils"))] - Frame::TestContract(tc) => (tc.id.clone(), tc.func.clone()), + Frame::TestContract(tc) => (tc.id.clone(), tc.func), }; let Ok(ScVal::Symbol(function_name)) = host.from_host_val(function_name.to_raw()) else { return Err(host.err_status(xdr::ScHostObjErrorCode::UnexpectedType)) @@ -468,11 +467,11 @@ impl AuthorizationManager { // Should only be called in the recording mode. pub(crate) fn get_recorded_auth_payloads(&self) -> Result, HostError> { match &self.mode { - AuthorizationMode::Enforcing => return Err(ScUnknownErrorCode::General.into()), + AuthorizationMode::Enforcing => Err(ScUnknownErrorCode::General.into()), AuthorizationMode::Recording(_) => Ok(self .trackers .iter() - .map(|tracker| tracker.get_recorded_auth_payload()) + .map(AuthorizationTracker::get_recorded_auth_payload) .collect::, HostError>>()?), } } @@ -580,10 +579,10 @@ impl AuthorizationTracker { } else { false }; - let nonce = if !is_invoker { - Some(host.read_and_consume_nonce(&contract_id, &address)?) - } else { + let nonce = if is_invoker { None + } else { + Some(host.read_and_consume_nonce(contract_id, &address)?) }; // Create the stack of `None` leading to the current invocation to // represent invocations that didn't need authorization on behalf of @@ -626,10 +625,8 @@ impl AuthorizationTracker { if !self.is_valid { return Ok(false); } - let frame_is_already_authorized = match self.invocation_id_in_call_stack.last() { - Some(Some(_)) => true, - _ => false, - }; + let frame_is_already_authorized = + matches!(self.invocation_id_in_call_stack.last(), Some(Some(_))); if frame_is_already_authorized || !self.maybe_push_matching_invocation_frame(contract_id, function_name, args) { @@ -663,10 +660,8 @@ impl AuthorizationTracker { function_name: &ScSymbol, args: ScVec, ) -> Result<(), HostError> { - let frame_is_already_authorized = match self.invocation_id_in_call_stack.last() { - Some(Some(_)) => true, - _ => false, - }; + let frame_is_already_authorized = + matches!(self.invocation_id_in_call_stack.last(), Some(Some(_))); if frame_is_already_authorized { return Err(ScHostAuthErrorCode::DuplicateAuthorization.into()); } @@ -704,7 +699,7 @@ impl AuthorizationTracker { // Checks if there is at least one authorized invocation in the current call // stack. fn has_authorized_invocations_in_stack(&self) -> bool { - self.invocation_id_in_call_stack.iter().any(|i| i.is_some()) + self.invocation_id_in_call_stack.iter().any(Option::is_some) } fn invocation_to_xdr(&self, budget: &Budget) -> Result { @@ -774,15 +769,13 @@ impl AuthorizationTracker { break; } } - } else { - if !self.root_authorized_invocation.is_exhausted - && &self.root_authorized_invocation.contract_id == contract_id - && &self.root_authorized_invocation.function_name == function_name - && &self.root_authorized_invocation.args == args - { - frame_index = Some(0); - self.root_authorized_invocation.is_exhausted = true; - } + } else if !self.root_authorized_invocation.is_exhausted + && &self.root_authorized_invocation.contract_id == contract_id + && &self.root_authorized_invocation.function_name == function_name + && &self.root_authorized_invocation.args == args + { + frame_index = Some(0); + self.root_authorized_invocation.is_exhausted = true; } if frame_index.is_some() { *self.invocation_id_in_call_stack.last_mut().unwrap() = frame_index; @@ -820,7 +813,6 @@ impl AuthorizationTracker { invocation: self.invocation_to_xdr(host.budget_ref())?, nonce: self .nonce - .clone() .ok_or_else(|| host.err_general("unexpected missing nonce"))?, }); @@ -837,7 +829,7 @@ impl AuthorizationTracker { let payload = self.get_signature_payload(host)?; match address { ScAddress::Account(acc) => { - check_account_authentication(host, &acc, &payload, &self.signature_args)?; + check_account_authentication(host, acc, &payload, &self.signature_args)?; } ScAddress::Contract(acc_contract) => { check_account_contract_auth( @@ -898,7 +890,7 @@ impl Host { self.with_mut_storage(|storage| storage.get(&nonce_key, self.budget_ref()))?; match &entry.data { LedgerEntryData::ContractData(ContractDataEntry { val, .. }) => match val { - ScVal::U64(val) => val.clone(), + ScVal::U64(val) => *val, _ => { return Err(self.err_general("unexpected nonce entry type")); } @@ -929,7 +921,7 @@ impl Host { self.with_mut_storage(|storage| storage.get(&nonce_key, self.budget_ref()))?; match &entry.data { LedgerEntryData::ContractData(ContractDataEntry { val, .. }) => match val { - ScVal::U64(val) => val.clone(), + ScVal::U64(val) => *val, _ => { return Err(self.err_general("unexpected nonce entry type")); } diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index a2de23aa2..2e94831ea 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -102,7 +102,7 @@ pub enum CostType { // TODO: add XDR support for iterating over all the elements of an enum impl CostType { pub fn variants() -> std::slice::Iter<'static, CostType> { - static VARIANTS: &'static [CostType] = &[ + static VARIANTS: &[CostType] = &[ CostType::WasmInsnExec, CostType::WasmMemAlloc, CostType::HostEventDebug, @@ -208,8 +208,7 @@ impl CostModel { fn next_2(x: u64) -> u64 { match x.checked_next_power_of_two() { None => 0x1000_0000_0000_0000, - Some(0) => 2, - Some(1) => 2, + Some(0 | 1) => 2, Some(n) => n, } } @@ -399,7 +398,7 @@ impl Budget { where F: FnOnce(&mut u64), { - f(&mut self.0.borrow_mut().inputs[ty as usize]) + f(&mut self.0.borrow_mut().inputs[ty as usize]); } pub fn get_cpu_insns_count(&self) -> u64 { @@ -411,7 +410,7 @@ impl Budget { } pub fn reset_default(&self) { - *self.0.borrow_mut() = BudgetImpl::default() + *self.0.borrow_mut() = BudgetImpl::default(); } pub fn reset_unlimited(&self) { @@ -425,7 +424,7 @@ impl Budget { Ok(()) }) .unwrap(); // panic means multiple-mut-borrow bug - self.reset_inputs() + self.reset_inputs(); } pub fn reset_unlimited_mem(&self) { @@ -434,11 +433,11 @@ impl Budget { Ok(()) }) .unwrap(); // panic means multiple-mut-borrow bug - self.reset_inputs() + self.reset_inputs(); } pub fn reset_inputs(&self) { - for i in self.0.borrow_mut().inputs.iter_mut() { + for i in &mut self.0.borrow_mut().inputs { *i = 0; } } @@ -451,7 +450,7 @@ impl Budget { }) .unwrap(); // impossible to panic - self.reset_inputs() + self.reset_inputs(); } #[cfg(test)] @@ -477,7 +476,7 @@ impl Default for BudgetImpl { for ct in CostType::variants() { b.inputs.push(0); - let cpu = &mut b.cpu_insns.get_cost_model_mut(*ct); + let cpu = b.cpu_insns.get_cost_model_mut(*ct); match ct { // We might want to split wasm insns into separate cases; some are much more than // this and some are much less. diff --git a/soroban-env-host/src/events/debug.rs b/soroban-env-host/src/events/debug.rs index 0adc7d5f8..e6ff3cc50 100644 --- a/soroban-env-host/src/events/debug.rs +++ b/soroban-env-host/src/events/debug.rs @@ -35,8 +35,8 @@ impl Default for DebugArg { impl Display for DebugArg { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - DebugArg::Str(s) => write!(f, "{}", s), - DebugArg::Val(rv) => write!(f, "{:?}", rv), + DebugArg::Str(s) => write!(f, "{s}"), + DebugArg::Val(rv) => write!(f, "{rv:?}"), } } } @@ -44,7 +44,7 @@ impl Display for DebugArg { /// A cheap record type to store in the events buffer for diagnostic reporting /// when something goes wrong. Should cost very little even when enabled. See /// [host::Host::debug_event](crate::host::Host::debug_event) for normal use. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct DebugEvent { pub msg: Option>, pub args: TinyVec<[DebugArg; 2]>, @@ -55,13 +55,13 @@ impl core::fmt::Display for DebugEvent { match &self.msg { None => { for arg in self.args.iter() { - write!(f, "{}", arg)?; + write!(f, "{arg}")?; } Ok(()) } Some(fmt) => { let args = dyn_fmt::Arguments::new(fmt, self.args.as_slice()); - write!(f, "{}", args) + write!(f, "{args}") } } } @@ -69,10 +69,7 @@ impl core::fmt::Display for DebugEvent { impl DebugEvent { pub fn new() -> Self { - Self { - msg: None, - args: Default::default(), - } + Self::default() } pub fn msg(mut self, msg: impl Into>) -> Self { @@ -86,7 +83,7 @@ impl DebugEvent { } pub fn args>(mut self, args: impl IntoIterator) -> Self { - self.args.extend(args.into_iter().map(|arg| arg.into())); + self.args.extend(args.into_iter().map(Into::into)); self } } diff --git a/soroban-env-host/src/events/internal.rs b/soroban-env-host/src/events/internal.rs index 5e3db44e7..0ef8cf578 100644 --- a/soroban-env-host/src/events/internal.rs +++ b/soroban-env-host/src/events/internal.rs @@ -91,7 +91,7 @@ impl InternalEventsBuffer { InternalEvent::StructuredDebug(c) => debug!("StructuredDebug event: {:?}", c), } } - debug!("========End of events========") + debug!("========End of events========"); } /// Converts the internal events into their external representation. This should only be called diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index e87616048..1072a361a 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -280,7 +280,7 @@ impl Host { } pub fn set_ledger_info(&self, info: LedgerInfo) { - *self.0.ledger.borrow_mut() = Some(info) + *self.0.ledger.borrow_mut() = Some(info); } pub fn with_ledger_info(&self, f: F) -> Result @@ -299,7 +299,10 @@ impl Host { { match self.0.ledger.borrow_mut().as_mut() { None => Err(self.err_general("missing ledger info")), - Some(li) => Ok(f(li)), + Some(li) => { + f(li); + Ok(()) + } } } @@ -337,7 +340,7 @@ impl Host { where F: FnOnce(&mut InternalEventsBuffer) -> Result, { - f(&mut *self.0.events.borrow_mut()) + f(&mut self.0.events.borrow_mut()) } /// Records a debug event. This in itself is not necessarily an error; it @@ -387,7 +390,7 @@ impl Host { where F: FnOnce(&mut Storage) -> Result, { - f(&mut *self.0.storage.borrow_mut()) + f(&mut self.0.storage.borrow_mut()) } /// Accept a _unique_ (refcount = 1) host reference and destroy the @@ -668,7 +671,7 @@ impl Host { } pub fn get_events(&self) -> Result { - self.0.events.borrow().externalize(&self) + self.0.events.borrow().externalize(self) } pub(crate) fn from_host_val(&self, val: RawVal) -> Result { @@ -770,7 +773,7 @@ impl Host { self.charge_budget(CostType::ScVecToHostVec, v.len() as u64)?; let mut vv = Vec::with_capacity(v.len()); for e in v.iter() { - vv.push(self.to_host_val(e)?) + vv.push(self.to_host_val(e)?); } Ok(self.add_host_object(HostVec::from_vec(vv)?)?.into()) } @@ -780,7 +783,7 @@ impl Host { for pair in m.iter() { let k = self.to_host_val(&pair.key)?; let v = self.to_host_val(&pair.val)?; - mm.push((k, v)) + mm.push((k, v)); } Ok(self.add_host_object(HostMap::from_map(mm, self)?)?.into()) } @@ -797,12 +800,8 @@ impl Host { ScVal::I128(i) => Ok(self .add_host_object((i.lo as u128 | ((i.hi as u128) << 64)) as i128)? .into()), - ScVal::U256(u) => Ok(self - .add_host_object(U256::from_be_bytes(u.0.clone()))? - .into()), - ScVal::I256(i) => Ok(self - .add_host_object(I256::from_be_bytes(i.0.clone()))? - .into()), + ScVal::U256(u) => Ok(self.add_host_object(U256::from_be_bytes(u.0))?.into()), + ScVal::I256(i) => Ok(self.add_host_object(I256::from_be_bytes(i.0))?.into()), ScVal::Bytes(b) => Ok(self .add_host_object(b.metered_clone(&self.0.budget)?)? .into()), @@ -959,13 +958,12 @@ impl Host { } #[cfg(not(feature = "vm"))] ScContractExecutable::WasmRef(_) => Err(self.err_general("could not dispatch")), - ScContractExecutable::Token => self.with_frame( - Frame::Token(id.clone(), func.clone(), args.to_vec()), - || { + ScContractExecutable::Token => { + self.with_frame(Frame::Token(id.clone(), *func, args.to_vec()), || { use crate::native_contract::{NativeContract, Token}; Token.call(func, self, args) - }, - ), + }) + } } } @@ -991,15 +989,14 @@ impl Host { ) -> Result { // Internal host calls may call some special functions that otherwise // aren't allowed to be called. - if !internal_host_call { - if SymbolStr::try_from_val(self, &func)?.to_string().as_str() + if !internal_host_call + && SymbolStr::try_from_val(self, &func)?.to_string().as_str() == ACCOUNT_CONTRACT_CHECK_AUTH_FN_NAME - { - return Err(self.err_status_msg( - ScHostContextErrorCode::UnknownError, - "can't invoke a custom account contract directly", - )); - } + { + return Err(self.err_status_msg( + ScHostContextErrorCode::UnknownError, + "can't invoke a custom account contract directly", + )); } if !matches!(reentry_mode, ContractReentryMode::Allowed) { let mut is_last_non_host_frame = true; @@ -1040,9 +1037,9 @@ impl Host { // if let Some(cfs) = self.0.contracts.borrow().get(&id).cloned() { ... } // // maintains a borrow of self.0.contracts, which can cause borrow errors. - let cfs_option = self.0.contracts.borrow().get(&id).cloned(); + let cfs_option = self.0.contracts.borrow().get(id).cloned(); if let Some(cfs) = cfs_option { - let frame = TestContractFrame::new(id.clone(), func.clone(), args.to_vec()); + let frame = TestContractFrame::new(id.clone(), func, args.to_vec()); let panic = frame.panic.clone(); return self.with_frame(Frame::TestContract(frame), || { use std::any::Any; @@ -1150,11 +1147,11 @@ impl Host { } HostFunction::CreateContract(args) => self .with_frame(Frame::HostFunction(hf_type), || { - self.create_contract(args).map(|obj| ::from(obj)) + self.create_contract(args).map(::from) }), HostFunction::InstallContractCode(args) => self .with_frame(Frame::HostFunction(hf_type), || { - self.install_contract(args).map(|obj| ::from(obj)) + self.install_contract(args).map(::from) }), } } @@ -1174,8 +1171,8 @@ impl Host { ) -> Result<(), HostError> { let hash = self.hash_from_bytesobj_input("contract_id", contract_id)?; let mut contracts = self.0.contracts.borrow_mut(); - if !contracts.contains_key(&hash) { - contracts.insert(hash, contract_fns); + if let std::collections::hash_map::Entry::Vacant(e) = contracts.entry(hash) { + e.insert(contract_fns); Ok(()) } else { Err(self.err_general("vtable already exists")) @@ -1227,7 +1224,7 @@ impl Host { ContractId::SourceAccount(salt) => self.id_preimage_from_source_account(salt)?, ContractId::Ed25519PublicKey(key_with_signature) => { let signature_payload_preimage = self.create_contract_args_hash_preimage( - args.source.metered_clone(&self.budget_ref())?, + args.source.metered_clone(self.budget_ref())?, key_with_signature.salt.metered_clone(self.budget_ref())?, )?; let signature_payload = self.metered_hash_xdr(&signature_payload_preimage)?; @@ -1282,7 +1279,7 @@ impl Host { use ed25519_dalek::Verifier; self.charge_budget(CostType::VerifyEd25519Sig, payload.len() as u64)?; public_key - .verify(payload, &sig) + .verify(payload, sig) .map_err(|_| self.err_general("Failed ED25519 verification")) } @@ -1502,8 +1499,8 @@ impl EnvBase for Host { } let pair_iter = key_syms .iter() - .map(|s| s.to_raw()) - .zip(vals.iter().cloned()); + .map(Symbol::to_raw) + .zip(vals.iter().copied()); let map = HostMap::from_exact_iter(pair_iter, self)?; self.add_host_object(map) } @@ -1539,7 +1536,7 @@ impl EnvBase for Host { } fn vec_new_from_slice(&self, vals: &[RawVal]) -> Result { - let map = HostVec::from_exact_iter(vals.iter().cloned(), self.budget_ref())?; + let map = HostVec::from_exact_iter(vals.iter().copied(), self.budget_ref())?; self.add_host_object(map) } @@ -1564,10 +1561,8 @@ impl EnvBase for Host { fn symbol_index_in_strs(&self, sym: Symbol, slices: &[&str]) -> Result { let mut found = None; self.metered_scan_slice_of_slices(slices, |i, slice| { - if self.symbol_matches(slice.as_bytes(), sym)? { - if found.is_none() { - found = Some(i) - } + if self.symbol_matches(slice.as_bytes(), sym)? && found.is_none() { + found = Some(i); } Ok(()) })?; @@ -1606,10 +1601,10 @@ impl EnvBase for Host { ) -> Result<(), HostError> { let mut evt = DebugEvent::new().msg(fmt); for v in vals { - evt = evt.arg(*v) + evt = evt.arg(*v); } for s in strs { - evt = evt.arg(*s) + evt = evt.arg(*s); } self.record_debug_event(evt) } @@ -1639,7 +1634,7 @@ impl VmCallerEnv for Host { self.err_general("log_fmt_values fmt string contains is invalid utf8") })?; let args: HostVec = self.visit_obj(args, move |hv: &HostVec| Ok(hv.clone()))?; - self.record_debug_event(DebugEvent::new().msg(fmt).args(args.iter().cloned()))?; + self.record_debug_event(DebugEvent::new().msg(fmt).args(args.iter().copied()))?; } Ok(RawVal::VOID) } @@ -1659,7 +1654,7 @@ impl VmCallerEnv for Host { a: RawVal, b: RawVal, ) -> Result { - let res = match unsafe { + let res = if let Some(res) = unsafe { match (Object::try_from(a), Object::try_from(b)) { // We were given two objects: compare them. (Ok(a), Ok(b)) => self.unchecked_visit_val_obj(a, |ao| { @@ -1689,22 +1684,15 @@ impl VmCallerEnv for Host { (Err(_), Err(_)) => return Err(self.err_general("two non-object args to obj_cmp")), } } { - // If any of the above got us a result, great, use it. - Some(res) => res, - - // Otherwise someone gave us an object and a non-paired value (not a small-value - // case of the same type). Order hese by their tags. - None => { - let atag = a.get_tag(); - let btag = b.get_tag(); - if atag == btag { - // This shouldn't have happened, but if it does there's a logic error. - return Err( - self.err_general("equal-tagged values rejected by small-value obj_cmp") - ); - } - a.get_tag().cmp(&b.get_tag()) + res + } else { + let atag = a.get_tag(); + let btag = b.get_tag(); + if atag == btag { + // This shouldn't have happened, but if it does there's a logic error. + return Err(self.err_general("equal-tagged values rejected by small-value obj_cmp")); } + a.get_tag().cmp(&b.get_tag()) }; // Finally, translate Ordering::Foo to a number to return to caller. Ok(match res { @@ -1721,7 +1709,7 @@ impl VmCallerEnv for Host { data: RawVal, ) -> Result { self.record_contract_event(ContractEventType::Contract, topics, data)?; - Ok(RawVal::VOID.into()) + Ok(RawVal::VOID) } // Notes on metering: covered by the components. @@ -1729,11 +1717,9 @@ impl VmCallerEnv for Host { &self, _vmcaller: &mut VmCaller, ) -> Result { - Ok(self - .add_host_object(ScAddress::Contract( - self.get_current_contract_id_internal()?, - ))? - .into()) + self.add_host_object(ScAddress::Contract( + self.get_current_contract_id_internal()?, + )) } // Notes on metering: covered by `add_host_object`. @@ -1833,7 +1819,7 @@ impl VmCallerEnv for Host { ) -> Result { self.visit_obj(m, move |hm: &HostMap| { hm.get(&k, self)? - .map(|v| *v) + .copied() .ok_or_else(|| self.err_general("map key not found")) // FIXME: need error code }) } @@ -1845,7 +1831,7 @@ impl VmCallerEnv for Host { k: RawVal, ) -> Result { match self.visit_obj(m, |hm: &HostMap| hm.remove(&k, self))? { - Some((mnew, _)) => Ok(self.add_host_object(mnew)?.into()), + Some((mnew, _)) => self.add_host_object(mnew), None => Err(self.err_general("map key not found")), } } @@ -1926,7 +1912,7 @@ impl VmCallerEnv for Host { m: MapObject, ) -> Result { let vec = self.visit_obj(m, |hm: &HostMap| { - HostVec::from_exact_iter(hm.keys(self)?.cloned(), self.budget_ref()) + HostVec::from_exact_iter(hm.keys(self)?.copied(), self.budget_ref()) })?; self.add_host_object(vec) } @@ -1937,7 +1923,7 @@ impl VmCallerEnv for Host { m: MapObject, ) -> Result { let vec = self.visit_obj(m, |hm: &HostMap| { - HostVec::from_exact_iter(hm.values(self)?.cloned(), self.budget_ref()) + HostVec::from_exact_iter(hm.values(self)?.copied(), self.budget_ref()) })?; self.add_host_object(vec) } @@ -2071,7 +2057,7 @@ impl VmCallerEnv for Host { self.validate_index_lt_bound(i, hv.len())?; hv.set(i as usize, x, self.as_budget()) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_get( @@ -2112,7 +2098,7 @@ impl VmCallerEnv for Host { x: RawVal, ) -> Result { let vnew = self.visit_obj(v, move |hv: &HostVec| hv.push_front(x, self.as_budget()))?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_pop_front( @@ -2121,7 +2107,7 @@ impl VmCallerEnv for Host { v: VecObject, ) -> Result { let vnew = self.visit_obj(v, move |hv: &HostVec| hv.pop_front(self.as_budget()))?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_push_back( @@ -2131,7 +2117,7 @@ impl VmCallerEnv for Host { x: RawVal, ) -> Result { let vnew = self.visit_obj(v, move |hv: &HostVec| hv.push_back(x, self.as_budget()))?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_pop_back( @@ -2140,7 +2126,7 @@ impl VmCallerEnv for Host { v: VecObject, ) -> Result { let vnew = self.visit_obj(v, move |hv: &HostVec| hv.pop_back(self.as_budget()))?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_front(&self, _vmcaller: &mut VmCaller, v: VecObject) -> Result { @@ -2167,7 +2153,7 @@ impl VmCallerEnv for Host { self.validate_index_le_bound(i, hv.len())?; hv.insert(i as usize, x, self.as_budget()) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_append( @@ -2185,7 +2171,7 @@ impl VmCallerEnv for Host { } }) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_slice( @@ -2201,7 +2187,7 @@ impl VmCallerEnv for Host { let range = self.valid_range_from_start_end_bound(start, end, hv.len())?; hv.slice(range, self.as_budget()) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn vec_first_index_of( @@ -2212,7 +2198,7 @@ impl VmCallerEnv for Host { ) -> Result { self.visit_obj(v, |hv: &HostVec| { Ok( - match hv.first_index_of(|other| self.compare(&x, other), &self.as_budget())? { + match hv.first_index_of(|other| self.compare(&x, other), self.as_budget())? { Some(u) => self.usize_to_u32val(u)?.into(), None => RawVal::VOID.into(), }, @@ -2342,7 +2328,7 @@ impl VmCallerEnv for Host { contract_id, key, val, - }) => Ok(self.to_host_val(&val)?.into()), + }) => Ok(self.to_host_val(val)?), _ => Err(self.err_status_msg( ScHostStorageErrorCode::ExpectContractData, "expected contract data", @@ -2444,7 +2430,7 @@ impl VmCallerEnv for Host { let scv = self.from_host_val(v)?; let mut buf = Vec::::new(); self.metered_write_xdr(&scv, &mut buf)?; - Ok(self.add_host_object(self.scbytes_from_vec(buf)?)?.into()) + self.add_host_object(self.scbytes_from_vec(buf)?) } // Notes on metering: covered by components @@ -2456,7 +2442,7 @@ impl VmCallerEnv for Host { let scv = self.visit_obj(b, |hv: &ScBytes| { self.metered_from_xdr::(hv.as_slice()) })?; - Ok(self.to_host_val(&scv)?.into()) + self.to_host_val(&scv) } fn string_copy_to_linear_memory( @@ -2598,9 +2584,7 @@ impl VmCallerEnv for Host { // Notes on metering: covered by `add_host_object` fn bytes_new(&self, _vmcaller: &mut VmCaller) -> Result { - Ok(self - .add_host_object(self.scbytes_from_vec(Vec::::new())?)? - .into()) + self.add_host_object(self.scbytes_from_vec(Vec::::new())?) } // Notes on metering: `get_mut` is free @@ -2623,7 +2607,7 @@ impl VmCallerEnv for Host { } } })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } // Notes on metering: `get` is free @@ -2655,7 +2639,7 @@ impl VmCallerEnv for Host { vnew.remove(i as usize); Ok(ScBytes(vnew.try_into()?)) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } // Notes on metering: `len` is free @@ -2703,7 +2687,7 @@ impl VmCallerEnv for Host { vnew.push(u); Ok(ScBytes(vnew.try_into()?)) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } // Notes on metering: `pop` is free @@ -2721,7 +2705,7 @@ impl VmCallerEnv for Host { } Ok(ScBytes(vnew.try_into()?)) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } // Notes on metering: `first` is free @@ -2766,7 +2750,7 @@ impl VmCallerEnv for Host { vnew.insert(i as usize, u); Ok(ScBytes(vnew.try_into()?)) })?; - Ok(self.add_host_object(vnew)?.into()) + self.add_host_object(vnew) } fn bytes_append( @@ -2776,15 +2760,15 @@ impl VmCallerEnv for Host { b2: BytesObject, ) -> Result { let mut vnew: Vec = self - .visit_obj(b1, |hv: &ScBytes| Ok(hv.metered_clone(&self.0.budget)?))? + .visit_obj(b1, |hv: &ScBytes| hv.metered_clone(&self.0.budget))? .into(); - let b2 = self.visit_obj(b2, |hv: &ScBytes| Ok(hv.metered_clone(&self.0.budget)?))?; + let b2 = self.visit_obj(b2, |hv: &ScBytes| hv.metered_clone(&self.0.budget))?; if b2.len() > u32::MAX as usize - vnew.len() { return Err(self.err_status_msg(ScHostFnErrorCode::InputArgsInvalid, "u32 overflow")); } self.charge_budget(CostType::BytesAppend, (vnew.len() + b2.len()) as u64)?; // worst case can cause rellocation vnew.extend(b2.iter()); - Ok(self.add_host_object(ScBytes(vnew.try_into()?))?.into()) + self.add_host_object(ScBytes(vnew.try_into()?)) } fn bytes_slice( @@ -2846,11 +2830,9 @@ impl VmCallerEnv for Host { &self, _vmcaller: &mut VmCaller, ) -> Result { - Ok(self - .with_ledger_info(|li| { - self.add_host_object(self.scbytes_from_slice(li.network_id.as_slice())?) - })? - .into()) + self.with_ledger_info(|li| { + self.add_host_object(self.scbytes_from_slice(li.network_id.as_slice())?) + }) } fn get_current_call_stack( @@ -2861,7 +2843,7 @@ impl VmCallerEnv for Host { let get_host_val_tuple = |id: &Hash, function: &Symbol| -> Result<[RawVal; 2], HostError> { let id_val = self.add_host_object(self.scbytes_from_hash(id)?)?.into(); - let function_val = function.clone().into(); + let function_val = function.into(); Ok([id_val, function_val]) }; @@ -2873,14 +2855,14 @@ impl VmCallerEnv for Host { get_host_val_tuple(&vm.contract_id, &function)? } Frame::HostFunction(_) => continue, - Frame::Token(id, function, _) => get_host_val_tuple(&id, &function)?, + Frame::Token(id, function, _) => get_host_val_tuple(id, function)?, #[cfg(any(test, feature = "testutils"))] Frame::TestContract(tc) => get_host_val_tuple(&tc.id, &tc.func)?, }; let inner = MeteredVector::from_array(&vals, self.as_budget())?; outer.push(self.add_host_object(inner)?.into()); } - Ok(self.add_host_object(HostVec::from_vec(outer)?)?.into()) + self.add_host_object(HostVec::from_vec(outer)?) } fn fail_with_status( @@ -2940,7 +2922,7 @@ impl VmCallerEnv for Host { #[cfg(any(test, feature = "testutils"))] Frame::TestContract(c) => &c.args, }; - Ok(self.rawvals_to_scvec(args.iter())?) + self.rawvals_to_scvec(args.iter()) })?; Ok(self diff --git a/soroban-env-host/src/host/comparison.rs b/soroban-env-host/src/host/comparison.rs index 0e255f33f..84e20a796 100644 --- a/soroban-env-host/src/host/comparison.rs +++ b/soroban-env-host/src/host/comparison.rs @@ -71,22 +71,25 @@ impl Compare for Host { // List out at least one side of all the remaining cases here so // we don't accidentally forget to update this when/if a new // HostObject type is added. - (U64(_), _) - | (TimePoint(_), _) - | (Duration(_), _) - | (I64(_), _) - | (U128(_), _) - | (I128(_), _) - | (U256(_), _) - | (I256(_), _) - | (Vec(_), _) - | (Map(_), _) - | (Bytes(_), _) - | (String(_), _) - | (Symbol(_), _) - | (ContractExecutable(_), _) - | (Address(_), _) - | (NonceKey(_), _) => { + ( + U64(_) + | TimePoint(_) + | Duration(_) + | I64(_) + | U128(_) + | I128(_) + | U256(_) + | I256(_) + | Vec(_) + | Map(_) + | Bytes(_) + | String(_) + | Symbol(_) + | ContractExecutable(_) + | Address(_) + | NonceKey(_), + _, + ) => { let a = host_obj_discriminant(a); let b = host_obj_discriminant(b); Ok(a.cmp(&b)) @@ -134,7 +137,7 @@ impl Compare> for Budg CostType::BytesCmp, ::DECLARED_SIZE, )?; - Ok(a.0.cmp(&b.0)) + Ok(a.0.cmp(b.0)) } } @@ -244,7 +247,7 @@ impl Compare for Budget { (Vec(Some(a)), Vec(Some(b))) => self.compare(a, b), (Map(Some(a)), Map(Some(b))) => self.compare(a, b), - (Vec(None), _) | (_, Vec(None)) | (Map(None), _) | (_, Map(None)) => { + (Vec(None) | Map(None), _) | (_, Vec(None) | Map(None)) => { Err(ScHostValErrorCode::MissingObject.into()) } @@ -260,28 +263,31 @@ impl Compare for Budget { >::compare(self, &a.as_slice(), &b.as_slice()) } - (Bool(_), _) - | (Void, _) - | (Status(_), _) - | (U32(_), _) - | (I32(_), _) - | (U64(_), _) - | (I64(_), _) - | (Timepoint(_), _) - | (Duration(_), _) - | (U128(_), _) - | (I128(_), _) - | (U256(_), _) - | (I256(_), _) - | (Bytes(_), _) - | (String(_), _) - | (Symbol(_), _) - | (Vec(_), _) - | (Map(_), _) - | (ContractExecutable(_), _) - | (Address(_), _) - | (LedgerKeyContractExecutable, _) - | (LedgerKeyNonce(_), _) => Ok(a.cmp(b)), + ( + Bool(_) + | Void + | Status(_) + | U32(_) + | I32(_) + | U64(_) + | I64(_) + | Timepoint(_) + | Duration(_) + | U128(_) + | I128(_) + | U256(_) + | I256(_) + | Bytes(_) + | String(_) + | Symbol(_) + | Vec(_) + | Map(_) + | ContractExecutable(_) + | Address(_) + | LedgerKeyContractExecutable + | LedgerKeyNonce(_), + _, + ) => Ok(a.cmp(b)), } } } @@ -307,15 +313,11 @@ impl Compare for Budget { // List out one side of each remaining unequal-discriminant case so // we remember to update this code if LedgerKey changes. We don't // charge for these since they're just 1-integer compares. - (Account(_), _) - | (Trustline(_), _) - | (Offer(_), _) - | (Data(_), _) - | (ClaimableBalance(_), _) - | (LiquidityPool(_), _) - | (ContractData(_), _) - | (ContractCode(_), _) - | (ConfigSetting(_), _) => Ok(a.cmp(b)), + ( + Account(_) | Trustline(_) | Offer(_) | Data(_) | ClaimableBalance(_) + | LiquidityPool(_) | ContractData(_) | ContractCode(_) | ConfigSetting(_), + _, + ) => Ok(a.cmp(b)), } } } @@ -350,15 +352,11 @@ impl Compare for Budget { (ContractCode(a), ContractCode(b)) => self.compare(&a, &b), (ConfigSetting(a), ConfigSetting(b)) => self.compare(&a, &b), - (Account(_), _) - | (Trustline(_), _) - | (Offer(_), _) - | (Data(_), _) - | (ClaimableBalance(_), _) - | (LiquidityPool(_), _) - | (ContractData(_), _) - | (ContractCode(_), _) - | (ConfigSetting(_), _) => Ok(a.cmp(b)), + ( + Account(_) | Trustline(_) | Offer(_) | Data(_) | ClaimableBalance(_) + | LiquidityPool(_) | ContractData(_) | ContractCode(_) | ConfigSetting(_), + _, + ) => Ok(a.cmp(b)), } } } diff --git a/soroban-env-host/src/host/conversion.rs b/soroban-env-host/src/host/conversion.rs index 17c1e7229..1db511b15 100644 --- a/soroban-env-host/src/host/conversion.rs +++ b/soroban-env-host/src/host/conversion.rs @@ -23,7 +23,7 @@ impl Host { // Notes on metering: free pub(crate) fn usize_to_u32val(&self, u: usize) -> Result { - self.usize_to_u32(u).map(|v| v.into()) + self.usize_to_u32(u).map(Into::into) } // Notes on metering: free @@ -254,7 +254,7 @@ impl Host { pub(crate) fn call_args_from_obj(&self, args: VecObject) -> Result, HostError> { self.visit_obj(args, |hv: &HostVec| { // Metering: free - Ok(hv.iter().cloned().collect()) + Ok(hv.iter().copied().collect()) }) } diff --git a/soroban-env-host/src/host/data_helper.rs b/soroban-env-host/src/host/data_helper.rs index 273d8f728..8ed1e555f 100644 --- a/soroban-env-host/src/host/data_helper.rs +++ b/soroban-env-host/src/host/data_helper.rs @@ -245,9 +245,9 @@ impl Host { // weight. let weight = min(signer.weight, u8::MAX as u32); // We've found the target signer in the account signers, so return the weight - return Ok(weight + return weight .try_into() - .map_err(|_| self.err_general("signer weight overflow"))?); + .map_err(|_| self.err_general("signer weight overflow")); } } } diff --git a/soroban-env-host/src/host/err_helper.rs b/soroban-env-host/src/host/err_helper.rs index cea4cd9ca..3a57d9c8e 100644 --- a/soroban-env-host/src/host/err_helper.rs +++ b/soroban-env-host/src/host/err_helper.rs @@ -55,7 +55,7 @@ impl Host { { let mut e = DebugError::new(status).msg(msg); for arg in args { - e = e.arg(*arg) + e = e.arg(*arg); } self.err(e) } @@ -94,7 +94,7 @@ impl Host { where DebugError: From, { - res.map_err(|e| self.err(e.into())) + res.map_err(|e| self.err(e)) } } diff --git a/soroban-env-host/src/host/error.rs b/soroban-env-host/src/host/error.rs index 37eccc1e8..602810ed5 100644 --- a/soroban-env-host/src/host/error.rs +++ b/soroban-env-host/src/host/error.rs @@ -17,6 +17,8 @@ impl std::error::Error for HostError {} impl Debug for HostError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // TODO: maybe make this something users can adjust? + const MAX_DEBUG_EVENTS: usize = 10; // We do a little trimming here, skipping the first two frames (which // are always into, from, and one or more Host::err_foo calls) and all // the frames _after_ the short-backtrace marker that rust compiles-in. @@ -24,7 +26,7 @@ impl Debug for HostError { fn frame_name_matches(frame: &BacktraceFrame, pat: &str) -> bool { for sym in frame.symbols() { match sym.name() { - Some(sn) if format!("{:}", sn).contains(pat) => { + Some(sn) if format!("{sn:}").contains(pat) => { return true; } _ => (), @@ -56,8 +58,6 @@ impl Debug for HostError { let bt: Backtrace = frames.into(); writeln!(f, "HostError")?; writeln!(f, "Value: {:?}", self.status)?; - // TODO: maybe make this something users can adjust? - const MAX_DEBUG_EVENTS: usize = 10; match &self.events { None => (), Some(ev) => { @@ -74,20 +74,20 @@ impl Debug for HostError { .enumerate() { if !wrote_heading { - writeln!(f, "")?; + writeln!(f)?; writeln!(f, "Debug events (newest first):")?; wrote_heading = true; } - writeln!(f, " {:?}: {:?}", i, e)?; + writeln!(f, " {i:?}: {e:?}")?; } if ev.0.len() > MAX_DEBUG_EVENTS { - writeln!(f, " {:?}: ... elided ...", MAX_DEBUG_EVENTS)?; + writeln!(f, " {MAX_DEBUG_EVENTS:?}: ... elided ...")?; } } } - writeln!(f, "")?; + writeln!(f)?; writeln!(f, "Backtrace (newest first):")?; - writeln!(f, "{:?}", bt) + writeln!(f, "{bt:?}") } } diff --git a/soroban-env-host/src/host/mem_helper.rs b/soroban-env-host/src/host/mem_helper.rs index 932da7282..182e3075c 100644 --- a/soroban-env-host/src/host/mem_helper.rs +++ b/soroban-env-host/src/host/mem_helper.rs @@ -207,7 +207,7 @@ impl Host { )?; for (i, slice) in slices.iter().enumerate() { self.charge_budget(CostType::VmMemRead, slice.len() as u64)?; - callback(i, *slice)?; + callback(i, slice)?; } Ok(()) } diff --git a/soroban-env-host/src/host/metered_clone.rs b/soroban-env-host/src/host/metered_clone.rs index 565fd875f..43f0dfc7f 100644 --- a/soroban-env-host/src/host/metered_clone.rs +++ b/soroban-env-host/src/host/metered_clone.rs @@ -333,6 +333,7 @@ impl MeteredClone for Vec { impl MeteredClone for Box { const IS_SHALLOW: bool = false; + #[allow(clippy::explicit_auto_deref)] fn charge_for_substructure(&self, budget: &Budget) -> Result<(), HostError> { // first take care of the Box clone: allocating memory and shallow cloning of data type. charge_heap_alloc::(1, budget)?; diff --git a/soroban-env-host/src/host/metered_map.rs b/soroban-env-host/src/host/metered_map.rs index 80e0f8c9a..996534490 100644 --- a/soroban-env-host/src/host/metered_map.rs +++ b/soroban-env-host/src/host/metered_map.rs @@ -94,6 +94,7 @@ where }) } + #[allow(clippy::match_same_arms)] pub fn from_map(map: Vec<(K, V)>, ctx: &Ctx) -> Result { // Construction cost already paid for by caller, just check // that input has sorted and unique keys. @@ -247,6 +248,10 @@ where self.map.len() } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + pub fn contains_key(&self, key: &Q, ctx: &Ctx) -> Result where K: Borrow, @@ -388,7 +393,7 @@ where type IntoIter = core::slice::Iter<'a, (K, V)>; fn into_iter(self) -> Self::IntoIter { - (&self.map).into_iter() + self.map.iter() } } diff --git a/soroban-env-host/src/host/metered_vector.rs b/soroban-env-host/src/host/metered_vector.rs index e53d00e33..c943556ac 100644 --- a/soroban-env-host/src/host/metered_vector.rs +++ b/soroban-env-host/src/host/metered_vector.rs @@ -111,13 +111,17 @@ where self.vec.len() } + pub fn is_empty(&self) -> bool { + self.vec.is_empty() + } + pub fn push_front(&self, value: A, budget: &Budget) -> Result { let iter = [value].into_iter().chain(self.vec.iter().cloned()); Self::from_exact_iter(iter, budget) } pub fn pop_front(&self, budget: &Budget) -> Result { - if self.vec.len() == 0 { + if self.is_empty() { Err(ScHostObjErrorCode::VecIndexOutOfBound.into()) } else { let iter = self.vec.iter().skip(1).cloned(); @@ -144,7 +148,7 @@ where } pub fn pop_back(&self, budget: &Budget) -> Result { - if self.vec.len() == 0 { + if self.is_empty() { Err(ScHostObjErrorCode::VecIndexOutOfBound.into()) } else { let count = Self::sub_or_overflow(self.vec.len(), 1)?; @@ -213,14 +217,11 @@ where F: Fn(&A) -> Result, { self.charge_scan(budget)?; - let mut i = 0; - let mut iter = self.vec.iter(); // this is similar logic to `iter.position(f)` but is fallible - while let Some(val) = iter.next() { + for (i, val) in self.vec.iter().enumerate() { if f(val)? == Ordering::Equal { return Ok(Some(i)); } - i += 1; } Ok(None) } diff --git a/soroban-env-host/src/host/metered_xdr.rs b/soroban-env-host/src/host/metered_xdr.rs index 68a173d4a..d7829b1ce 100644 --- a/soroban-env-host/src/host/metered_xdr.rs +++ b/soroban-env-host/src/host/metered_xdr.rs @@ -19,7 +19,7 @@ where fn write(&mut self, buf: &[u8]) -> std::io::Result { self.host .charge_budget(CostType::ValSer, buf.len() as u64) - .map_err(|e| Into::::into(e))?; + .map_err(Into::::into)?; self.w.write(buf) } @@ -34,7 +34,7 @@ impl Host { obj: &impl WriteXdr, w: &mut Vec, ) -> Result<(), HostError> { - let mut w = MeteredWrite { host: &self, w }; + let mut w = MeteredWrite { host: self, w }; obj.write_xdr(&mut w).map_err(|e| { if let Some(e2) = e.source() { if let Some(e3) = (*e2).downcast_ref::() { diff --git a/soroban-env-host/src/host_object.rs b/soroban-env-host/src/host_object.rs index a8968d28e..2238dab75 100644 --- a/soroban-env-host/src/host_object.rs +++ b/soroban-env-host/src/host_object.rs @@ -102,7 +102,7 @@ impl HostObject { | HostObject::Address(_) | HostObject::NonceKey(_) => None, }; - return Ok(res); + Ok(res) } } diff --git a/soroban-env-host/src/native_contract/account_contract.rs b/soroban-env-host/src/native_contract/account_contract.rs index aa27e0df9..b1aa2fdcd 100644 --- a/soroban-env-host/src/native_contract/account_contract.rs +++ b/soroban-env-host/src/native_contract/account_contract.rs @@ -144,7 +144,7 @@ pub(crate) fn check_account_authentication( } host.verify_sig_ed25519( - payload_obj.clone(), + payload_obj, sig.public_key.clone().into(), sig.signature.into(), )?; diff --git a/soroban-env-host/src/native_contract/base_types.rs b/soroban-env-host/src/native_contract/base_types.rs index 1534fdae1..8cbe7a460 100644 --- a/soroban-env-host/src/native_contract/base_types.rs +++ b/soroban-env-host/src/native_contract/base_types.rs @@ -69,18 +69,12 @@ impl From> for Bytes { impl Bytes { pub fn push(&mut self, x: u8) -> Result<(), HostError> { let x32: u32 = x.into(); - self.object = self - .host - .bytes_push(self.object.into(), x32.into())? - .try_into()?; + self.object = self.host.bytes_push(self.object, x32.into())?; Ok(()) } pub fn append(&mut self, other: Bytes) -> Result<(), HostError> { - self.object = self - .host - .bytes_append(self.object.into(), other.object.into())? - .try_into()?; + self.object = self.host.bytes_append(self.object, other.object)?; Ok(()) } @@ -107,7 +101,7 @@ impl TryFromVal for BytesN { fn try_from_val(env: &Host, val: &BytesObject) -> Result { let val = *val; - let len: u32 = env.bytes_len(val.into())?.try_into()?; + let len: u32 = env.bytes_len(val)?.try_into()?; if len == N.try_into() .map_err(|_| env.err_general("bytes buffer overflow"))? @@ -148,9 +142,9 @@ impl TryFromVal> for RawVal { } } -impl Into for BytesN { - fn into(self) -> RawVal { - self.object.into() +impl From> for RawVal { + fn from(val: BytesN) -> Self { + val.object.into() } } @@ -173,7 +167,7 @@ impl BytesN { let mut slice = [0_u8; N]; self.host.charge_budget(CostType::BytesClone, N as u64)?; self.host - .bytes_copy_to_slice(self.object.into(), RawVal::U32_ZERO, &mut slice)?; + .bytes_copy_to_slice(self.object, RawVal::U32_ZERO, &mut slice)?; Ok(slice) } @@ -243,7 +237,7 @@ impl From for MapObject { impl Map { pub fn new(env: &Host) -> Result { - let map = env.map_new()?.try_into()?; + let map = env.map_new()?; Ok(Self { host: env.clone(), object: map, @@ -258,7 +252,7 @@ impl Map { HostError: From<>::Error>, { let k_rv = RawVal::try_from_val(&self.host, k)?; - let v_rv = self.host.map_get(self.object.into(), k_rv)?; + let v_rv = self.host.map_get(self.object, k_rv)?; Ok(V::try_from_val(&self.host, &v_rv)?) } @@ -271,10 +265,7 @@ impl Map { { let k_rv = RawVal::try_from_val(&self.host, k)?; let v_rv = RawVal::try_from_val(&self.host, v)?; - self.object = self - .host - .map_put(self.object.into(), k_rv, v_rv)? - .try_into()?; + self.object = self.host.map_put(self.object, k_rv, v_rv)?; Ok(()) } } @@ -323,9 +314,9 @@ impl TryFromVal for RawVal { } } -impl Into for Vec { - fn into(self) -> RawVal { - self.object.into() +impl From for RawVal { + fn from(val: Vec) -> Self { + val.object.into() } } @@ -341,7 +332,7 @@ impl TryFromVal> for Vec { fn try_from_val(env: &Host, vals: &std::vec::Vec) -> Result { let mut v = Vec::new(env)?; for rv in vals { - v.push_raw(rv.clone())? + v.push_raw(*rv)?; } Ok(v) } @@ -357,7 +348,7 @@ impl TryFromVal> for Vec { impl Vec { pub fn new(env: &Host) -> Result { - let vec: VecObject = env.vec_new(().into())?.try_into()?; + let vec: VecObject = env.vec_new(().into())?; Ok(Self { host: env.clone(), object: vec, @@ -403,7 +394,7 @@ impl TryFromVal for Address { fn try_from_val(env: &Host, obj: &AddressObject) -> Result { Ok(Address { host: env.clone(), - object: obj.clone(), + object: *obj, }) } } diff --git a/soroban-env-host/src/native_contract/testutils.rs b/soroban-env-host/src/native_contract/testutils.rs index 6b2958182..e2944ac50 100644 --- a/soroban-env-host/src/native_contract/testutils.rs +++ b/soroban-env-host/src/native_contract/testutils.rs @@ -21,7 +21,7 @@ impl HostVec { pub(crate) fn from_array(host: &Host, vals: &[RawVal]) -> Result { let mut res = HostVec::new(host)?; for val in vals { - res.push_raw(val.clone())?; + res.push_raw(*val)?; } Ok(res) } @@ -75,9 +75,11 @@ pub(crate) enum TestSigner<'a> { AccountContract(AccountContractSigner<'a>), } +type SignFn<'a> = Box HostVec + 'a>; + pub(crate) struct AccountContractSigner<'a> { pub(crate) id: Hash, - pub(crate) sign: Box HostVec + 'a>, + pub(crate) sign: SignFn<'a>, } pub(crate) struct AccountSigner<'a> { @@ -120,7 +122,7 @@ impl<'a> TestSigner<'a> { let signature_args = match self { TestSigner::AccountInvoker(_) => host_vec![host], TestSigner::Account(account_signer) => { - let mut signatures = HostVec::new(&host).unwrap(); + let mut signatures = HostVec::new(host).unwrap(); for key in &account_signer.signers { signatures .push(&sign_payload_for_account(host, key, payload)) @@ -157,7 +159,7 @@ pub(crate) fn authorize_single_invocation_with_nonce( let address_with_nonce = match signer { TestSigner::AccountInvoker(_) => None, TestSigner::Account(_) | TestSigner::AccountContract(_) => Some(AddressWithNonce { - address: sc_address.clone(), + address: sc_address, nonce: nonce.unwrap(), }), TestSigner::ContractInvoker(_) => { @@ -176,7 +178,7 @@ pub(crate) fn authorize_single_invocation_with_nonce( let signature_payload = if let Some(addr_with_nonce) = &address_with_nonce { let signature_payload_preimage = HashIdPreimage::ContractAuth(HashIdPreimageContractAuth { network_id: host - .with_ledger_info(|li: &LedgerInfo| Ok(li.network_id.clone())) + .with_ledger_info(|li: &LedgerInfo| Ok(li.network_id)) .unwrap() .try_into() .unwrap(), @@ -209,7 +211,7 @@ pub(crate) fn authorize_single_invocation( TestSigner::AccountInvoker(_) => None, TestSigner::Account(_) | TestSigner::AccountContract(_) => Some( host.read_nonce( - &Hash(contract_id.to_vec().clone().try_into().unwrap()), + &Hash(contract_id.to_vec().try_into().unwrap()), &signer.address(host).to_sc_address().unwrap(), ) .unwrap(), @@ -259,6 +261,7 @@ pub(crate) fn sign_payload_for_ed25519( .unwrap() } +#[allow(clippy::too_many_arguments)] pub(crate) fn create_account( host: &Host, account_id: &AccountId, @@ -272,7 +275,7 @@ pub(crate) fn create_account( sponsorships: Option<(u32, u32)>, flags: u32, ) { - let key = host.to_account_key(account_id.clone().into()); + let key = host.to_account_key(account_id.clone()); let account_id = match key.as_ref() { LedgerKey::Account(acc) => acc.account_id.clone(), _ => unreachable!(), diff --git a/soroban-env-host/src/native_contract/token/admin.rs b/soroban-env-host/src/native_contract/token/admin.rs index ab3018a0c..b70536c20 100644 --- a/soroban-env-host/src/native_contract/token/admin.rs +++ b/soroban-env-host/src/native_contract/token/admin.rs @@ -9,7 +9,7 @@ use soroban_env_common::{Compare, Env, TryIntoVal}; fn read_administrator(e: &Host) -> Result { let key = DataKey::Admin; let rv = e.get_contract_data(key.try_into_val(e)?)?; - Ok(rv.try_into_val(e)?) + rv.try_into_val(e) } // Metering: covered by components @@ -22,7 +22,9 @@ pub fn write_administrator(e: &Host, id: Address) -> Result<(), HostError> { // Metering: *mostly* covered by components. pub fn check_admin(e: &Host, addr: &Address) -> Result<(), HostError> { let admin = read_administrator(e)?; - if e.compare(&admin, addr)? != core::cmp::Ordering::Equal { + if e.compare(&admin, addr)? == core::cmp::Ordering::Equal { + Ok(()) + } else { Err(err!( e, ContractError::UnauthorizedError, @@ -30,7 +32,5 @@ pub fn check_admin(e: &Host, addr: &Address) -> Result<(), HostError> { addr.clone(), admin )) - } else { - Ok(()) } } diff --git a/soroban-env-host/src/native_contract/token/balance.rs b/soroban-env-host/src/native_contract/token/balance.rs index 1552ae66e..bb92a2a4d 100644 --- a/soroban-env-host/src/native_contract/token/balance.rs +++ b/soroban-env-host/src/native_contract/token/balance.rs @@ -83,7 +83,7 @@ pub fn receive_balance(e: &Host, addr: Address, amount: i128) -> Result<(), Host BalanceValue { amount: 0, authorized: true, - clawback: is_asset_clawback_enabled(&e)?, + clawback: is_asset_clawback_enabled(e)?, } }; @@ -112,7 +112,7 @@ pub fn spend_balance_no_authorization_check( "spent amount is too large for an i64", ) })?; - transfer_classic_balance(e, acc_id, -(i64_amount as i64)) + transfer_classic_balance(e, acc_id, -i64_amount) } ScAddress::Contract(_) => { // If a balance exists, calculate new amount and write the existing authorized state as is because @@ -128,15 +128,14 @@ pub fn spend_balance_no_authorization_check( balance, amount )); - } else { - let new_balance = balance - .amount - .checked_sub(amount) - .ok_or_else(|| e.err_status(ContractError::OverflowError))?; - balance.amount = new_balance; - - write_balance(e, addr, balance)? } + let new_balance = balance + .amount + .checked_sub(amount) + .ok_or_else(|| e.err_status(ContractError::OverflowError))?; + balance.amount = new_balance; + + write_balance(e, addr, balance)?; } else if amount > 0 { return Err(err!( e, @@ -194,7 +193,7 @@ pub fn write_authorization(e: &Host, addr: Address, authorize: bool) -> Result<( let balance = BalanceValue { amount: 0, authorized: authorize, - clawback: is_asset_clawback_enabled(&e)?, + clawback: is_asset_clawback_enabled(e)?, }; write_balance(e, addr, balance) } @@ -223,12 +222,10 @@ pub fn check_clawbackable(e: &Host, addr: Address) -> Result<(), HostError> { match addr.to_sc_address()? { ScAddress::Account(acc_id) => match read_metadata(e)? { - Metadata::Native => { - return Err(e.err_status_msg( - ContractError::OperationNotSupportedError, - "cannot clawback native asset", - )) - } + Metadata::Native => Err(e.err_status_msg( + ContractError::OperationNotSupportedError, + "cannot clawback native asset", + )), Metadata::AlphaNum4(asset) => { let issuer_account_id = e.account_id_from_bytesobj(asset.issuer.into())?; validate_trustline( @@ -287,7 +284,7 @@ pub fn transfer_classic_balance(e: &Host, to_key: AccountId, amount: i64) -> Res e.create_asset_4(asset.asset_code.to_array()?, issuer_account_id.clone()), issuer_account_id, to_key, - )? + )?; } Metadata::AlphaNum12(asset) => { let issuer_account_id = e.account_id_from_bytesobj(asset.issuer.into())?; @@ -295,7 +292,7 @@ pub fn transfer_classic_balance(e: &Host, to_key: AccountId, amount: i64) -> Res e.create_asset_12(asset.asset_code.to_array()?, issuer_account_id.clone()), issuer_account_id, to_key, - )? + )?; } }; Ok(()) @@ -339,7 +336,7 @@ fn get_classic_balance(e: &Host, to_key: AccountId) -> Result<(i64, i64), HostEr // Metering: *mostly* covered by components. The arithmetics are free. fn transfer_account_balance(e: &Host, account_id: AccountId, amount: i64) -> Result<(), HostError> { - let lk = e.to_account_key(account_id.clone()); + let lk = e.to_account_key(account_id); e.with_mut_storage(|storage| { let mut le = storage @@ -353,9 +350,7 @@ fn transfer_account_balance(e: &Host, account_id: AccountId, amount: i64) -> Res let (min_balance, max_balance) = get_min_max_account_balance(e, &ae)?; - let new_balance = if amount <= 0 { - ae.balance + amount - } else if ae.balance <= i64::MAX - amount { + let new_balance = if amount <= 0 || ae.balance <= i64::MAX - amount { ae.balance + amount } else { return Err(e.err_status_msg(ContractError::BalanceError, "resulting balance overflow")); @@ -397,9 +392,7 @@ fn transfer_trustline_balance( let (min_balance, max_balance) = get_min_max_trustline_balance(e, &tl)?; - let new_balance = if amount <= 0 { - tl.balance + amount - } else if tl.balance <= i64::MAX - amount { + let new_balance = if amount <= 0 || tl.balance <= i64::MAX - amount { tl.balance + amount } else { return Err(e.err_status_msg(ContractError::BalanceError, "resulting balance overflow")); @@ -424,7 +417,7 @@ fn transfer_trustline_balance( // TODO: Metering analysis //returns (total balance, spendable balance) fn get_account_balance(e: &Host, account_id: AccountId) -> Result<(i64, i64), HostError> { - let lk = e.to_account_key(account_id.clone()); + let lk = e.to_account_key(account_id); e.with_mut_storage(|storage| { let le = storage @@ -436,7 +429,7 @@ fn get_account_balance(e: &Host, account_id: AccountId) -> Result<(i64, i64), Ho _ => Err(e.err_status_msg(ContractError::InternalError, "unexpected entry found")), }?; - let min = get_min_max_account_balance(e, &ae)?.0; + let min = get_min_max_account_balance(e, ae)?.0; if ae.balance < min { return Err(e.err_status_msg( ContractError::InternalError, @@ -596,7 +589,7 @@ fn set_authorization(e: &Host, to_key: AccountId, authorize: bool) -> Result<(), )); } - let issuer_acc = e.load_account(issuer.clone())?; + let issuer_acc = e.load_account(issuer)?; if !authorize && (issuer_acc.flags & (AccountFlags::RevocableFlag as u32) == 0) { return Err(e.err_status_msg( @@ -609,12 +602,10 @@ fn set_authorization(e: &Host, to_key: AccountId, authorize: bool) -> Result<(), }; match read_metadata(e)? { - Metadata::Native => { - return Err(e.err_status_msg( - ContractError::OperationNotSupportedError, - "expected trustline asset", - )) - } + Metadata::Native => Err(e.err_status_msg( + ContractError::OperationNotSupportedError, + "expected trustline asset", + )), Metadata::AlphaNum4(asset) => { let issuer_account_id = e.account_id_from_bytesobj(asset.issuer.into())?; set_trustline_authorization_safe( diff --git a/soroban-env-host/src/native_contract/token/contract.rs b/soroban-env-host/src/native_contract/token/contract.rs index 32a00e18e..e01438fb2 100644 --- a/soroban-env-host/src/native_contract/token/contract.rs +++ b/soroban-env-host/src/native_contract/token/contract.rs @@ -106,7 +106,7 @@ fn check_non_native(e: &Host) -> Result<(), HostError> { // Metering: *mostly* covered by components. impl TokenTrait for Token { fn init_asset(e: &Host, asset_bytes: Bytes) -> Result<(), HostError> { - if has_metadata(&e)? { + if has_metadata(e)? { return Err(e.err_status_msg( ContractError::AlreadyInitializedError, "token has been already initialized", @@ -128,13 +128,13 @@ impl TokenTrait for Token { } match asset { Asset::Native => { - write_metadata(&e, Metadata::Native)?; + write_metadata(e, Metadata::Native)?; //No admin for the Native token } Asset::CreditAlphanum4(asset4) => { - write_administrator(&e, Address::from_account(e, &asset4.issuer)?)?; + write_administrator(e, Address::from_account(e, &asset4.issuer)?)?; write_metadata( - &e, + e, Metadata::AlphaNum4(AlphaNum4Metadata { asset_code: BytesN::<4>::try_from_val( e, @@ -148,9 +148,9 @@ impl TokenTrait for Token { )?; } Asset::CreditAlphanum12(asset12) => { - write_administrator(&e, Address::from_account(e, &asset12.issuer)?)?; + write_administrator(e, Address::from_account(e, &asset12.issuer)?)?; write_metadata( - &e, + e, Metadata::AlphaNum12(AlphaNum12Metadata { asset_code: BytesN::<12>::try_from_val( e, @@ -180,11 +180,11 @@ impl TokenTrait for Token { ) -> Result<(), HostError> { check_nonnegative_amount(e, amount)?; from.require_auth()?; - let allowance = read_allowance(&e, from.clone(), spender.clone())?; + let allowance = read_allowance(e, from.clone(), spender.clone())?; let new_allowance = allowance .checked_add(amount) .ok_or_else(|| e.err_status(ContractError::OverflowError))?; - write_allowance(&e, from.clone(), spender.clone(), new_allowance)?; + write_allowance(e, from.clone(), spender.clone(), new_allowance)?; event::incr_allow(e, from, spender, amount)?; Ok(()) } @@ -197,11 +197,11 @@ impl TokenTrait for Token { ) -> Result<(), HostError> { check_nonnegative_amount(e, amount)?; from.require_auth()?; - let allowance = read_allowance(&e, from.clone(), spender.clone())?; + let allowance = read_allowance(e, from.clone(), spender.clone())?; if amount >= allowance { - write_allowance(&e, from.clone(), spender.clone(), 0)?; + write_allowance(e, from.clone(), spender.clone(), 0)?; } else { - write_allowance(&e, from.clone(), spender.clone(), allowance - amount)?; + write_allowance(e, from.clone(), spender.clone(), allowance - amount)?; } event::decr_allow(e, from, spender, amount)?; Ok(()) @@ -218,7 +218,7 @@ impl TokenTrait for Token { // Metering: covered by components fn authorized(e: &Host, addr: Address) -> Result { - is_authorized(&e, addr) + is_authorized(e, addr) } // Metering: covered by components @@ -253,7 +253,7 @@ impl TokenTrait for Token { check_nonnegative_amount(e, amount)?; check_non_native(e)?; from.require_auth()?; - spend_balance(&e, from.clone(), amount)?; + spend_balance(e, from.clone(), amount)?; event::burn(e, from, amount)?; Ok(()) } @@ -263,8 +263,8 @@ impl TokenTrait for Token { check_nonnegative_amount(e, amount)?; check_non_native(e)?; spender.require_auth()?; - spend_allowance(&e, from.clone(), spender, amount)?; - spend_balance(&e, from.clone(), amount)?; + spend_allowance(e, from.clone(), spender, amount)?; + spend_balance(e, from.clone(), amount)?; event::burn(e, from, amount)?; Ok(()) } @@ -273,9 +273,9 @@ impl TokenTrait for Token { fn clawback(e: &Host, admin: Address, from: Address, amount: i128) -> Result<(), HostError> { check_nonnegative_amount(e, amount)?; check_admin(e, &admin)?; - check_clawbackable(&e, from.clone())?; + check_clawbackable(e, from.clone())?; admin.require_auth()?; - spend_balance_no_authorization_check(e, from.clone(), amount.clone())?; + spend_balance_no_authorization_check(e, from.clone(), amount)?; event::clawback(e, admin, from, amount)?; Ok(()) } @@ -313,10 +313,10 @@ impl TokenTrait for Token { } fn name(e: &Host) -> Result { - read_name(&e) + read_name(e) } fn symbol(e: &Host) -> Result { - read_symbol(&e) + read_symbol(e) } } diff --git a/soroban-env-host/src/native_contract/token/event.rs b/soroban-env-host/src/native_contract/token/event.rs index 6cf1dde88..6b39979ce 100644 --- a/soroban-env-host/src/native_contract/token/event.rs +++ b/soroban-env-host/src/native_contract/token/event.rs @@ -15,7 +15,7 @@ pub(crate) fn incr_allow( topics.push(&Symbol::try_from_val(e, &"incr_allow")?)?; topics.push(&from)?; topics.push(&to)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), amount.try_into_val(e)?)?; Ok(()) } @@ -30,7 +30,7 @@ pub(crate) fn decr_allow( topics.push(&Symbol::try_from_val(e, &"decr_allow")?)?; topics.push(&from)?; topics.push(&to)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), amount.try_into_val(e)?)?; Ok(()) } @@ -45,7 +45,7 @@ pub(crate) fn transfer( topics.push(&Symbol::try_from_val(e, &"transfer")?)?; topics.push(&from)?; topics.push(&to)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), amount.try_into_val(e)?)?; Ok(()) } @@ -55,7 +55,7 @@ pub(crate) fn mint(e: &Host, admin: Address, to: Address, amount: i128) -> Resul topics.push(&Symbol::try_from_val(e, &"mint")?)?; topics.push(&admin)?; topics.push(&to)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), amount.try_into_val(e)?)?; Ok(()) } @@ -70,7 +70,7 @@ pub(crate) fn clawback( topics.push(&Symbol::try_from_val(e, &"clawback")?)?; topics.push(&admin)?; topics.push(&from)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), amount.try_into_val(e)?)?; Ok(()) } @@ -85,7 +85,7 @@ pub(crate) fn set_auth( topics.push(&Symbol::try_from_val(e, &"set_auth")?)?; topics.push(&admin)?; topics.push(&id)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), authorize.try_into_val(e)?)?; Ok(()) } @@ -94,7 +94,7 @@ pub(crate) fn set_admin(e: &Host, admin: Address, new_admin: Address) -> Result< let mut topics = Vec::new(e)?; topics.push(&Symbol::try_from_val(e, &"set_admin")?)?; topics.push(&admin)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), new_admin.try_into_val(e)?)?; Ok(()) } @@ -103,7 +103,7 @@ pub(crate) fn burn(e: &Host, from: Address, amount: i128) -> Result<(), HostErro let mut topics = Vec::new(e)?; topics.push(&Symbol::try_from_val(e, &"burn")?)?; topics.push(&from)?; - topics.push(&read_name(&e)?)?; + topics.push(&read_name(e)?)?; e.contract_event(topics.into(), amount.try_into_val(e)?)?; Ok(()) } diff --git a/soroban-env-host/src/native_contract/token/metadata.rs b/soroban-env-host/src/native_contract/token/metadata.rs index 67bda8bf3..6e6381e5f 100644 --- a/soroban-env-host/src/native_contract/token/metadata.rs +++ b/soroban-env-host/src/native_contract/token/metadata.rs @@ -15,7 +15,7 @@ pub fn write_metadata(e: &Host, metadata: Metadata) -> Result<(), HostError> { pub fn read_metadata(e: &Host) -> Result { let key = DataKey::Metadata; let rv = e.get_contract_data(key.try_into_val(e)?)?; - Ok(rv.try_into_val(e)?) + rv.try_into_val(e) } // Metering: *mostly* covered by components. diff --git a/soroban-env-host/src/native_contract/token/test_token.rs b/soroban-env-host/src/native_contract/token/test_token.rs index e64e84071..e7fa3cc61 100644 --- a/soroban-env-host/src/native_contract/token/test_token.rs +++ b/soroban-env-host/src/native_contract/token/test_token.rs @@ -233,24 +233,22 @@ impl<'a> TestToken<'a> { } pub(crate) fn name(&self) -> Result { - Ok(self - .host + self.host .call( self.id.clone().into(), Symbol::try_from_val(self.host, &"name")?, host_vec![self.host].into(), )? - .try_into_val(self.host)?) + .try_into_val(self.host) } pub(crate) fn symbol(&self) -> Result { - Ok(self - .host + self.host .call( self.id.clone().into(), Symbol::try_from_val(self.host, &"symbol")?, host_vec![self.host].into(), )? - .try_into_val(self.host)?) + .try_into_val(self.host) } } diff --git a/soroban-synth-wasm/src/func_emitter.rs b/soroban-synth-wasm/src/func_emitter.rs index c1858c0f8..0b43ce126 100644 --- a/soroban-synth-wasm/src/func_emitter.rs +++ b/soroban-synth-wasm/src/func_emitter.rs @@ -134,9 +134,7 @@ impl FuncEmitter { *is_first_arg = false; let insn = match op.into() { Operand::StackTop => { - if !first { - panic!("can only use Operand::StackTop as first arg"); - } + assert!(first, "can only use Operand::StackTop as first arg"); return self; } Operand::Local(loc) => Instruction::LocalGet(loc.0), diff --git a/soroban-synth-wasm/src/mod_emitter.rs b/soroban-synth-wasm/src/mod_emitter.rs index fb162c431..975632119 100644 --- a/soroban-synth-wasm/src/mod_emitter.rs +++ b/soroban-synth-wasm/src/mod_emitter.rs @@ -56,6 +56,12 @@ pub struct ModEmitter { import_refs: HashMap<(String, String, Arity), FuncRef>, } +impl Default for ModEmitter { + fn default() -> Self { + Self::new() + } +} + impl ModEmitter { pub fn new() -> Self { let mut module = Module::new(); @@ -121,6 +127,7 @@ impl ModEmitter { /// Return the unique [`TypeRef`] for a function with a given [`Arity`], /// creating such a type in the `type` section of the module if such a type /// does not already exist. + #[allow(clippy::map_entry)] pub fn get_fn_type(&mut self, arity: Arity) -> TypeRef { if self.type_refs.contains_key(&arity) { self.type_refs[&arity] @@ -138,10 +145,12 @@ impl ModEmitter { /// Return the unique [`FuncRef`] for a function import with a given module /// name, function name, and arity, creating such an import in the `import` /// section of the module if it does not already exist. + #[allow(clippy::map_entry)] pub fn import_func(&mut self, module: &str, fname: &str, arity: Arity) -> FuncRef { - if self.funcs.len() != 0 { - panic!("must import all functions before defining any exports"); - } + assert!( + self.funcs.is_empty(), + "must import all functions before defining any exports" + ); let key = (module.to_owned(), fname.to_owned(), arity); if self.import_refs.contains_key(&key) { self.import_refs[&key] From 058b2e5d90a11acf5eef8eef125d7285e76de2d3 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 13:34:11 -0400 Subject: [PATCH 06/19] chore: potentially valid --- soroban-env-host/src/host.rs | 2 +- soroban-env-host/src/storage.rs | 25 ++++++++----------------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 1072a361a..a75020139 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -765,7 +765,7 @@ impl Host { } } - pub(crate) fn to_host_obj<'a>(&self, ob: &ScValObjRef<'a>) -> Result { + pub(crate) fn to_host_obj(&self, ob: &ScValObjRef) -> Result { self.charge_budget(CostType::ValXdrConv, 1)?; let val: &ScVal = (*ob).into(); match val { diff --git a/soroban-env-host/src/storage.rs b/soroban-env-host/src/storage.rs index fcdf6a9b9..8963af0e3 100644 --- a/soroban-env-host/src/storage.rs +++ b/soroban-env-host/src/storage.rs @@ -76,15 +76,13 @@ impl Footprint { ) -> Result<(), HostError> { if let Some(existing) = self.0.get::>(key, budget)? { match (existing, ty.clone()) { - (AccessType::ReadOnly, AccessType::ReadOnly) => Ok(()), (AccessType::ReadOnly, AccessType::ReadWrite) => { // The only interesting case is an upgrade // from previously-read-only to read-write. self.0 = self.0.insert(Rc::clone(key), ty, budget)?; Ok(()) } - (AccessType::ReadWrite, AccessType::ReadOnly) => Ok(()), - (AccessType::ReadWrite, AccessType::ReadWrite) => Ok(()), + _ => Ok(()), } } else { self.0 = self.0.insert(Rc::clone(key), ty, budget)?; @@ -100,12 +98,10 @@ impl Footprint { ) -> Result<(), HostError> { if let Some(existing) = self.0.get::>(key, budget)? { match (existing, ty) { - (AccessType::ReadOnly, AccessType::ReadOnly) => Ok(()), (AccessType::ReadOnly, AccessType::ReadWrite) => { Err(ScHostStorageErrorCode::ReadwriteAccessToReadonlyEntry.into()) } - (AccessType::ReadWrite, AccessType::ReadOnly) => Ok(()), - (AccessType::ReadWrite, AccessType::ReadWrite) => Ok(()), + _ => Ok(()), } } else { Err(ScHostStorageErrorCode::AccessToUnknownEntry.into()) @@ -113,18 +109,13 @@ impl Footprint { } } -#[derive(Clone)] +#[derive(Clone, Default)] pub enum FootprintMode { Recording(Rc), + #[default] Enforcing, } -impl Default for FootprintMode { - fn default() -> Self { - FootprintMode::Enforcing - } -} - /// A special-purpose map from [LedgerKey]s to [LedgerEntry]s. Represents a /// transactional batch of contract IO from and to durable storage, while /// partitioning that IO between concurrently executing groups of contracts @@ -204,7 +195,7 @@ impl Storage { match self.map.get::>(key, budget)? { None => Err(ScHostStorageErrorCode::MissingKeyInGet.into()), Some(None) => Err(ScHostStorageErrorCode::GetOnDeletedKey.into()), - Some(Some(val)) => Ok(Rc::clone(&val)), + Some(Some(val)) => Ok(Rc::clone(val)), } } @@ -225,7 +216,7 @@ impl Storage { }; self.map = self .map - .insert(Rc::clone(key), val.map(|v| Rc::clone(v)), budget)?; + .insert(Rc::clone(key), val.map(Rc::clone), budget)?; Ok(()) } @@ -312,7 +303,7 @@ mod test_footprint { key: ScVal::I32(0), })); fp.record_access(&key, AccessType::ReadOnly, &budget)?; - assert_eq!(fp.0.contains_key::(&key, &budget)?, true); + assert!(fp.0.contains_key::(&key, &budget)?); assert_eq!( fp.0.get::(&key, &budget)?, Some(&AccessType::ReadOnly) @@ -405,7 +396,7 @@ pub(crate) mod test_storage { impl SnapshotSource for MockSnapshotSource { fn get(&self, key: &Rc) -> Result, HostError> { if let Some(val) = self.0.get(key) { - Ok(Rc::clone(&val)) + Ok(Rc::clone(val)) } else { Err(ScUnknownErrorCode::General.into()) } From 035ea9cbd5825695f340b13a31602685be31572b Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 13:42:46 -0400 Subject: [PATCH 07/19] chore: rebase on main --- soroban-env-common/src/env.rs | 1 - soroban-env-host/src/events/internal.rs | 2 +- soroban-env-host/src/host.rs | 20 +++++++------------ .../src/host/diagnostics_helper.rs | 6 ++---- soroban-env-host/src/host/error.rs | 6 +++--- soroban-env-host/src/test/event.rs | 2 +- 6 files changed, 14 insertions(+), 23 deletions(-) diff --git a/soroban-env-common/src/env.rs b/soroban-env-common/src/env.rs index 381ce8bea..887d31905 100644 --- a/soroban-env-common/src/env.rs +++ b/soroban-env-common/src/env.rs @@ -46,7 +46,6 @@ pub trait EnvBase: Sized + Clone { /// Used to check two environments are the same, trapping if not. fn check_same_env(&self, other: &Self); - /// Used to clone an environment deeply, not just a handle to it. #[allow(clippy::return_self_not_must_use)] fn deep_clone(&self) -> Self; diff --git a/soroban-env-host/src/events/internal.rs b/soroban-env-host/src/events/internal.rs index 0ef8cf578..306f2b130 100644 --- a/soroban-env-host/src/events/internal.rs +++ b/soroban-env-host/src/events/internal.rs @@ -84,7 +84,7 @@ impl InternalEventsBuffer { pub fn dump_to_debug_log(&self) { use log::debug; debug!("=======Start of events======="); - for e in self.vec.iter() { + for e in &self.vec { match &e.0 { InternalEvent::Contract(c) => debug!("Contract event: {:?}", c), InternalEvent::Debug(d) => debug!("Debug event: {}", d), diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index a75020139..b733d5070 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -142,18 +142,13 @@ pub struct LedgerInfo { pub base_reserve: u32, } -#[derive(Clone)] +#[derive(Clone, Default)] pub enum DiagnosticLevel { + #[default] None, Debug, } -impl Default for DiagnosticLevel { - fn default() -> Self { - DiagnosticLevel::None - } -} - #[derive(Clone, Default)] pub(crate) struct HostImpl { source_account: RefCell>, @@ -1026,7 +1021,7 @@ impl Host { } } - self.fn_call_diagnostics(&id, &func, args)?; + self.fn_call_diagnostics(id, &func, args)?; // "testutils" is not covered by budget metering. #[cfg(any(test, feature = "testutils"))] @@ -1110,14 +1105,13 @@ impl Host { } } - let res = self.call_contract_fn(&id, &func, args); + let res = self.call_contract_fn(id, &func, args); - match &res { - Ok(res) => self.fn_return_diagnostics(id, &func, &res)?, - Err(err) => {} + if let Ok(res) = &res { + self.fn_return_diagnostics(id, &func, res)?; } - return res; + res } // Notes on metering: covered by the called components. diff --git a/soroban-env-host/src/host/diagnostics_helper.rs b/soroban-env-host/src/host/diagnostics_helper.rs index 87432430f..59a3f17af 100644 --- a/soroban-env-host/src/host/diagnostics_helper.rs +++ b/soroban-env-host/src/host/diagnostics_helper.rs @@ -76,8 +76,7 @@ impl Host { self.record_system_debug_contract_event( ContractEventType::Diagnostic, calling_contract, - self.add_host_object(HostVec::from_vec(topics)?)? - .try_into()?, + self.add_host_object(HostVec::from_vec(topics)?)?, self.vec_new_from_slice(args)?.into(), ) }) @@ -102,8 +101,7 @@ impl Host { self.record_system_debug_contract_event( ContractEventType::Diagnostic, Some(self.hash_to_bytesobj(contract_id)?), - self.add_host_object(HostVec::from_vec(topics)?)? - .try_into()?, + self.add_host_object(HostVec::from_vec(topics)?)?, *res, ) }) diff --git a/soroban-env-host/src/host/error.rs b/soroban-env-host/src/host/error.rs index 602810ed5..d8ef4d5bc 100644 --- a/soroban-env-host/src/host/error.rs +++ b/soroban-env-host/src/host/error.rs @@ -66,9 +66,9 @@ impl Debug for HostError { ev.0.iter() .rev() .filter_map(|ev| match ev.event.clone() { - Event::Debug(e) => Some(format!("Debug {:}", e)), - Event::StructuredDebug(e) => Some(format!("StructuredDebug {:?}", e)), - _ => None, + Event::Debug(e) => Some(format!("Debug {e:}")), + Event::StructuredDebug(e) => Some(format!("StructuredDebug {e:?}")), + Event::Contract(_) => None, }) .take(MAX_DEBUG_EVENTS) .enumerate() diff --git a/soroban-env-host/src/test/event.rs b/soroban-env-host/src/test/event.rs index 816ba1bde..e57844095 100644 --- a/soroban-env-host/src/test/event.rs +++ b/soroban-env-host/src/test/event.rs @@ -98,7 +98,7 @@ fn test_event_rollback() -> Result<(), HostError> { let args = host.test_vec_obj::(&[1, 2])?; host.register_test_contract(id, test_contract)?; assert_eq!( - host.call(id, sym.into(), args.into())?.get_payload(), + host.call(id, sym, args)?.get_payload(), RawVal::from_void().to_raw().get_payload() ); host.0.events.borrow_mut().rollback(1)?; From 3ddac75cb840c3ffe25cce94e4e2e6afe25fd6f6 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 16:22:39 -0400 Subject: [PATCH 08/19] fix(CI): remove extra `--` --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b09ab2b18..773765f51 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -64,7 +64,7 @@ jobs: with: name: cargo-hack version: 0.5.16 - - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator + - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator - if: matrix.sys.test run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }} From fe46da808730084aac8b32bd8a554bd778892c92 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 15 Mar 2023 16:52:31 -0400 Subject: [PATCH 09/19] fix: add `-Aclippy::unnecessary_wraps` --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 773765f51..0da6afe16 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -64,7 +64,7 @@ jobs: with: name: cargo-hack version: 0.5.16 - - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator + - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator -Aclippy::unnecessary_wraps - if: matrix.sys.test run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }} From 65f9e2a9d92fd8f5a8b93f94bf38c509d55437f5 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 16 Mar 2023 09:10:53 -0400 Subject: [PATCH 10/19] chore: PR comments --- soroban-env-host/src/auth.rs | 10 +++++----- soroban-synth-wasm/src/mod_emitter.rs | 7 +------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/soroban-env-host/src/auth.rs b/soroban-env-host/src/auth.rs index bd1aee179..a461b296b 100644 --- a/soroban-env-host/src/auth.rs +++ b/soroban-env-host/src/auth.rs @@ -350,11 +350,6 @@ impl AuthorizationManager { .get(&address_obj_handle) { let tracker = &mut self.trackers[*tracker_id]; - // The recording invariant is that trackers are created - // with the first authorized invocation, which means - // that when their stack no longer has authorized - // invocation, then we've popped frames past its root - // and hence need to create a new tracker. if tracker.has_authorized_invocations_in_stack() { return self.trackers[*tracker_id].record_invocation( host, @@ -363,6 +358,11 @@ impl AuthorizationManager { args, ); } + // The recording invariant is that trackers are created + // with the first authorized invocation, which means + // that when their stack no longer has authorized + // invocation, then we've popped frames past its root + // and hence need to create a new tracker. recording_info .tracker_by_address_handle .remove(&address_obj_handle); diff --git a/soroban-synth-wasm/src/mod_emitter.rs b/soroban-synth-wasm/src/mod_emitter.rs index 975632119..afdd574c0 100644 --- a/soroban-synth-wasm/src/mod_emitter.rs +++ b/soroban-synth-wasm/src/mod_emitter.rs @@ -1,3 +1,4 @@ +#![allow(clippy::new_without_default)] use crate::FuncEmitter; use std::collections::HashMap; use wasm_encoder::{ @@ -56,12 +57,6 @@ pub struct ModEmitter { import_refs: HashMap<(String, String, Arity), FuncRef>, } -impl Default for ModEmitter { - fn default() -> Self { - Self::new() - } -} - impl ModEmitter { pub fn new() -> Self { let mut module = Module::new(); From bee67806265a895664ef285a2d70f53a20687914 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 16 Mar 2023 09:11:05 -0400 Subject: [PATCH 11/19] chore: clippy all targets --- soroban-env-host/src/budget.rs | 2 +- .../cost_types/record_contract_event.rs | 2 +- .../src/cost_runner/cost_types/val_ser.rs | 2 +- soroban-env-host/src/cost_runner/runner.rs | 2 +- soroban-env-host/src/host.rs | 8 +-- soroban-env-host/src/test/address.rs | 4 +- soroban-env-host/src/test/bytes.rs | 6 +- soroban-env-host/src/test/crypto.rs | 5 +- soroban-env-host/src/test/event.rs | 25 +++---- soroban-env-host/src/test/ledger.rs | 2 +- soroban-env-host/src/test/map.rs | 18 ++--- soroban-env-host/src/test/token.rs | 71 ++++++++++--------- soroban-env-host/src/test/tuple.rs | 5 +- soroban-env-host/src/test/util.rs | 2 +- soroban-env-host/tests/integration.rs | 4 +- 15 files changed, 74 insertions(+), 84 deletions(-) diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 2e94831ea..1a44eed12 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -308,7 +308,7 @@ impl BudgetDimension { #[cfg(test)] pub fn reset_models(&mut self) { for model in &mut self.cost_models { - model.reset() + model.reset(); } } } diff --git a/soroban-env-host/src/cost_runner/cost_types/record_contract_event.rs b/soroban-env-host/src/cost_runner/cost_types/record_contract_event.rs index 03f052ab4..3d4fb2286 100644 --- a/soroban-env-host/src/cost_runner/cost_types/record_contract_event.rs +++ b/soroban-env-host/src/cost_runner/cost_types/record_contract_event.rs @@ -17,7 +17,7 @@ impl CostRunner for RecordContractEventRun { fn run_iter(host: &crate::Host, _iter: u64, sample: Self::SampleType) { host.record_contract_event(ContractEventType::Contract, sample.topics, sample.data) - .expect("contract event") + .expect("contract event"); } fn get_total_input(_host: &crate::Host, sample: &Self::SampleType) -> u64 { diff --git a/soroban-env-host/src/cost_runner/cost_types/val_ser.rs b/soroban-env-host/src/cost_runner/cost_types/val_ser.rs index 850971a0f..6ec155edc 100644 --- a/soroban-env-host/src/cost_runner/cost_types/val_ser.rs +++ b/soroban-env-host/src/cost_runner/cost_types/val_ser.rs @@ -8,6 +8,6 @@ impl CostRunner for ValSerRun { fn run_iter(host: &crate::Host, _iter: u64, sample: Self::SampleType) { let mut buf = Vec::::new(); - host.metered_write_xdr(&sample, &mut buf).unwrap() + host.metered_write_xdr(&sample, &mut buf).unwrap(); } } diff --git a/soroban-env-host/src/cost_runner/runner.rs b/soroban-env-host/src/cost_runner/runner.rs index 11374c771..2ceb0c08f 100644 --- a/soroban-env-host/src/cost_runner/runner.rs +++ b/soroban-env-host/src/cost_runner/runner.rs @@ -22,7 +22,7 @@ pub trait CostRunner: Sized { /// `run_iter` with iter set to each number in 0..RUN_ITERATIONS. fn run(host: &Host, samples: Vec) { for (iter, sample) in samples.into_iter().enumerate() { - Self::run_iter(host, iter as u64, sample) + Self::run_iter(host, iter as u64, sample); } } diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index b733d5070..16b6501de 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -1091,10 +1091,10 @@ impl Host { // the . else if cfg!(feature = "hostfn_log_fmt_values") { if let Some(str) = panic_payload.downcast_ref::<&str>() { - let msg: String = format!("caught panic '{}' from contract function '{:?}'", str, func); + let msg: String = format!("caught panic '{str}' from contract function '{func:?}'"); event = DebugEvent::new().msg(msg); } else if let Some(str) = panic_payload.downcast_ref::() { - let msg: String = format!("caught panic '{}' from contract function '{:?}'", str, func); + let msg: String = format!("caught panic '{str}' from contract function '{func:?}'"); event = DebugEvent::new().msg(msg); } } @@ -3020,9 +3020,9 @@ mod testutils { set_hook(Box::new(move |info| { let calling_test_contract = TEST_CONTRACT_CALL_COUNT.with(|c| c.get() != 0); if !calling_test_contract { - existing_panic_hook(info) + existing_panic_hook(info); } - })) + })); }); TEST_CONTRACT_CALL_COUNT.with(|c| { diff --git a/soroban-env-host/src/test/address.rs b/soroban-env-host/src/test/address.rs index 396ddd61d..1eba8db17 100644 --- a/soroban-env-host/src/test/address.rs +++ b/soroban-env-host/src/test/address.rs @@ -16,7 +16,7 @@ fn test_account_address_conversions() { host.visit_obj(address_obj, |addr: &ScAddress| { Ok(addr.clone()) }) .unwrap(), ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256( - account_pk.clone() + account_pk )))) ); let restored_pk_obj = host @@ -50,7 +50,7 @@ fn test_contract_address_conversions() { assert_eq!( host.visit_obj(address_obj, |addr: &ScAddress| { Ok(addr.clone()) }) .unwrap(), - ScAddress::Contract(Hash(contract_id.clone())) + ScAddress::Contract(Hash(contract_id)) ); let restored_contract_id_obj = host .address_to_contract_id(address_obj) diff --git a/soroban-env-host/src/test/bytes.rs b/soroban-env-host/src/test/bytes.rs index bc8884b67..e467c312b 100644 --- a/soroban-env-host/src/test/bytes.rs +++ b/soroban-env-host/src/test/bytes.rs @@ -99,8 +99,8 @@ fn bytes_slice_start_greater_than_len() -> Result<(), HostError> { fn bytes_xdr_roundtrip() -> Result<(), HostError> { let host = Host::default(); let roundtrip = |v: ScVal| -> Result<(), HostError> { - let rv: RawVal = host.to_host_val(&v)?.into(); - let bo = host.serialize_to_bytes(rv.clone())?; + let rv: RawVal = host.to_host_val(&v)?; + let bo = host.serialize_to_bytes(rv)?; let rv_back = host.deserialize_from_bytes(bo)?; assert_eq!(host.compare(&rv, &rv_back)?, core::cmp::Ordering::Equal); Ok(()) @@ -117,7 +117,7 @@ fn bytes_xdr_roundtrip() -> Result<(), HostError> { { // vec let scval = ScVal::Vec(Some(host.test_scvec::(&[1, 2])?)); - roundtrip(scval)? + roundtrip(scval)?; // TODO: add other types } // Symbol diff --git a/soroban-env-host/src/test/crypto.rs b/soroban-env-host/src/test/crypto.rs index f7738e53d..dba57f007 100644 --- a/soroban-env-host/src/test/crypto.rs +++ b/soroban-env-host/src/test/crypto.rs @@ -55,9 +55,6 @@ fn ed25519_verify_test() -> Result<(), HostError> { let res_failed = host.verify_sig_ed25519(obj_msg2, obj_pub, obj_sig); - match res_failed { - Ok(_) => panic!("verification test failed"), - _ => (), - }; + assert!(res_failed.is_err(), "verification test failed"); Ok(()) } diff --git a/soroban-env-host/src/test/event.rs b/soroban-env-host/src/test/event.rs index e57844095..5fc09c585 100644 --- a/soroban-env-host/src/test/event.rs +++ b/soroban-env-host/src/test/event.rs @@ -33,7 +33,7 @@ fn contract_event() -> Result<(), HostError> { let args = host.test_vec_obj::(&[1, 2])?; host.register_test_contract(id, test_contract)?; assert_eq!( - host.call(id, sym.into(), args.into())?.get_payload(), + host.call(id, sym, args)?.get_payload(), RawVal::from_void().to_raw().get_payload() ); @@ -53,21 +53,14 @@ fn contract_event() -> Result<(), HostError> { // Fish out the last contract event and check that it is // correct, and formats as expected. let events = host.get_events()?.0; - match events.last() { - Some(HostEvent { - event, - failed_call: _, - }) => match event { - Event::Contract(ce) => { - assert_eq!(*ce, event_ref) - } - _ => { - panic!("missing contract event") - } - }, - _ => { - panic!("missing contract event") - } + if let Some(HostEvent { + event: Event::Contract(ce), + failed_call: _, + }) = events.last() + { + assert_eq!(*ce, event_ref); + } else { + panic!("missing contract event"); }; Ok(()) } diff --git a/soroban-env-host/src/test/ledger.rs b/soroban-env-host/src/test/ledger.rs index aed1cfd3e..d0282b6f0 100644 --- a/soroban-env-host/src/test/ledger.rs +++ b/soroban-env-host/src/test/ledger.rs @@ -12,7 +12,7 @@ fn ledger_network_id() -> Result<(), HostError> { let storage = Storage::with_enforcing_footprint_and_map(Footprint::default(), StorageMap::new(&budget)?); - let host = Host::with_storage_and_budget(storage, budget.clone()); + let host = Host::with_storage_and_budget(storage, budget); host.set_ledger_info(LedgerInfo { protocol_version: 0, sequence_number: 0, diff --git a/soroban-env-host/src/test/map.rs b/soroban-env-host/src/test/map.rs index 6e2e4184d..49d7de740 100644 --- a/soroban-env-host/src/test/map.rs +++ b/soroban-env-host/src/test/map.rs @@ -185,9 +185,9 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> { let mut test_map = host.map_new()?; test_map = host.map_put(test_map, 2u32.into(), 4u32.into())?; - test_map = host.map_put(test_map, obj_map.clone().into(), 4u32.into())?; - test_map = host.map_put(test_map, obj_vec.clone().into(), 4u32.into())?; - test_map = host.map_put(test_map, sym.clone().into(), 4u32.into())?; + test_map = host.map_put(test_map, obj_map, 4u32.into())?; + test_map = host.map_put(test_map, obj_vec, 4u32.into())?; + test_map = host.map_put(test_map, sym.into(), 4u32.into())?; // The key ordering should be [u32, symbol, vec, map] // prev { @@ -200,17 +200,17 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> { RawVal::from_u32(2).to_raw().get_payload() ); assert_eq!( - host.map_prev_key(test_map, sym.clone().into())? + host.map_prev_key(test_map, sym.into())? .get_payload(), RawVal::from_u32(2).to_raw().get_payload() ); assert_eq!( - host.map_prev_key(test_map, obj_vec.clone().into())? + host.map_prev_key(test_map, obj_vec)? .get_payload(), sym.as_raw().get_payload() ); assert_eq!( - host.map_prev_key(test_map, obj_map.clone().into())? + host.map_prev_key(test_map, obj_map)? .get_payload(), obj_vec.get_payload() ); @@ -226,17 +226,17 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> { sym.as_raw().get_payload() ); assert_eq!( - host.map_next_key(test_map, sym.clone().into())? + host.map_next_key(test_map, sym.into())? .get_payload(), obj_vec.get_payload() ); assert_eq!( - host.map_next_key(test_map, obj_vec.clone().into())? + host.map_next_key(test_map, obj_vec)? .get_payload(), obj_map.get_payload() ); assert_eq!( - host.map_next_key(test_map, obj_map.clone().into())? + host.map_next_key(test_map, obj_map)? .get_payload(), Status::UNKNOWN_ERROR.to_raw().get_payload() ); diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs index a9aad005d..dea6707c8 100644 --- a/soroban-env-host/src/test/token.rs +++ b/soroban-env-host/src/test/token.rs @@ -1,3 +1,4 @@ +#![allow(clippy::too_many_arguments)] use std::{convert::TryInto, rc::Rc}; use crate::{ @@ -48,7 +49,7 @@ impl TokenTest { host.set_ledger_info(LedgerInfo { protocol_version: 20, sequence_number: 123, - timestamp: 123456, + timestamp: 123_456, network_id: [5; 32], base_reserve: 5_000_000, }); @@ -85,7 +86,7 @@ impl TokenTest { let asset = Asset::CreditAlphanum4(AlphaNum4 { asset_code: AssetCode4(self.asset_code), - issuer: issuer_id.clone(), + issuer: issuer_id, }); TestToken::new_from_asset(&self.host, asset) @@ -130,7 +131,7 @@ impl TokenTest { fn get_trustline_balance(&self, key: &Rc) -> i64 { self.host .with_mut_storage(|s| match &s.get(key, self.host.as_budget()).unwrap().data { - LedgerEntryData::Trustline(trustline) => Ok(trustline.balance.clone()), + LedgerEntryData::Trustline(trustline) => Ok(trustline.balance), _ => unreachable!(), }) .unwrap() @@ -368,14 +369,14 @@ fn test_asset_init(asset_code: &[u8]) { code.clone_from_slice(asset_code); Asset::CreditAlphanum4(AlphaNum4 { asset_code: AssetCode4(code), - issuer: issuer_id.clone(), + issuer: issuer_id, }) } else { let mut code = [0_u8; 12]; code.clone_from_slice(asset_code); Asset::CreditAlphanum12(AlphaNum12 { asset_code: AssetCode12(code), - issuer: issuer_id.clone(), + issuer: issuer_id, }) }; let token = TestToken::new_from_asset(&test.host, asset.clone()); @@ -392,7 +393,7 @@ fn test_asset_init(asset_code: &[u8]) { test.issuer_key.public.to_bytes().to_vec() ] .concat() - .to_vec() + ); let user = TestSigner::account_with_multisig(&account_id, vec![&test.user_key]); @@ -403,7 +404,7 @@ fn test_asset_init(asset_code: &[u8]) { #[test] fn test_asset4_smart_init() { - test_asset_init(&[0, 'a' as u8, 'b' as u8, 255]); + test_asset_init(&[0, b'a', b'b', 255]); } #[test] @@ -421,7 +422,7 @@ fn test_zero_amounts() { let user_2 = TestSigner::account(&test.user_key_2); let user_contract_id = generate_bytes_array(); - let user_contract_address = contract_id_to_address(&test.host, user_contract_id.clone()); + let user_contract_address = contract_id_to_address(&test.host, user_contract_id); test.create_default_account(&user); test.create_default_account(&user_2); @@ -459,7 +460,7 @@ fn test_zero_amounts() { .set_auth(&admin, user_contract_address.clone(), true) .unwrap(); token - .clawback(&admin, user_contract_address.clone(), 0) + .clawback(&admin, user_contract_address, 0) .unwrap(); } @@ -929,7 +930,7 @@ fn test_clawback_on_contract() { let issuer_ledger_key = test .host - .to_account_key(keypair_to_account_id(&test.issuer_key).into()); + .to_account_key(keypair_to_account_id(&test.issuer_key)); let user_1 = generate_bytes_array(); let user_2 = generate_bytes_array(); @@ -982,9 +983,9 @@ fn test_clawback_on_contract() { ContractError::BalanceError ); - assert_eq!(token.balance(user_1_addr.clone()).unwrap(), 20_000_000); + assert_eq!(token.balance(user_1_addr).unwrap(), 20_000_000); - assert_eq!(token.balance(user_2_addr.clone()).unwrap(), 100_000_000); + assert_eq!(token.balance(user_2_addr).unwrap(), 100_000_000); } #[test] @@ -1092,7 +1093,7 @@ fn test_account_spendable_balance() { assert_eq!(token.balance(user_addr.clone()).unwrap(), 100_000_000); // base reserve = 5_000_000 // signer + account = 3 base reserves - assert_eq!(token.spendable(user_addr.clone()).unwrap(), 85_000_000); + assert_eq!(token.spendable(user_addr).unwrap(), 85_000_000); } #[test] @@ -1377,7 +1378,7 @@ fn test_account_invoker_auth_with_issuer_admin() { // Contract invoker can't perform unauthorized admin operation. let contract_id = generate_bytes_array(); - let contract_invoker = TestSigner::ContractInvoker(Hash(contract_id.clone())); + let contract_invoker = TestSigner::ContractInvoker(Hash(contract_id)); let contract_id_bytes = BytesN::<32>::try_from_val( &test.host, &test.host.bytes_new_from_slice(&contract_id).unwrap(), @@ -1401,10 +1402,10 @@ fn test_contract_invoker_auth() { let admin_contract_id = generate_bytes_array(); let user_contract_id = generate_bytes_array(); - let admin_contract_invoker = TestSigner::ContractInvoker(Hash(admin_contract_id.clone())); - let user_contract_invoker = TestSigner::ContractInvoker(Hash(user_contract_id.clone())); - let admin_contract_address = contract_id_to_address(&test.host, admin_contract_id.clone()); - let user_contract_address = contract_id_to_address(&test.host, user_contract_id.clone()); + let admin_contract_invoker = TestSigner::ContractInvoker(Hash(admin_contract_id)); + let user_contract_invoker = TestSigner::ContractInvoker(Hash(user_contract_id)); + let admin_contract_address = contract_id_to_address(&test.host, admin_contract_id); + let user_contract_address = contract_id_to_address(&test.host, user_contract_id); let admin_contract_id_bytes = BytesN::<32>::try_from_val( &test.host, &test.host.bytes_new_from_slice(&admin_contract_id).unwrap(), @@ -1512,7 +1513,7 @@ fn test_auth_rejected_for_incorrect_nonce() { .host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .is_err()); @@ -1529,7 +1530,7 @@ fn test_auth_rejected_for_incorrect_nonce() { test.host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .unwrap(); @@ -1547,7 +1548,7 @@ fn test_auth_rejected_for_incorrect_nonce() { .host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .is_err()); @@ -1563,9 +1564,9 @@ fn test_auth_rejected_for_incorrect_nonce() { ); test.host .call( - token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), - args.clone().into(), + token.id.into(), + Symbol::try_from_small_str("mint").unwrap(), + args.into(), ) .unwrap(); } @@ -1592,7 +1593,7 @@ fn test_auth_rejected_for_incorrect_payload() { .host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .is_err()); @@ -1603,7 +1604,7 @@ fn test_auth_rejected_for_incorrect_payload() { .host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .is_err()); @@ -1625,7 +1626,7 @@ fn test_auth_rejected_for_incorrect_payload() { .host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .is_err()); @@ -1647,7 +1648,7 @@ fn test_auth_rejected_for_incorrect_payload() { .host .call( token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + Symbol::try_from_small_str("mint").unwrap(), args.clone().into(), ) .is_err()); @@ -1657,8 +1658,8 @@ fn test_auth_rejected_for_incorrect_payload() { test.host .call( - token.id.clone().into(), - Symbol::try_from_small_str("mint").unwrap().into(), + token.id.into(), + Symbol::try_from_small_str("mint").unwrap(), args.into(), ) .unwrap(); @@ -1885,10 +1886,10 @@ fn test_classic_account_multisig_auth() { token .xfer( &TestSigner::Account(AccountSigner { - account_id: account_id, + account_id, signers: out_of_order_signers, }), - receiver.clone(), + receiver, 100, ) .err() @@ -2016,7 +2017,7 @@ fn test_native_token_classic_balance_boundaries( to_contract_err( token .xfer( - &user, + user, new_balance_signer.address(&test.host), (init_balance - expected_min_balance + 1).into(), ) @@ -2029,7 +2030,7 @@ fn test_native_token_classic_balance_boundaries( // now transfer spendable balance token .xfer( - &user, + user, new_balance_signer.address(&test.host), (init_balance - expected_min_balance).into(), ) @@ -2676,7 +2677,7 @@ fn test_recording_auth_for_token() { .unwrap(), Hash(token.id.to_array().unwrap()), xdr::ScSymbol("mint".try_into().unwrap()), - test.host.call_args_to_scvec(args.clone().into()).unwrap() + test.host.call_args_to_scvec(args.into()).unwrap() )] ); } diff --git a/soroban-env-host/src/test/tuple.rs b/soroban-env-host/src/test/tuple.rs index 7ff269038..4b1f9aca2 100644 --- a/soroban-env-host/src/test/tuple.rs +++ b/soroban-env-host/src/test/tuple.rs @@ -12,7 +12,7 @@ fn tuple_conversions() -> Result<(), HostError> { obj = host.vec_push_back(obj, (1u32).into())?; obj = host.vec_push_back(obj, (1i32).into())?; - assert_eq!(host.obj_cmp(raw.try_into()?, obj.into())?, 0); + assert_eq!(host.obj_cmp(raw, obj.into())?, 0); let roundtrip: (u32, i32) = raw.try_into_val(&host)?; assert_eq!(roundtrip, (1u32, 1i32)); @@ -25,8 +25,7 @@ fn tuple_array_conversions() -> Result<(), HostError> { let host = Host::default(); let raw: [RawVal; 0] = ().try_into_val(&host)?; - let unit: () = raw.try_into_val(&host)?; - assert_eq!(unit, ()); + raw.try_into_val(&host)?; let raw: [RawVal; 2] = (1u32, 1i32).try_into_val(&host)?; let roundtrip: (u32, i32) = raw.try_into_val(&host)?; diff --git a/soroban-env-host/src/test/util.rs b/soroban-env-host/src/test/util.rs index ce025eb63..869a66c90 100644 --- a/soroban-env-host/src/test/util.rs +++ b/soroban-env-host/src/test/util.rs @@ -108,7 +108,7 @@ impl Host { } pub(crate) fn test_scvec(&self, vals: &[T]) -> Result { - let v: Vec = vals.iter().map(|x| x.as_scval()).collect(); + let v: Vec = vals.iter().map(T::as_scval).collect(); self.map_err(v.try_into()) } diff --git a/soroban-env-host/tests/integration.rs b/soroban-env-host/tests/integration.rs index 0bc31694a..567a6670b 100644 --- a/soroban-env-host/tests/integration.rs +++ b/soroban-env-host/tests/integration.rs @@ -52,9 +52,9 @@ fn debug_fmt() { match &events.last().unwrap().event { Event::Debug(de) => { assert_eq!( - format!("{}", de), + format!("{de}"), "can't convert I32(1) to alloc::vec::Vec" - ) + ); } _ => { panic!("missing debug event") From 2963b97f1adb918802949e867a8c3a5d43b3ac69 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 16 Mar 2023 13:58:19 -0400 Subject: [PATCH 12/19] chore: fmt --- soroban-env-host/src/test/map.rs | 18 ++++++------------ soroban-env-host/src/test/token.rs | 5 +---- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/soroban-env-host/src/test/map.rs b/soroban-env-host/src/test/map.rs index 49d7de740..80f28c5dc 100644 --- a/soroban-env-host/src/test/map.rs +++ b/soroban-env-host/src/test/map.rs @@ -200,18 +200,15 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> { RawVal::from_u32(2).to_raw().get_payload() ); assert_eq!( - host.map_prev_key(test_map, sym.into())? - .get_payload(), + host.map_prev_key(test_map, sym.into())?.get_payload(), RawVal::from_u32(2).to_raw().get_payload() ); assert_eq!( - host.map_prev_key(test_map, obj_vec)? - .get_payload(), + host.map_prev_key(test_map, obj_vec)?.get_payload(), sym.as_raw().get_payload() ); assert_eq!( - host.map_prev_key(test_map, obj_map)? - .get_payload(), + host.map_prev_key(test_map, obj_map)?.get_payload(), obj_vec.get_payload() ); } @@ -226,18 +223,15 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> { sym.as_raw().get_payload() ); assert_eq!( - host.map_next_key(test_map, sym.into())? - .get_payload(), + host.map_next_key(test_map, sym.into())?.get_payload(), obj_vec.get_payload() ); assert_eq!( - host.map_next_key(test_map, obj_vec)? - .get_payload(), + host.map_next_key(test_map, obj_vec)?.get_payload(), obj_map.get_payload() ); assert_eq!( - host.map_next_key(test_map, obj_map)? - .get_payload(), + host.map_next_key(test_map, obj_map)?.get_payload(), Status::UNKNOWN_ERROR.to_raw().get_payload() ); } diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs index dea6707c8..cf78c978b 100644 --- a/soroban-env-host/src/test/token.rs +++ b/soroban-env-host/src/test/token.rs @@ -393,7 +393,6 @@ fn test_asset_init(asset_code: &[u8]) { test.issuer_key.public.to_bytes().to_vec() ] .concat() - ); let user = TestSigner::account_with_multisig(&account_id, vec![&test.user_key]); @@ -459,9 +458,7 @@ fn test_zero_amounts() { token .set_auth(&admin, user_contract_address.clone(), true) .unwrap(); - token - .clawback(&admin, user_contract_address, 0) - .unwrap(); + token.clawback(&admin, user_contract_address, 0).unwrap(); } #[test] From b58d689c5d999ae2d2ab2b30aba52896a46616fc Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 16 Mar 2023 14:46:07 -0400 Subject: [PATCH 13/19] feat: use local config to allow comments in toml --- .cargo/config.toml | 158 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..0073bff89 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,158 @@ +# paths = ["/path/to/override"] # path dependency overrides + +# [alias] # command aliases +# b = "build" +# c = "check" +# t = "test" +# r = "run" +# rr = "run --release" +# recursive_example = "rr --example recursions" +# space_example = ["run", "--release", "--", "\"command list\""] + +[build] +# jobs = 1 # number of parallel jobs, defaults to # of CPUs +# rustc = "rustc" # the rust compiler tool +# rustc-wrapper = "…" # run this wrapper instead of `rustc` +# rustc-workspace-wrapper = "…" # run this wrapper instead of `rustc` for workspace members +# rustdoc = "rustdoc" # the doc generator tool +# target = "triple" # build for the target triple (ignored by `cargo install`) +# target-dir = "target" # path of where to place all generated artifacts +rustflags = [ + "-Dwarnings", + "-Dclippy::pedantic", + "-Aclippy::module_name_repetitions", + "-Aclippy::needless_pass_by_value", + "-Aclippy::too_many_lines", + "-Aclippy::must_use_candidate", + "-Aclippy::missing_errors_doc", + "-Aclippy::missing_safety_doc", + "-Aclippy::inline_always", + "-Aclippy::cast_possible_truncation", + "-Aclippy::cast_sign_loss", + "-Aclippy::cast_possible_wrap", + "-Aclippy::similar_names", + "-Aclippy::doc_markdown", + "-Aclippy::default_trait_access", + "-Aclippy::missing_safety_doc", + "-Aclippy::struct_excessive_bools", + "-Aclippy::missing_panics_doc", + "-Aclippy::cast_lossless", + "-Aclippy::trivially_copy_pass_by_ref", + "-Aclippy::wrong_self_convention", + "-Aclippy::unused_self", + "-Aclippy::enum_glob_use", + "-Aclippy::return_self_not_must_use", + "-Aclippy::map_entry", + "-Aclippy::match_same_arms", + "-Aclippy::iter_not_returning_iterator", + "-Aclippy::unnecessary_wraps", +] # custom flags to pass to all compiler invocations +# rustdocflags = ["…", "…"] # custom flags to pass to rustdoc +# incremental = true # whether or not to enable incremental compilation +# dep-info-basedir = "…" # path for the base directory for targets in depfiles + +# [doc] +# browser = "chromium" # browser to use with `cargo doc --open`, +# # overrides the `BROWSER` environment variable + +# [env] +# # Set ENV_VAR_NAME=value for any process run by Cargo +# ENV_VAR_NAME = "value" +# # Set even if already present in environment +# ENV_VAR_NAME_2 = { value = "value", force = true } +# # Value is relative to .cargo directory containing `config.toml`, make absolute +# ENV_VAR_NAME_3 = { value = "relative/path", relative = true } + +# [future-incompat-report] +# frequency = 'always' # when to display a notification about a future incompat report + +# [cargo-new] +# vcs = "none" # VCS to use ('git', 'hg', 'pijul', 'fossil', 'none') + +# [http] +# debug = false # HTTP debugging +# proxy = "host:port" # HTTP proxy in libcurl format +# ssl-version = "tlsv1.3" # TLS version to use +# ssl-version.max = "tlsv1.3" # maximum TLS version +# ssl-version.min = "tlsv1.1" # minimum TLS version +# timeout = 30 # timeout for each HTTP request, in seconds +# low-speed-limit = 10 # network timeout threshold (bytes/sec) +# cainfo = "cert.pem" # path to Certificate Authority (CA) bundle +# check-revoke = true # check for SSL certificate revocation +# multiplexing = true # HTTP/2 multiplexing +# user-agent = "…" # the user-agent header + +# [install] +# root = "/some/path" # `cargo install` destination directory + +# [net] +# retry = 2 # network retries +# git-fetch-with-cli = true # use the `git` executable for git operations +# offline = true # do not access the network + +# [net.ssh] +# known-hosts = ["..."] # known SSH host keys + +# [patch.] +# # Same keys as for [patch] in Cargo.toml + +# [profile.] # Modify profile settings via config. +# inherits = "dev" # Inherits settings from [profile.dev]. +# opt-level = 0 # Optimization level. +# debug = true # Include debug info. +# split-debuginfo = '...' # Debug info splitting behavior. +# debug-assertions = true # Enables debug assertions. +# overflow-checks = true # Enables runtime integer overflow checks. +# lto = false # Sets link-time optimization. +# panic = 'unwind' # The panic strategy. +# incremental = true # Incremental compilation. +# codegen-units = 16 # Number of code generation units. +# rpath = false # Sets the rpath linking option. +# [profile..build-override] # Overrides build-script settings. +# # Same keys for a normal profile. +# [profile..package.] # Override profile for a package. +# # Same keys for a normal profile (minus `panic`, `lto`, and `rpath`). + +# [registries.] # registries other than crates.io +# index = "…" # URL of the registry index +# token = "…" # authentication token for the registry + +# [registry] +# default = "…" # name of the default registry +# token = "…" # authentication token for crates.io + +# [source.] # source definition and replacement +# replace-with = "…" # replace this source with the given named source +# directory = "…" # path to a directory source +# registry = "…" # URL to a registry source +# local-registry = "…" # path to a local registry source +# git = "…" # URL of a git repository source +# branch = "…" # branch name for the git repository +# tag = "…" # tag name for the git repository +# rev = "…" # revision for the git repository + +# [target.] +# linker = "…" # linker to use +# runner = "…" # wrapper to run executables +# rustflags = ["…", "…"] # custom flags for `rustc` + +# [target.] +# runner = "…" # wrapper to run executables +# rustflags = ["…", "…"] # custom flags for `rustc` + +# [target..] # `links` build script override +# rustc-link-lib = ["foo"] +# rustc-link-search = ["/path/to/foo"] +# rustc-flags = ["-L", "/some/path"] +# rustc-cfg = ['key="value"'] +# rustc-env = {key = "value"} +# rustc-cdylib-link-arg = ["…"] +# metadata_key1 = "value" +# metadata_key2 = "value" + +# [term] +# quiet = false # whether cargo output is quiet +# verbose = false # whether cargo provides verbose output +# color = 'auto' # whether cargo colorizes output +# progress.when = 'auto' # whether cargo shows progress bar +# progress.width = 80 # width of progress bar From 9cf6dedeca4fd3a21dbac8caad975d0a1ca7c1d9 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 16 Mar 2023 14:46:33 -0400 Subject: [PATCH 14/19] chore: clippy all features --- .github/workflows/rust.yml | 2 +- soroban-env-host/src/host.rs | 2 +- soroban-env-host/src/test/lifecycle.rs | 7 ++----- soroban-env-host/src/test/token.rs | 10 +++++----- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 0da6afe16..efc34a0ac 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -6,7 +6,7 @@ on: pull_request: env: - RUSTFLAGS: -D warnings + CARGO_HOME: .cargo/ci/config.toml jobs: diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 16b6501de..400d39738 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -1418,7 +1418,7 @@ impl EnvBase for Host { Ok(()) }); let escalation = self.err_status_msg(e.status, "escalating error '{}' to panic"); - panic!("{:?}", escalation) + panic!("{escalation:?}") } fn as_mut_any(&mut self) -> &mut dyn std::any::Any { diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index ad760aab5..b81dd8ad7 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -198,7 +198,7 @@ fn create_contract_using_parent_id_test() { .try_into() .unwrap(), Symbol::try_from_small_str("create").unwrap().into(), - args.clone().into(), + args.into(), ) .is_err()); @@ -216,10 +216,7 @@ fn create_contract_using_parent_id_test() { // Now successfully create the child contract itself. host.call( - host.test_bin_obj(&parent_contract_id.0) - .unwrap() - .try_into() - .unwrap(), + host.test_bin_obj(&parent_contract_id.0).unwrap(), Symbol::try_from_small_str("create").unwrap().into(), args.into(), ) diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs index cf78c978b..3497a0cb9 100644 --- a/soroban-env-host/src/test/token.rs +++ b/soroban-env-host/src/test/token.rs @@ -2543,7 +2543,7 @@ fn test_custom_account_auth() { // Initialize the admin account test.host .call( - account_contract_id_obj.clone(), + account_contract_id_obj, Symbol::try_from_small_str("init").unwrap(), host_vec![&test.host, admin_public_key.clone()].into(), ) @@ -2562,7 +2562,7 @@ fn test_custom_account_auth() { // owner. let new_admin_kp = generate_keypair(); let new_admin = TestSigner::AccountContract(AccountContractSigner { - id: account_contract_id.clone(), + id: account_contract_id, sign: simple_account_sign_fn(&test.host, &new_admin_kp), }); let new_admin_public_key = BytesN::<32>::try_from_val( @@ -2582,13 +2582,13 @@ fn test_custom_account_auth() { &admin, &account_contract_id_obj.try_into_val(&test.host).unwrap(), "set_owner", - host_vec![&test.host, new_admin_public_key.clone()], + host_vec![&test.host, new_admin_public_key], ); // Change the owner of the account. test.host .call( - account_contract_id_obj.clone(), + account_contract_id_obj, Symbol::try_from_small_str("set_owner").unwrap(), host_vec![&test.host, new_admin_public_key].into(), ) @@ -2600,7 +2600,7 @@ fn test_custom_account_auth() { assert_eq!(token.balance(user_address.clone()).unwrap(), 200); // And they shouldn't work with the old owner signatures. - assert!(token.mint(&admin, user_address.clone(), 100).is_err()); + assert!(token.mint(&admin, user_address, 100).is_err()); } #[test] From 7e839f79d88828cd510000d75eb1efa4976675ec Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 20 Mar 2023 10:19:52 -0400 Subject: [PATCH 15/19] chore: more clippy --- .cargo/config.toml | 1 + Makefile | 3 ++ .../cost_runner/cost_types/wasm_insn_exec.rs | 2 +- soroban-env-host/src/host.rs | 6 ++-- soroban-env-host/src/test/auth.rs | 8 ++--- soroban-env-host/src/test/budget_metering.rs | 6 ++-- soroban-env-host/src/test/bytes.rs | 2 +- soroban-env-host/src/test/invocation.rs | 36 +++++++++---------- soroban-env-host/src/test/lifecycle.rs | 27 +++++++------- soroban-env-host/src/test/token.rs | 2 +- soroban-env-host/src/vm.rs | 21 ++++++----- 11 files changed, 56 insertions(+), 58 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 0073bff89..05d667d49 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -46,6 +46,7 @@ rustflags = [ "-Aclippy::match_same_arms", "-Aclippy::iter_not_returning_iterator", "-Aclippy::unnecessary_wraps", + "-Aclippy::type_complexity", ] # custom flags to pass to all compiler invocations # rustdocflags = ["…", "…"] # custom flags to pass to rustdoc # incremental = true # whether or not to enable incremental compilation diff --git a/Makefile b/Makefile index 72a1051d2..3b4f45302 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,9 @@ test: build: cargo hack --feature-powerset check +clippy: + cargo hack --feature-powerset clippy --all-targets + watch: cargo watch --clear --watch-when-idle --shell '$(MAKE)' diff --git a/soroban-env-host/src/cost_runner/cost_types/wasm_insn_exec.rs b/soroban-env-host/src/cost_runner/cost_types/wasm_insn_exec.rs index f57d396c6..492aa7b1b 100644 --- a/soroban-env-host/src/cost_runner/cost_types/wasm_insn_exec.rs +++ b/soroban-env-host/src/cost_runner/cost_types/wasm_insn_exec.rs @@ -65,7 +65,7 @@ pub enum WasmInsnType { impl WasmInsnType { pub fn variants() -> std::slice::Iter<'static, WasmInsnType> { - static VARIANTS: &'static [WasmInsnType] = &[ + static VARIANTS: &[WasmInsnType] = &[ WasmInsnType::LocalGet, WasmInsnType::LocalSet, WasmInsnType::LocalTee, diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 400d39738..2e75f4b18 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -2561,10 +2561,8 @@ impl VmCallerEnv for Host { pos, len as usize, |i, slice| { - if self.symbol_matches(slice, sym)? { - if found.is_none() { - found = Some(self.usize_to_u32(i)?) - } + if self.symbol_matches(slice, sym)? && found.is_none() { + found = Some(self.usize_to_u32(i)?) } Ok(()) }, diff --git a/soroban-env-host/src/test/auth.rs b/soroban-env-host/src/test/auth.rs index 2c3d1a2db..2b65b8667 100644 --- a/soroban-env-host/src/test/auth.rs +++ b/soroban-env-host/src/test/auth.rs @@ -155,8 +155,8 @@ impl AuthTest { self.last_nonces.clear(); - for address_id in 0..self.keys.len() { - let sc_address = self.key_to_sc_address(&self.keys[address_id]); + for (address_id, key) in self.keys.iter().enumerate() { + let sc_address = self.key_to_sc_address(&key); let mut next_nonce = HashMap::, u64>::new(); for sign_root in &sign_payloads[address_id] { let contract_id_vec = sign_root.contract_id.to_vec(); @@ -184,7 +184,7 @@ impl AuthTest { let payload_preimage = HashIdPreimage::ContractAuth(HashIdPreimageContractAuth { network_id: self .host - .with_ledger_info(|li: &LedgerInfo| Ok(li.network_id.clone())) + .with_ledger_info(|li: &LedgerInfo| Ok(li.network_id)) .unwrap() .try_into() .unwrap(), @@ -246,7 +246,7 @@ impl AuthTest { // returns the recorded payloads. fn tree_run_recording(&self, root: &SetupNode) -> Vec { let addresses = self.get_addresses(); - let tree = self.convert_setup_tree(&root); + let tree = self.convert_setup_tree(root); self.run_recording( &root.contract_id, Symbol::try_from_small_str("tree_fn").unwrap(), diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index f7c77421c..c556499c1 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -55,7 +55,7 @@ fn vm_hostfn_invocation() -> Result<(), HostError> { let args = host.test_vec_obj::(&[1])?; // try_call - host.try_call(id_obj, sym.into(), args.clone().into())?; + host.try_call(id_obj, sym, args.into())?; host.with_budget(|budget| { assert_eq!(budget.get_input(CostType::InvokeVmFunction), 1); assert_eq!(budget.get_input(CostType::InvokeHostFunction), 2); @@ -152,7 +152,7 @@ fn map_insert_key_vec_obj() -> Result<(), HostError> { #[test] fn test_recursive_type_clone() -> Result<(), HostError> { let host = Host::test_host() - .test_budget(100000, 100000) + .test_budget(100_000, 100_000) .enable_model(CostType::HostMemAlloc) .enable_model(CostType::HostMemCpy); let scmap: ScMap = host.map_err( @@ -171,7 +171,7 @@ fn test_recursive_type_clone() -> Result<(), HostError> { let v: Vec> = vec![ Box::new(scmap.clone()), Box::new(scmap.clone()), - Box::new(scmap.clone()), + Box::new(scmap), ]; v.metered_clone(host.as_budget())?; diff --git a/soroban-env-host/src/test/bytes.rs b/soroban-env-host/src/test/bytes.rs index e467c312b..b33dd9a0c 100644 --- a/soroban-env-host/src/test/bytes.rs +++ b/soroban-env-host/src/test/bytes.rs @@ -141,7 +141,7 @@ fn linear_memory_operations() -> Result<(), HostError> { let id_obj = host.register_test_contract_wasm(LINEAR_MEMORY)?; // tests bytes_new_from_linear_memory { - let args = host.test_vec_obj::(&[0xaabbccdd])?; + let args = host.test_vec_obj::(&[0xaabb_ccdd])?; let obj: BytesObject = host .call( id_obj, diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs index 01daee484..62679d048 100644 --- a/soroban-env-host/src/test/invocation.rs +++ b/soroban-env-host/src/test/invocation.rs @@ -20,7 +20,7 @@ fn invoke_single_contract_function() -> Result<(), HostError> { let vm = Vm::new(&host, id, ADD_I32)?; let a = 4i32; let b = 7i32; - let c = 0x7fffffff_i32; + let c = 0x7fff_ffff_i32; let scvec0: ScVec = host.test_scvec::(&[a, b])?; let res = vm.invoke_function(&host, "add", &scvec0)?; match res { @@ -45,7 +45,7 @@ fn invoke_cross_contract(diagnostics: bool) -> Result<(), HostError> { // prepare arguments let sym = Symbol::try_from_small_str("add").unwrap(); let args = host.test_vec_obj::(&[1, 2])?; - let res = host.call(id_obj, sym.into(), args.into())?; + let res = host.call(id_obj, sym, args)?; assert!(res.is::()); assert!(res.get_tag() == Tag::I32Val); let i: i32 = res.try_into()?; @@ -72,7 +72,7 @@ fn invoke_cross_contract_with_err() -> Result<(), HostError> { let args = host.test_vec_obj::(&[1])?; // try_call - let sv = host.try_call(id_obj, sym.into(), args.clone().into())?; + let sv = host.try_call(id_obj, sym, args)?; let code = ScHostObjErrorCode::VecIndexOutOfBound; let exp_st: Status = code.into(); assert_eq!(sv.get_payload(), exp_st.to_raw().get_payload()); @@ -97,16 +97,16 @@ fn invoke_cross_contract_with_err() -> Result<(), HostError> { ) ); } else { - panic!("got: {:?}, want debug arg containing Val", de); + panic!("got: {de:?}, want debug arg containing Val"); } } _ => { - panic!("got: {:?}, want debug event", last_event); + panic!("got: {last_event:?}, want debug event"); } }; // call - let res = host.call(id_obj, sym.into(), args.into()); + let res = host.call(id_obj, sym, args); assert!(HostError::result_matches_err_status(res, code)); let events = host.get_events()?.0; @@ -129,11 +129,11 @@ fn invoke_cross_contract_with_err() -> Result<(), HostError> { ) ); } else { - panic!("got: {:?}, want debug arg containing Val", de); + panic!("got: {de:?}, want debug arg containing Val"); } } _ => { - panic!("got: {:?}, want debug event", last_event); + panic!("got: {last_event:?}, want debug event"); } }; @@ -150,7 +150,7 @@ fn invoke_cross_contract_indirect() -> Result<(), HostError> { let args = host.test_vec_obj::(&[5, 6])?; let args = host.vec_push_back(args, id1_obj.to_raw())?; // try call - let val = host.call(id0_obj, sym.into(), args.clone().into())?; + let val = host.call(id0_obj, sym, args)?; let exp: RawVal = 11i32.into(); assert_eq!(val.get_payload(), exp.get_payload()); Ok(()) @@ -166,7 +166,7 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> { let args = host.vec_push_back(args, id1_obj.into())?; // try call -- add will trap, and add_with will trap, but we will get a status - let status = host.try_call(id0_obj, sym.into(), args.clone().into())?; + let status = host.try_call(id0_obj, sym, args)?; let code = ScVmErrorCode::TrapUnreachable; let exp: Status = code.into(); assert_eq!(status.get_payload(), exp.to_raw().get_payload()); @@ -191,16 +191,16 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> { ) ); } else { - panic!("got: {:?}, want debug arg containing Val", de); + panic!("got: {de:?}, want debug arg containing Val"); } } _ => { - panic!("got: {:?}, want debug event", last_event); + panic!("got: {last_event:?}, want debug event"); } }; // call - let res = host.call(id0_obj, sym.into(), args.clone().into()); + let res = host.call(id0_obj, sym, args); assert!(HostError::result_matches_err_status(res, code)); let events = host.get_events()?.0; @@ -223,11 +223,11 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> { ) ); } else { - panic!("got: {:?}, want debug arg containing Val", de); + panic!("got: {de:?}, want debug arg containing Val"); } } _ => { - panic!("got: {:?}, want debug event", last_event); + panic!("got: {last_event:?}, want debug event"); } }; @@ -241,11 +241,11 @@ fn invoke_contract_with_reentry() -> Result<(), HostError> { // prepare arguments let sym = Symbol::try_from_small_str("add_with").unwrap(); let args = host.test_vec_obj::(&[i32::MAX, 1])?; - let args = host.vec_push_back(args, id0_obj.clone().into())?; // trying to call its own `add` function + let args = host.vec_push_back(args, id0_obj.into())?; // trying to call its own `add` function // try call -- add will trap, and add_with will trap, but we will get a status - let res = host.call(id0_obj.clone(), sym.into(), args.clone().into()); - let status = host.try_call(id0_obj, sym.into(), args.clone().into())?; + let res = host.call(id0_obj, sym, args); + let status = host.try_call(id0_obj, sym, args)?; let code = ScHostContextErrorCode::UnknownError; let exp: Status = code.into(); assert!(HostError::result_matches_err_status(res, code)); diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index b81dd8ad7..9ed09183b 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -57,7 +57,7 @@ fn test_host() -> Host { Footprint::default(), StorageMap::new(&budget).unwrap(), ); - let host = Host::with_storage_and_budget(storage, budget.clone()); + let host = Host::with_storage_and_budget(storage, budget); host.set_ledger_info(LedgerInfo { network_id: generate_bytes_array(), ..Default::default() @@ -73,7 +73,7 @@ fn test_create_contract_from_source_account(host: &Host, code: &[u8]) -> Hash { // Make contractID so we can include it in the footprint let id_pre_image = HashIdPreimage::ContractIdFromSourceAccount(HashIdPreimageSourceAccountContractId { - source_account: source_account, + source_account, salt: Uint256(salt.to_vec().try_into().unwrap()), network_id: host .hash_from_bytesobj_input("network_id", host.get_ledger_network_id().unwrap()) @@ -109,7 +109,7 @@ fn test_create_contract_from_source_account(host: &Host, code: &[u8]) -> Hash { // Create contract let wasm_id: RawVal = host - .invoke_function(HostFunction::InstallContractCode(install_args.clone())) + .invoke_function(HostFunction::InstallContractCode(install_args)) .unwrap() .try_into_val(host) .unwrap(); @@ -129,9 +129,9 @@ fn test_create_contract_from_source_account(host: &Host, code: &[u8]) -> Hash { ); assert_eq!( wasm_hash.as_slice(), - get_contract_wasm_ref(&host, contract_id.clone()).as_slice() + get_contract_wasm_ref(host, contract_id.clone()).as_slice() ); - assert_eq!(code, get_contract_wasm(&host, wasm_hash)); + assert_eq!(code, get_contract_wasm(host, wasm_hash)); contract_id } @@ -144,7 +144,7 @@ fn create_contract_using_parent_id_test() { let salt = generate_bytes_array(); let child_pre_image = HashIdPreimage::ContractIdFromContract(HashIdPreimageContractId { contract_id: parent_contract_id.clone(), - salt: Uint256(salt.clone()), + salt: Uint256(salt), network_id: host .hash_from_bytesobj_input("network_id", host.get_ledger_network_id().unwrap()) .unwrap(), @@ -157,7 +157,7 @@ fn create_contract_using_parent_id_test() { }; // Install the code for the child contract. - let wasm_hash = sha256_hash_id_preimage(install_args.clone()); + let wasm_hash = sha256_hash_id_preimage(install_args); // Add the contract code and code reference access to the footprint. host.with_mut_storage(|s: &mut Storage| { s.footprint @@ -193,12 +193,9 @@ fn create_contract_using_parent_id_test() { // Can't create the contract yet, as the code hasn't been installed yet. assert!(host .call( - host.test_bin_obj(&parent_contract_id.0) - .unwrap() - .try_into() - .unwrap(), - Symbol::try_from_small_str("create").unwrap().into(), - args.into(), + host.test_bin_obj(&parent_contract_id.0).unwrap(), + Symbol::try_from_small_str("create").unwrap(), + args, ) .is_err()); @@ -217,8 +214,8 @@ fn create_contract_using_parent_id_test() { // Now successfully create the child contract itself. host.call( host.test_bin_obj(&parent_contract_id.0).unwrap(), - Symbol::try_from_small_str("create").unwrap().into(), - args.into(), + Symbol::try_from_small_str("create").unwrap(), + args, ) .unwrap(); diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs index 3497a0cb9..ba4eee5f8 100644 --- a/soroban-env-host/src/test/token.rs +++ b/soroban-env-host/src/test/token.rs @@ -2545,7 +2545,7 @@ fn test_custom_account_auth() { .call( account_contract_id_obj, Symbol::try_from_small_str("init").unwrap(), - host_vec![&test.host, admin_public_key.clone()].into(), + host_vec![&test.host, admin_public_key].into(), ) .unwrap(); diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 1a28e198c..02ac8d00d 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -114,16 +114,15 @@ impl Vm { let mut cursor = Cursor::new(env_meta); for env_meta_entry in ScEnvMetaEntry::read_xdr_iter(&mut cursor) { match host.map_err(env_meta_entry)? { - ScEnvMetaEntry::ScEnvMetaKindInterfaceVersion(v) => { - if SUPPORTED_INTERFACE_VERSION_RANGE.contains(&v) { - return Ok(()); - } else { - return Err(host.err_status_msg( - ScHostFnErrorCode::InputArgsInvalid, - "unexpected environment interface version", - )); - } + ScEnvMetaEntry::ScEnvMetaKindInterfaceVersion(v) + if !SUPPORTED_INTERFACE_VERSION_RANGE.contains(&v) => + { + return Err(host.err_status_msg( + ScHostFnErrorCode::InputArgsInvalid, + "unexpected environment interface version", + )) } + #[allow(unreachable_patterns)] _ => (), } @@ -309,7 +308,7 @@ impl Vm { name: e.name().to_string(), param_count: f.params().len(), result_count: f.results().len(), - }) + }); } } res @@ -338,7 +337,7 @@ impl Vm { where F: FnOnce(&mut VmCaller) -> T, { - let store: &mut Store = &mut *self.store.borrow_mut(); + let store: &mut Store = &mut self.store.borrow_mut(); let mut ctx: StoreContextMut = store.into(); let caller: Caller = Caller::new(&mut ctx, Some(self.instance)); let mut vmcaller: VmCaller = VmCaller(Some(caller)); From 112766aa007dfec7d415d97f1f5f747ba1a2d84a Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 20 Mar 2023 11:22:45 -0400 Subject: [PATCH 16/19] chore: more clippy --- .../benches/common/cost_types/bytes_ops.rs | 8 +++++- .../common/cost_types/ed25519_scalar_mul.rs | 10 ++++---- .../benches/common/cost_types/im_map_ops.rs | 14 +++++------ .../benches/common/cost_types/im_vec_ops.rs | 4 +-- .../benches/common/cost_types/invoke.rs | 4 +-- .../benches/common/cost_types/val_xdr_conv.rs | 2 +- .../common/cost_types/verify_ed25519_sig.rs | 2 +- .../benches/common/cost_types/vm_ops.rs | 16 ++++-------- .../common/cost_types/wasm_insn_exec.rs | 14 ++++++++--- soroban-env-host/benches/common/measure.rs | 25 +++++++++---------- soroban-env-host/benches/common/mod.rs | 7 +++--- soroban-env-host/benches/common/modelfit.rs | 16 ++++++------ .../benches/variation_histograms.rs | 4 ++- .../benches/worst_case_linear_models.rs | 5 +++- soroban-env-host/src/auth.rs | 2 +- .../src/cost_runner/cost_types/vm_ops.rs | 25 ++++++++++++++++--- soroban-env-host/src/host.rs | 12 ++++----- soroban-env-host/src/host/mem_helper.rs | 10 ++++---- soroban-env-host/src/test/auth.rs | 13 +++++----- soroban-env-host/src/test/budget_metering.rs | 2 +- soroban-env-host/src/test/bytes.rs | 10 +++----- soroban-env-host/src/vm.rs | 21 ++++++---------- 22 files changed, 120 insertions(+), 106 deletions(-) diff --git a/soroban-env-host/benches/common/cost_types/bytes_ops.rs b/soroban-env-host/benches/common/cost_types/bytes_ops.rs index 51bd00f00..109af21f0 100644 --- a/soroban-env-host/benches/common/cost_types/bytes_ops.rs +++ b/soroban-env-host/benches/common/cost_types/bytes_ops.rs @@ -1,6 +1,12 @@ use crate::HostCostMeasurement; use rand::{rngs::StdRng, RngCore}; -use soroban_env_host::{cost_runner::*, Host}; +use soroban_env_host::{ + cost_runner::{ + BytesAppendRun, BytesCloneRun, BytesCmpRun, BytesDelRun, BytesInsertRun, BytesPopRun, + BytesPushRun, CostRunner, + }, + Host, +}; pub struct BytesCloneMeasure; impl HostCostMeasurement for BytesCloneMeasure { type Runner = BytesCloneRun; diff --git a/soroban-env-host/benches/common/cost_types/ed25519_scalar_mul.rs b/soroban-env-host/benches/common/cost_types/ed25519_scalar_mul.rs index fc797c468..4c90475fa 100644 --- a/soroban-env-host/benches/common/cost_types/ed25519_scalar_mul.rs +++ b/soroban-env-host/benches/common/cost_types/ed25519_scalar_mul.rs @@ -27,16 +27,16 @@ impl HostCostMeasurement for Ed25519ScalarMulMeasure { let A = constants::ED25519_BASEPOINT_COMPRESSED .decompress() .unwrap(); - Ed25519ScalarMulSample { a, b, A } + Ed25519ScalarMulSample { a, A, b } } fn new_worst_case(_host: &Host, _rng: &mut StdRng, _input: u64) -> Ed25519ScalarMulSample { let a = scalar::Scalar::from_bytes_mod_order([0xff; 32]); - let b = a.clone(); + let b = a; let A = constants::ED25519_BASEPOINT_COMPRESSED .decompress() .unwrap(); - Ed25519ScalarMulSample { a, b, A } + Ed25519ScalarMulSample { a, A, b } } fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> Ed25519ScalarMulSample { @@ -48,7 +48,7 @@ impl HostCostMeasurement for Ed25519ScalarMulMeasure { // occurrences here to have more than a handful of contiguous zeroes here // but for thoroughness we'll let it go up to 64 (8 bytes). for i in 0..rng.gen_range(0, 9) { - buf[31 - i] = 0 + buf[31 - i] = 0; } scalar::Scalar::from_bytes_mod_order(buf) } @@ -57,6 +57,6 @@ impl HostCostMeasurement for Ed25519ScalarMulMeasure { let A = constants::ED25519_BASEPOINT_COMPRESSED .decompress() .unwrap(); - Ed25519ScalarMulSample { a, b, A } + Ed25519ScalarMulSample { a, A, b } } } diff --git a/soroban-env-host/benches/common/cost_types/im_map_ops.rs b/soroban-env-host/benches/common/cost_types/im_map_ops.rs index 4f9834f84..a7eecad28 100644 --- a/soroban-env-host/benches/common/cost_types/im_map_ops.rs +++ b/soroban-env-host/benches/common/cost_types/im_map_ops.rs @@ -10,9 +10,7 @@ pub(crate) struct ImMapNewMeasure; impl HostCostMeasurement for ImMapNewMeasure { type Runner = ImMapNewRun; - fn new_random_case(_host: &Host, _rng: &mut StdRng, _input: u64) { - () - } + fn new_random_case(_host: &Host, _rng: &mut StdRng, _input: u64) {} } pub(crate) struct ImMapImmutEntryMeasure; @@ -26,7 +24,7 @@ impl HostCostMeasurement for ImMapImmutEntryMeasure { let input = input * 100; let mut keys: Vec<_> = util::to_rawval_u32(0..(input as u32)).collect(); keys.shuffle(rng); - let om = keys.iter().cloned().zip(keys.iter().cloned()).collect(); + let om = keys.iter().copied().zip(keys.iter().copied()).collect(); let map: MeteredOrdMap<_, _, _> = MeteredOrdMap::from_map(om, host).unwrap(); keys.shuffle(rng); ImMapImmutEntrySample { map, keys } @@ -35,15 +33,15 @@ impl HostCostMeasurement for ImMapImmutEntryMeasure { fn new_worst_case(host: &Host, _rng: &mut StdRng, input: u64) -> ImMapImmutEntrySample { let input = input * 100; let keys: Vec<_> = util::to_rawval_u32(0..(input as u32)).collect(); - let om = keys.iter().cloned().zip(keys.iter().cloned()).collect(); + let om = keys.iter().copied().zip(keys.iter().copied()).collect(); let map: MeteredOrdMap<_, _, _> = MeteredOrdMap::from_map(om, host).unwrap(); - let keys = util::to_rawval_u32([0, u32::MAX].iter().cloned()).collect(); + let keys = util::to_rawval_u32([0, u32::MAX].iter().copied()).collect(); ImMapImmutEntrySample { map, keys } } fn new_best_case(host: &Host, _rng: &mut StdRng) -> ImMapImmutEntrySample { - let keys: Vec<_> = util::to_rawval_u32([0].iter().cloned()).collect(); - let om = keys.iter().cloned().zip(keys.iter().cloned()).collect(); + let keys: Vec<_> = util::to_rawval_u32([0].iter().copied()).collect(); + let om = keys.iter().copied().zip(keys.iter().copied()).collect(); let map: MeteredOrdMap<_, _, _> = MeteredOrdMap::from_map(om, host).unwrap(); ImMapImmutEntrySample { map, keys } } diff --git a/soroban-env-host/benches/common/cost_types/im_vec_ops.rs b/soroban-env-host/benches/common/cost_types/im_vec_ops.rs index d1271cbcd..8b701080f 100644 --- a/soroban-env-host/benches/common/cost_types/im_vec_ops.rs +++ b/soroban-env-host/benches/common/cost_types/im_vec_ops.rs @@ -10,9 +10,7 @@ pub(crate) struct ImVecNewMeasure; impl HostCostMeasurement for ImVecNewMeasure { type Runner = ImVecNewRun; - fn new_random_case(_host: &Host, _rng: &mut StdRng, _input: u64) { - () - } + fn new_random_case(_host: &Host, _rng: &mut StdRng, _input: u64) {} } pub(crate) struct ImVecImmutEntryMeasure; diff --git a/soroban-env-host/benches/common/cost_types/invoke.rs b/soroban-env-host/benches/common/cost_types/invoke.rs index 0679a6230..75f696f7d 100644 --- a/soroban-env-host/benches/common/cost_types/invoke.rs +++ b/soroban-env-host/benches/common/cost_types/invoke.rs @@ -36,7 +36,7 @@ impl HostCostMeasurement for InvokeVmFunctionMeasure { fn new_random_case(host: &Host, _rng: &mut StdRng, _input: u64) -> Rc { let id: Hash = [0; 32].into(); let code = wasm_module_with_empty_invoke(); - Vm::new(&host, id, &code).unwrap() + Vm::new(host, id, &code).unwrap() } } @@ -54,6 +54,6 @@ impl HostCostMeasurement for InvokeHostFunctionMeasure { let input = input * 1000; let id: Hash = [0; 32].into(); let code = wasm_module_with_dummy_hostfn_invoke(input); - Vm::new(&host, id, &code).unwrap() + Vm::new(host, id, &code).unwrap() } } diff --git a/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs b/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs index 5b0fa180c..dea190bd3 100644 --- a/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs +++ b/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs @@ -11,7 +11,7 @@ impl HostCostMeasurement for ValXdrConvMeasure { fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> (RawVal, ScVal) { let v = rng.next_u32(); let rv: RawVal = v.into(); - let scv: ScVal = ScVal::U32(v as u32); + let scv: ScVal = ScVal::U32(v); (rv, scv) } } diff --git a/soroban-env-host/benches/common/cost_types/verify_ed25519_sig.rs b/soroban-env-host/benches/common/cost_types/verify_ed25519_sig.rs index e2894b651..4036d0bd0 100644 --- a/soroban-env-host/benches/common/cost_types/verify_ed25519_sig.rs +++ b/soroban-env-host/benches/common/cost_types/verify_ed25519_sig.rs @@ -17,7 +17,7 @@ impl HostCostMeasurement for VerifyEd25519SigMeasure { fn new_random_case(_host: &Host, rng: &mut StdRng, input: u64) -> VerifyEd25519SigSample { let size = input * 10000; let keypair: Keypair = Keypair::generate(rng); - let key: PublicKey = keypair.public.clone(); + let key: PublicKey = keypair.public; let msg: Vec = (0..size).map(|x| x as u8).collect(); let sig: Signature = keypair.sign(msg.as_slice()); VerifyEd25519SigSample { key, msg, sig } diff --git a/soroban-env-host/benches/common/cost_types/vm_ops.rs b/soroban-env-host/benches/common/cost_types/vm_ops.rs index c176cf8be..0364a997c 100644 --- a/soroban-env-host/benches/common/cost_types/vm_ops.rs +++ b/soroban-env-host/benches/common/cost_types/vm_ops.rs @@ -16,19 +16,14 @@ impl HostCostMeasurement for VmInstantiationMeasure { type Runner = VmInstantiationRun; fn new_best_case(_host: &Host, _rng: &mut StdRng) -> VmInstantiationSample { - let id: xdr::Hash = [0; 32].into(); - let wasm: Vec = soroban_test_wasms::ADD_I32.clone().into(); - VmInstantiationSample { id, wasm } + VmInstantiationSample::default_id(soroban_test_wasms::ADD_I32) } fn new_worst_case(_host: &Host, _rng: &mut StdRng, _input: u64) -> VmInstantiationSample { - let id: xdr::Hash = [0; 32].into(); - let wasm: Vec = soroban_test_wasms::COMPLEX.clone().into(); - VmInstantiationSample { id, wasm } + VmInstantiationSample::default_id(soroban_test_wasms::COMPLEX) } fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> VmInstantiationSample { - let id: xdr::Hash = [0; 32].into(); let wasm = match rng.gen_range(0, 9) { 0 => soroban_test_wasms::ADD_I32, 1 => soroban_test_wasms::COMPLEX, @@ -41,8 +36,7 @@ impl HostCostMeasurement for VmInstantiationMeasure { 8 => soroban_test_wasms::VEC, _ => unreachable!(), }; - let wasm: Vec = wasm.clone().into(); - VmInstantiationSample { id, wasm } + VmInstantiationSample::default_id(wasm) } } @@ -57,7 +51,7 @@ impl HostCostMeasurement for VmMemReadMeasure { let input = (input * 1000) as usize; let id: xdr::Hash = [0; 32].into(); let code = soroban_test_wasms::ADD_I32; - let vm = Vm::new(&host, id, &code).unwrap(); + let vm = Vm::new(host, id, code).unwrap(); (vm, input) } } @@ -75,7 +69,7 @@ impl HostCostMeasurement for VmMemWriteMeasure { rng.fill_bytes(buf.as_mut_slice()); let id: xdr::Hash = [0; 32].into(); let code = soroban_test_wasms::ADD_I32; - let vm = Vm::new(&host, id, &code).unwrap(); + let vm = Vm::new(host, id, code).unwrap(); (vm, buf) } } diff --git a/soroban-env-host/benches/common/cost_types/wasm_insn_exec.rs b/soroban-env-host/benches/common/cost_types/wasm_insn_exec.rs index 651244bd8..007abbe15 100644 --- a/soroban-env-host/benches/common/cost_types/wasm_insn_exec.rs +++ b/soroban-env-host/benches/common/cost_types/wasm_insn_exec.rs @@ -1,7 +1,15 @@ use crate::common::HostCostMeasurement; use rand::{rngs::StdRng, RngCore}; use soroban_env_host::{ - cost_runner::*, + cost_runner::{ + BrRun, BrTableRun, CallIndirectRun, CallRun, ConstRun, DropRun, GlobalGetRun, GlobalSetRun, + I64AddRun, I64AndRun, I64ClzRun, I64CtzRun, I64DivSRun, I64EqRun, I64EqzRun, I64GeSRun, + I64GtSRun, I64LeSRun, I64Load16SRun, I64Load32SRun, I64Load8SRun, I64LoadRun, I64LtSRun, + I64MulRun, I64NeRun, I64OrRun, I64PopcntRun, I64RemSRun, I64RotlRun, I64RotrRun, I64ShlRun, + I64ShrSRun, I64Store16Run, I64Store32Run, I64Store8Run, I64StoreRun, I64SubRun, I64XorRun, + LocalGetRun, LocalSetRun, LocalTeeRun, MemoryGrowRun, MemorySizeRun, SelectRun, + WasmInsnExecRun, WasmInsnExecSample, WasmInsnSample, WasmMemAllocRun, + }, xdr::{Hash, ScVal, ScVec}, Host, Symbol, Vm, }; @@ -467,7 +475,7 @@ impl HostCostMeasurement for WasmInsnExecMeasure { let args = ScVec(vec![ScVal::U64(5)].try_into().unwrap()); let id: Hash = [0; 32].into(); let code = wasm_module_with_4n_insns(insns as usize); - let vm = Vm::new(&host, id, &code).unwrap(); + let vm = Vm::new(host, id, &code).unwrap(); WasmInsnExecSample { insns, args, vm } } } @@ -487,7 +495,7 @@ impl HostCostMeasurement for WasmMemAllocMeasure { let n_pages = input as usize; let id: Hash = [0; 32].into(); let code = wasm_module_with_mem_grow(n_pages); - let vm = Vm::new(&host, id, &code).unwrap(); + let vm = Vm::new(host, id, &code).unwrap(); (vm, n_pages) } } diff --git a/soroban-env-host/benches/common/measure.rs b/soroban-env-host/benches/common/measure.rs index 9a3b65612..9112e49d2 100644 --- a/soroban-env-host/benches/common/measure.rs +++ b/soroban-env-host/benches/common/measure.rs @@ -44,7 +44,6 @@ impl AllocationTracker for MemTracker { _current_group_id: tracking_allocator::AllocationGroupId, ) { // No-Op, see comment above. - () } } @@ -58,6 +57,8 @@ pub struct Measurement { #[derive(Clone, Debug, Default)] pub struct Measurements(pub Vec); + +#[allow(clippy::cast_precision_loss)] impl Measurements { /// Subtracts the values of the first measurement from all subsequent /// measurements, to eliminate constant factors; this should only be done if @@ -78,10 +79,12 @@ impl Measurements { } } + #[allow(clippy::cast_precision_loss)] pub fn report_histogram(&self, out_name: &str, get_output: F) where F: Fn(&Measurement) -> u64, { + use textplots::{Chart, Plot, Shape}; use thousands::Separable; let points: Vec<(f32, f32)> = self .0 @@ -92,7 +95,7 @@ impl Measurements { let ymin = points.iter().map(|(_, y)| *y).reduce(f32::min).unwrap(); let ymax = points.iter().map(|(_, y)| *y).reduce(f32::max).unwrap(); - if ymin == ymax { + if (ymin - ymax).abs() < 0.00001 { return; } let hist = textplots::utils::histogram(&points, ymin, ymax, 30); @@ -110,7 +113,6 @@ impl Measurements { .reduce(f32::max) .unwrap(); - use textplots::{Chart, Plot, Shape}; println!( "cost input: min {}; max {}; max/min = {}", in_min, @@ -141,7 +143,7 @@ impl Measurements { "input\tcpu insns\tmem bytes\ttime nsecs\tinsns/input\tbytes/input\tnsecs/input" ) .unwrap(); - for m in self.0.iter() { + for m in &self.0 { writeln!( &mut tw, "{}\t{}\t{}\t{}ns\t{}\t{}\t{}ns", @@ -167,7 +169,7 @@ impl Measurements { x: m.input as f64, y: m.cpu_insns as f64, }) - .collect(); + .collect::>(); fit_model(&data) } @@ -179,7 +181,7 @@ impl Measurements { x: m.input as f64, y: m.mem_bytes as f64, }) - .collect(); + .collect::>(); fit_model(&data) } } @@ -218,7 +220,7 @@ pub trait HostCostMeasurement: Sized { } fn run(host: &Host, sample: Vec<::SampleType>) { - ::run(host, sample) + ::run(host, sample); } fn get_total_input(host: &Host, sample: &::SampleType) -> u64 { @@ -262,7 +264,7 @@ mod cpu { pub struct InstructionCounter(u64); impl InstructionCounter { fn get() -> u64 { - use rusagev4::*; + use rusagev4::{proc_pid_rusage, rusage_info_v4, RUSAGE_INFO_V4}; use std::os::raw::c_int; let mut ri = rusage_info_v4::default(); let pid: c_int = std::process::id() as c_int; @@ -307,10 +309,7 @@ where // prepare the measurement let host = Host::default(); host.with_budget(|budget| budget.reset_unlimited()); - let sample = match next_sample(&host) { - Some(s) => s, - None => break, - }; + let Some(sample) = next_sample(&host) else { break }; let iterations = ::RUN_ITERATIONS; let mvec = (0..iterations).map(|_| sample.clone()).collect(); // start the cpu and mem measurement @@ -325,7 +324,7 @@ where drop(alloc_guard); let stop = Instant::now(); let input = HCM::get_total_input(&host, &sample) / iterations; - cpu_insns = cpu_insns - HCM::get_insns_overhead(&host, &sample); + cpu_insns -= HCM::get_insns_overhead(&host, &sample); let mem_bytes = mem_tracker.0.load(Ordering::SeqCst) / iterations; let time_nsecs = stop.duration_since(start).as_nanos() as u64 / iterations; ret.push(Measurement { diff --git a/soroban-env-host/benches/common/mod.rs b/soroban-env-host/benches/common/mod.rs index 5fc32118b..83747207a 100644 --- a/soroban-env-host/benches/common/mod.rs +++ b/soroban-env-host/benches/common/mod.rs @@ -5,6 +5,7 @@ mod measure; mod modelfit; mod util; +#[allow(clippy::wildcard_imports)] use cost_types::*; pub use measure::*; pub use modelfit::*; @@ -20,7 +21,7 @@ pub(crate) trait Benchmark { } fn get_explicit_bench_names() -> Option> { - let bare_args: Vec<_> = std::env::args().filter(|x| !x.starts_with("-")).collect(); + let bare_args: Vec<_> = std::env::args().filter(|x| !x.starts_with('-')).collect(); match bare_args.len() { 0 | 1 => None, _ => Some(bare_args[1..].into()), @@ -34,7 +35,7 @@ fn should_run() -> bool { .last() .unwrap() .to_string(); - bench_names.into_iter().find(|arg| *arg == name).is_some() + bench_names.into_iter().any(|arg| arg == name) } else { true } @@ -90,7 +91,7 @@ pub(crate) fn for_each_host_cost_measurement() -> std::io::Result< if get_explicit_bench_names().is_none() { for cost in CostType::variants() { if !costs.contains(cost) { - eprintln!("warning: missing cost measurement for {:?}", cost); + eprintln!("warning: missing cost measurement for {cost:?}"); } } // Missing because we need some kind of size limits: diff --git a/soroban-env-host/benches/common/modelfit.rs b/soroban-env-host/benches/common/modelfit.rs index 5009114b0..d7c355fd5 100644 --- a/soroban-env-host/benches/common/modelfit.rs +++ b/soroban-env-host/benches/common/modelfit.rs @@ -21,13 +21,13 @@ pub struct FPCostModel { // confused due to rounding/truncation. impl FPCostModel { pub fn new(params: &[f64]) -> Self { - let mut fcm = FPCostModel::default(); - fcm.const_param = params[0]; - fcm.log_param = params[1]; - fcm.log_base_param = params[2]; - fcm.lin_param = params[3]; - fcm.quad_param = params[4]; - fcm + FPCostModel { + const_param: params[0], + log_param: params[1], + log_base_param: params[2], + lin_param: params[3], + quad_param: params[4], + } } // This is the same as the 'evaluate' function in the integral cost model, // just using f64 ops rather than saturating integer ops. @@ -44,7 +44,7 @@ impl FPCostModel { // Fits a FloatCostModel to the provided data, using least-squares // gradient descent optimization. -pub fn fit_model(data: &Vec) -> FPCostModel { +pub fn fit_model(data: &[FPPoint]) -> FPCostModel { // We construct an objective function for the optimizer here that treats its // inputs as parametes to a FloatCostModel, and returns the sum (over all // provided data points) of squares of differences between the data.y value diff --git a/soroban-env-host/benches/variation_histograms.rs b/soroban-env-host/benches/variation_histograms.rs index 34792625b..b785e7e72 100644 --- a/soroban-env-host/benches/variation_histograms.rs +++ b/soroban-env-host/benches/variation_histograms.rs @@ -1,7 +1,9 @@ // Run this with // $ cargo bench --features vm --bench variation_histograms -- --nocapture mod common; -use common::*; +use common::{ + for_each_host_cost_measurement, measure_cost_variation, Benchmark, HostCostMeasurement, +}; struct LinearModelTables; impl Benchmark for LinearModelTables { diff --git a/soroban-env-host/benches/worst_case_linear_models.rs b/soroban-env-host/benches/worst_case_linear_models.rs index 3c0719cb5..e85532376 100644 --- a/soroban-env-host/benches/worst_case_linear_models.rs +++ b/soroban-env-host/benches/worst_case_linear_models.rs @@ -1,7 +1,10 @@ // Run this with // $ cargo bench --features vm --bench worst_case_linear_models -- --nocapture mod common; -use common::*; +use common::{ + for_each_host_cost_measurement, for_each_wasm_insn_measurement, measure_worst_case_costs, + Benchmark, HostCostMeasurement, +}; struct WorstCaseLinearModels; impl Benchmark for WorstCaseLinearModels { diff --git a/soroban-env-host/src/auth.rs b/soroban-env-host/src/auth.rs index a461b296b..e107269c6 100644 --- a/soroban-env-host/src/auth.rs +++ b/soroban-env-host/src/auth.rs @@ -422,7 +422,7 @@ impl AuthorizationManager { let (contract_id, function_name) = match frame { #[cfg(feature = "vm")] Frame::ContractVM(vm, fn_name, _) => { - (vm.contract_id.metered_clone(&self.budget)?, fn_name.clone()) + (vm.contract_id.metered_clone(&self.budget)?, *fn_name) } // Just skip the host function stack frames for now. // We could also make this included into the authorized stack to diff --git a/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs b/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs index 4572b6ca9..bab29112f 100644 --- a/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs +++ b/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs @@ -9,6 +9,22 @@ pub struct VmInstantiationSample { pub wasm: Vec, } +impl VmInstantiationSample { + pub fn new>>(id: Hash, wasm: I) -> Self { + Self { + id, + wasm: wasm.into(), + } + } + + pub fn default_id>>(wasm: I) -> Self { + Self { + id: [0; 32].into(), + wasm: wasm.into(), + } + } +} + impl CostRunner for VmInstantiationRun { const COST_TYPE: CostType = CostType::VmInstantiation; const RUN_ITERATIONS: u64 = 10; @@ -30,8 +46,8 @@ impl CostRunner for VmMemReadRun { let vm = sample.0; vm.with_vmcaller(|caller| { host.metered_vm_read_bytes_from_linear_memory(caller, &vm, 0, &mut buf) - .unwrap() - }) + .unwrap(); + }); } } @@ -41,11 +57,12 @@ impl CostRunner for VmMemWriteRun { type SampleType = (Rc, Vec); + #[allow(clippy::unnecessary_mut_passed)] fn run_iter(host: &crate::Host, _iter: u64, mut sample: Self::SampleType) { let vm = sample.0; vm.with_vmcaller(|caller| { host.metered_vm_write_bytes_to_linear_memory(caller, &vm, 0, &mut sample.1) - .unwrap() - }) + .unwrap(); + }); } } diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 2e75f4b18..c8ac71141 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -1964,14 +1964,14 @@ impl VmCallerEnv for Host { &vm, vals_pos, vals.as_mut_slice(), - |buf| RawVal::from_payload(u64::from_le_bytes(buf.clone())), + |buf| RawVal::from_payload(u64::from_le_bytes(*buf)), )?; // Step 3: turn pairs into a map. let pair_iter = key_syms .iter() - .map(|s| s.to_raw()) - .zip(vals.iter().cloned()); + .map(Symbol::to_raw) + .zip(vals.iter().copied()); let map = HostMap::from_exact_iter(pair_iter, self)?; self.add_host_object(map) } @@ -2246,7 +2246,7 @@ impl VmCallerEnv for Host { &vm, pos, vals.as_mut_slice(), - |buf| RawVal::from_payload(u64::from_le_bytes(buf.clone())), + |buf| RawVal::from_payload(u64::from_le_bytes(*buf)), )?; self.add_host_object(HostVec::from_vec(vals)?) } @@ -2562,7 +2562,7 @@ impl VmCallerEnv for Host { len as usize, |i, slice| { if self.symbol_matches(slice, sym)? && found.is_none() { - found = Some(self.usize_to_u32(i)?) + found = Some(self.usize_to_u32(i)?); } Ok(()) }, @@ -2844,7 +2844,7 @@ impl VmCallerEnv for Host { let vals = match frame { #[cfg(feature = "vm")] Frame::ContractVM(vm, function, _) => { - get_host_val_tuple(&vm.contract_id, &function)? + get_host_val_tuple(&vm.contract_id, function)? } Frame::HostFunction(_) => continue, Frame::Token(id, function, _) => get_host_val_tuple(id, function)?, diff --git a/soroban-env-host/src/host/mem_helper.rs b/soroban-env-host/src/host/mem_helper.rs index 182e3075c..7df8ba78c 100644 --- a/soroban-env-host/src/host/mem_helper.rs +++ b/soroban-env-host/src/host/mem_helper.rs @@ -41,7 +41,7 @@ impl Host { let mem = vm.get_memory(self)?; self.map_err( mem.write(vmcaller.try_mut()?, mem_pos as usize, buf) - .map_err(|me| wasmi::Error::Memory(me)), + .map_err(wasmi::Error::Memory), ) } @@ -57,7 +57,7 @@ impl Host { let mem = vm.get_memory(self)?; self.map_err( mem.read(vmcaller.try_mut()?, mem_pos as usize, buf) - .map_err(|me| wasmi::Error::Memory(me)), + .map_err(wasmi::Error::Memory), ) } @@ -87,7 +87,7 @@ impl Host { let mem_slice = mem_data.get_mut(mem_range).ok_or(ScVmErrorCode::Memory)?; self.charge_budget(CostType::VmMemWrite, byte_len as u64)?; - for (src, dst) in buf.iter().zip(mem_slice.chunks_mut(VAL_SZ as usize)) { + for (src, dst) in buf.iter().zip(mem_slice.chunks_mut(VAL_SZ)) { if dst.len() != VAL_SZ { // This should be impossible unless there's an error above, but just in case. return Err(ScHostObjErrorCode::VecIndexOutOfBound.into()); @@ -125,7 +125,7 @@ impl Host { self.charge_budget(CostType::VmMemRead, byte_len as u64)?; let mut tmp: [u8; VAL_SZ] = [0u8; VAL_SZ]; - for (dst, src) in buf.iter_mut().zip(mem_slice.chunks(VAL_SZ as usize)) { + for (dst, src) in buf.iter_mut().zip(mem_slice.chunks(VAL_SZ)) { if let Ok(src) = TryInto::<&[u8; VAL_SZ]>::try_into(src) { tmp.copy_from_slice(src); *dst = from_le_bytes(&tmp); @@ -183,7 +183,7 @@ impl Host { .ok_or(ScHostValErrorCode::U32OutOfRange)?; let slice_range = slice_ptr as usize..slice_end as usize; let slice = mem_data.get(slice_range).ok_or(ScVmErrorCode::Memory)?; - callback(i, slice)? + callback(i, slice)?; } else { // This should be impossible unless there's an error above, but just in case. return Err(ScHostObjErrorCode::VecIndexOutOfBound.into()); diff --git a/soroban-env-host/src/test/auth.rs b/soroban-env-host/src/test/auth.rs index 2b65b8667..ea6ab4e68 100644 --- a/soroban-env-host/src/test/auth.rs +++ b/soroban-env-host/src/test/auth.rs @@ -96,14 +96,14 @@ impl AuthTest { &AccountId(PublicKey::PublicKeyTypeEd25519(Uint256( account.public.to_bytes(), ))), - vec![(&account, 1)], + vec![(account, 1)], 100_000_000, 1, [1, 0, 0, 0], None, None, 0, - ) + ); } let mut contracts = vec![]; for _ in 0..contract_cnt { @@ -135,9 +135,8 @@ impl AuthTest { host_vec![ &self.host, self.get_addresses(), - self.convert_setup_tree(&root) - ] - .into(), + self.convert_setup_tree(root) + ], sign_payloads, success, ); @@ -156,7 +155,7 @@ impl AuthTest { self.last_nonces.clear(); for (address_id, key) in self.keys.iter().enumerate() { - let sc_address = self.key_to_sc_address(&key); + let sc_address = self.key_to_sc_address(key); let mut next_nonce = HashMap::, u64>::new(); for sign_root in &sign_payloads[address_id] { let contract_id_vec = sign_root.contract_id.to_vec(); @@ -213,7 +212,7 @@ impl AuthTest { self.host.set_authorization_entries(contract_auth).unwrap(); assert_eq!( self.host - .call(contract_id.clone().into(), fn_name, args.into(),) + .call(contract_id.into(), fn_name, args.into()) .is_ok(), success ); diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index c556499c1..e80b526fd 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -55,7 +55,7 @@ fn vm_hostfn_invocation() -> Result<(), HostError> { let args = host.test_vec_obj::(&[1])?; // try_call - host.try_call(id_obj, sym, args.into())?; + host.try_call(id_obj, sym, args)?; host.with_budget(|budget| { assert_eq!(budget.get_input(CostType::InvokeVmFunction), 1); assert_eq!(budget.get_input(CostType::InvokeHostFunction), 2); diff --git a/soroban-env-host/src/test/bytes.rs b/soroban-env-host/src/test/bytes.rs index b33dd9a0c..a632f0b17 100644 --- a/soroban-env-host/src/test/bytes.rs +++ b/soroban-env-host/src/test/bytes.rs @@ -145,8 +145,8 @@ fn linear_memory_operations() -> Result<(), HostError> { let obj: BytesObject = host .call( id_obj, - Symbol::try_from_small_str("bin_word").unwrap().into(), - args.into(), + Symbol::try_from_small_str("bin_word").unwrap(), + args, )? .try_into()?; let obj_ref: BytesObject = host.test_bin_obj(&[0xaa, 0xbb, 0xcc, 0xdd])?; @@ -161,11 +161,7 @@ fn linear_memory_operations() -> Result<(), HostError> { let mut args = host.vec_new(RawVal::from_void().into())?; args = host.vec_push_back(args, obj0.to_raw())?; let obj: BytesObject = host - .call( - id_obj, - Symbol::try_from_small_str("bin_inc").unwrap().into(), - args.into(), - )? + .call(id_obj, Symbol::try_from_small_str("bin_inc").unwrap(), args)? .try_into()?; let obj_ref = host.test_bin_obj(&[2, 3, 4, 5])?; assert_eq!( diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 02ac8d00d..de95b8bd9 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -124,7 +124,7 @@ impl Vm { } #[allow(unreachable_patterns)] - _ => (), + ScEnvMetaEntry::ScEnvMetaKindInterfaceVersion(_) => (), } } Err(host.err_status_msg( @@ -244,25 +244,18 @@ impl Vm { .collect(); let mut wasm_ret: [Value; 1] = [Value::I64(0)]; let func_ss: SymbolStr = func.try_into_val(host)?; - let ext = match self + let Some(ext) = self .instance - .get_export(&*self.store.borrow(), func_ss.as_ref()) - { - None => { + .get_export(&*self.store.borrow(), func_ss.as_ref()) else { return Err( host.err_status_msg(ScVmErrorCode::Unknown, "invoking unknown export") ) - } - Some(e) => e, - }; - let func = match ext.into_func() { - None => { + }; + let Some(func) = ext.into_func() else { return Err( host.err_status_msg(ScVmErrorCode::Unknown, "export is not a function") ) - } - Some(e) => e, - }; + }; host.map_err(func.call( &mut *self.store.borrow_mut(), wasm_args.as_slice(), @@ -296,7 +289,7 @@ impl Vm { raw_args.push(host.to_host_val(scv)?); } let raw_res = self.invoke_function_raw(host, &func_sym, raw_args.as_slice())?; - Ok(host.from_host_val(raw_res)?) + host.from_host_val(raw_res) } /// Returns a list of functions in the WASM module loaded into the [Vm]. From d1fa8a01422ee05e1e2715473182c2d55f88cb74 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 20 Mar 2023 11:30:46 -0400 Subject: [PATCH 17/19] fix(CI): rely on config.toml instead of passing rustflags explicitly --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index efc34a0ac..96e3c089e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -64,7 +64,7 @@ jobs: with: name: cargo-hack version: 0.5.16 - - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator -Aclippy::unnecessary_wraps + - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} - if: matrix.sys.test run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }} From 685303d0b883edd685715de47f71490e749dacf9 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 20 Mar 2023 14:46:24 -0400 Subject: [PATCH 18/19] fix: remove env var for config home --- .github/workflows/rust.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 96e3c089e..cfb9e0456 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -5,9 +5,6 @@ on: branches: [main, release/**] pull_request: -env: - CARGO_HOME: .cargo/ci/config.toml - jobs: complete: From b4d21f5f10a36dc61b213f37f7d50c5a93e66db7 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 20 Mar 2023 16:30:45 -0400 Subject: [PATCH 19/19] fix: clippy in guest --- soroban-env-guest/src/guest.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/soroban-env-guest/src/guest.rs b/soroban-env-guest/src/guest.rs index ccf502108..11eaa2a2f 100644 --- a/soroban-env-guest/src/guest.rs +++ b/soroban-env-guest/src/guest.rs @@ -173,12 +173,10 @@ impl EnvBase for Guest { } fn as_mut_any(&mut self) -> &mut dyn core::any::Any { - return self; + self } - fn check_same_env(&self, other: &Self) { - () - } + fn check_same_env(&self, other: &Self) {} fn deep_clone(&self) -> Self { Self