From 1bcc1605e7c9e765a092e04d23e7cded7e30e66d Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 27 Jul 2021 10:55:40 +0000 Subject: [PATCH] lint: add clippy --- .editorconfig | 1 + .github/workflows/rust.yml | 2 ++ core/src/error.rs | 24 ++++++------------------ core/src/eval/arithmetic.rs | 2 +- core/src/eval/bitwise.rs | 18 ++++++------------ core/src/eval/misc.rs | 3 ++- core/src/memory.rs | 1 + core/src/opcode.rs | 2 +- core/src/stack.rs | 6 ++++++ core/src/utils.rs | 18 ++++++++++-------- gasometer/src/costs.rs | 22 ++++++++++------------ gasometer/src/lib.rs | 1 + gasometer/src/utils.rs | 2 +- runtime/src/eval/system.rs | 8 ++++---- src/backend/memory.rs | 10 +++++----- src/executor/stack/mod.rs | 1 + src/executor/stack/state.rs | 3 ++- 17 files changed, 60 insertions(+), 64 deletions(-) diff --git a/.editorconfig b/.editorconfig index 01161bbe8..48984739c 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,4 +1,5 @@ root = true + [*] indent_style=tab indent_size=tab diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 94f5a9e11..ab16a7fa0 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -16,6 +16,8 @@ jobs: - uses: actions/checkout@v2 - name: Rustfmt run: cargo fmt --all -- --check + - name: Clippy + run: cargo clippy --all build: runs-on: ubuntu-latest steps: diff --git a/core/src/error.rs b/core/src/error.rs index f2820553f..8b930f717 100644 --- a/core/src/error.rs +++ b/core/src/error.rs @@ -33,34 +33,22 @@ pub enum ExitReason { impl ExitReason { /// Whether the exit is succeeded. pub fn is_succeed(&self) -> bool { - match self { - Self::Succeed(_) => true, - _ => false, - } + matches!(self, Self::Succeed(_)) } /// Whether the exit is error. pub fn is_error(&self) -> bool { - match self { - Self::Error(_) => true, - _ => false, - } + matches!(self, Self::Error(_)) } /// Whether the exit is revert. pub fn is_revert(&self) -> bool { - match self { - Self::Revert(_) => true, - _ => false, - } + matches!(self, Self::Revert(_)) } /// Whether the exit is fatal. pub fn is_fatal(&self) -> bool { - match self { - Self::Fatal(_) => true, - _ => false, - } + matches!(self, Self::Fatal(_)) } } @@ -120,8 +108,8 @@ pub enum ExitError { /// Create init code exceeds limit (runtime). CreateContractLimit, - /// An opcode accesses external information, but the request is off offset - /// limit (runtime). + /// An opcode accesses external information, but the request is off offset + /// limit (runtime). OutOfOffset, /// Execution runs out of gas (runtime). OutOfGas, diff --git a/core/src/eval/arithmetic.rs b/core/src/eval/arithmetic.rs index b5262a975..ae6e9a15c 100644 --- a/core/src/eval/arithmetic.rs +++ b/core/src/eval/arithmetic.rs @@ -81,7 +81,7 @@ pub fn exp(op1: U256, op2: U256) -> U256 { if op2 & 1.into() != 0.into() { r = r.overflowing_mul(op1).0; } - op2 = op2 >> 1; + op2 >>= 1; op1 = op1.overflowing_mul(op1).0; } diff --git a/core/src/eval/bitwise.rs b/core/src/eval/bitwise.rs index 6beabfeec..3d0dd19b3 100644 --- a/core/src/eval/bitwise.rs +++ b/core/src/eval/bitwise.rs @@ -58,33 +58,29 @@ pub fn byte(op1: U256, op2: U256) -> U256 { #[inline] pub fn shl(shift: U256, value: U256) -> U256 { - let ret = if value == U256::zero() || shift >= U256::from(256) { + if value == U256::zero() || shift >= U256::from(256) { U256::zero() } else { let shift: u64 = shift.as_u64(); value << shift as usize - }; - - ret + } } #[inline] pub fn shr(shift: U256, value: U256) -> U256 { - let ret = if value == U256::zero() || shift >= U256::from(256) { + if value == U256::zero() || shift >= U256::from(256) { U256::zero() } else { let shift: u64 = shift.as_u64(); value >> shift as usize - }; - - ret + } } #[inline] pub fn sar(shift: U256, value: U256) -> U256 { let value = I256::from(value); - let ret = if value == I256::zero() || shift >= U256::from(256) { + if value == I256::zero() || shift >= U256::from(256) { let I256(sign, _) = value; match sign { // value is 0 or >=1, pushing 0 @@ -104,7 +100,5 @@ pub fn sar(shift: U256, value: U256) -> U256 { I256(Sign::Minus, shifted).into() } } - }; - - ret + } } diff --git a/core/src/eval/misc.rs b/core/src/eval/misc.rs index d365ca583..db9306e48 100644 --- a/core/src/eval/misc.rs +++ b/core/src/eval/misc.rs @@ -29,6 +29,7 @@ pub fn calldataload(state: &mut Machine) -> Control { pop_u256!(state, index); let mut load = [0u8; 32]; + #[allow(clippy::needless_range_loop)] for i in 0..32 { if let Some(p) = index.checked_add(U256::from(i)) { if p <= U256::from(usize::MAX) { @@ -146,7 +147,7 @@ pub fn pc(state: &mut Machine, position: usize) -> Control { #[inline] pub fn msize(state: &mut Machine) -> Control { - push_u256!(state, U256::from(state.memory.effective_len())); + push_u256!(state, state.memory.effective_len()); Control::Continue(1) } diff --git a/core/src/memory.rs b/core/src/memory.rs index 8a8f8b84b..57c22ec23 100644 --- a/core/src/memory.rs +++ b/core/src/memory.rs @@ -85,6 +85,7 @@ impl Memory { let mut ret = Vec::new(); ret.resize(size, 0); + #[allow(clippy::needless_range_loop)] for index in 0..size { let position = offset + index; if position >= self.data.len() { diff --git a/core/src/opcode.rs b/core/src/opcode.rs index 9665bd782..6f7e72df5 100644 --- a/core/src/opcode.rs +++ b/core/src/opcode.rs @@ -242,7 +242,7 @@ impl Opcode { /// Whether the opcode is a push opcode. pub fn is_push(&self) -> Option { let value = self.0; - if value >= 0x60 && value <= 0x7f { + if (0x60..=0x7f).contains(&value) { Some(value - 0x60 + 1) } else { None diff --git a/core/src/stack.rs b/core/src/stack.rs index 3a9560933..893af517c 100644 --- a/core/src/stack.rs +++ b/core/src/stack.rs @@ -30,6 +30,12 @@ impl Stack { self.data.len() } + #[inline] + /// Whether the stack is empty. + pub fn is_empty(&self) -> bool { + self.data.is_empty() + } + #[inline] /// Stack data. pub fn data(&self) -> &Vec { diff --git a/core/src/utils.rs b/core/src/utils.rs index 7c2cdbf5e..c78dd49e6 100644 --- a/core/src/utils.rs +++ b/core/src/utils.rs @@ -57,26 +57,28 @@ impl Default for I256 { I256::zero() } } + impl From for I256 { fn from(val: U256) -> I256 { if val == U256::zero() { I256::zero() - } else if val & SIGN_BIT_MASK.into() == val { + } else if val & SIGN_BIT_MASK == val { I256(Sign::Plus, val) } else { I256(Sign::Minus, !val + U256::from(1u64)) } } } -impl Into for I256 { - fn into(self) -> U256 { - let sign = self.0; + +impl From for U256 { + fn from(value: I256) -> U256 { + let sign = value.0; if sign == Sign::NoSign { U256::zero() } else if sign == Sign::Plus { - self.1 + value.1 } else { - !self.1 + U256::from(1u64) + !value.1 + U256::from(1u64) } } } @@ -93,7 +95,7 @@ impl Div for I256 { return I256::min_value(); } - let d = (self.1 / other.1) & SIGN_BIT_MASK.into(); + let d = (self.1 / other.1) & SIGN_BIT_MASK; if d == U256::zero() { return I256::zero(); @@ -117,7 +119,7 @@ impl Rem for I256 { type Output = I256; fn rem(self, other: I256) -> I256 { - let r = (self.1 % other.1) & SIGN_BIT_MASK.into(); + let r = (self.1 % other.1) & SIGN_BIT_MASK; if r == U256::zero() { return I256::zero(); diff --git a/gasometer/src/costs.rs b/gasometer/src/costs.rs index 756c0650c..e5691c67f 100644 --- a/gasometer/src/costs.rs +++ b/gasometer/src/costs.rs @@ -19,6 +19,7 @@ pub fn suicide_refund(already_removed: bool) -> i64 { } } +#[allow(clippy::collapsible_else_if)] pub fn sstore_refund(original: H256, current: H256, new: H256, config: &Config) -> i64 { if config.sstore_gas_metering { if current == new { @@ -28,6 +29,7 @@ pub fn sstore_refund(original: H256, current: H256, new: H256, config: &Config) config.refund_sstore_clears } else { let mut refund = 0; + if original != H256::default() { if current == H256::default() { refund -= config.refund_sstore_clears; @@ -192,24 +194,20 @@ pub fn sstore_cost( config: &Config, ) -> Result { if config.sstore_gas_metering { - if config.sstore_revert_under_stipend { - if gas < config.call_stipend { - return Err(ExitError::OutOfGas); - } + if config.sstore_revert_under_stipend && gas < config.call_stipend { + return Err(ExitError::OutOfGas); } Ok(if new == current { config.gas_sload - } else { - if original == current { - if original == H256::zero() { - config.gas_sstore_set - } else { - config.gas_sstore_reset - } + } else if original == current { + if original == H256::zero() { + config.gas_sstore_set } else { - config.gas_sload + config.gas_sstore_reset } + } else { + config.gas_sload }) } else { Ok(if current == H256::zero() && new != H256::zero() { diff --git a/gasometer/src/lib.rs b/gasometer/src/lib.rs index 7fcd93b8e..36ddc662a 100644 --- a/gasometer/src/lib.rs +++ b/gasometer/src/lib.rs @@ -406,6 +406,7 @@ pub fn static_opcode_cost(opcode: Opcode) -> Option { } /// Calculate the opcode cost. +#[allow(clippy::nonminimal_bool)] pub fn dynamic_opcode_cost( address: H160, opcode: Opcode, diff --git a/gasometer/src/utils.rs b/gasometer/src/utils.rs index 3a170ea35..9630b9ab9 100644 --- a/gasometer/src/utils.rs +++ b/gasometer/src/utils.rs @@ -16,5 +16,5 @@ pub fn log2floor(value: U256) -> u64 { } } } - return l; + l } diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index b69b10101..d8909f554 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -294,7 +294,7 @@ pub fn create(runtime: &mut Runtime, is_create2: bool, handler: &mut match reason { ExitReason::Succeed(_) => { - push!(runtime, create_address.into()); + push!(runtime, create_address); Control::Continue } ExitReason::Revert(_) => { @@ -318,7 +318,7 @@ pub fn create(runtime: &mut Runtime, is_create2: bool, handler: &mut } } -pub fn call<'config, H: Handler>( +pub fn call( runtime: &mut Runtime, scheme: CallScheme, handler: &mut H, @@ -383,13 +383,13 @@ pub fn call<'config, H: Handler>( Some(Transfer { source: runtime.context.address, target: to.into(), - value: value.into(), + value, }) } else if scheme == CallScheme::CallCode { Some(Transfer { source: runtime.context.address, target: runtime.context.address, - value: value.into(), + value, }) } else { None diff --git a/src/backend/memory.rs b/src/backend/memory.rs index 083b710bd..6bae2af2a 100644 --- a/src/backend/memory.rs +++ b/src/backend/memory.rs @@ -129,8 +129,8 @@ impl<'vicinity> Backend for MemoryBackend<'vicinity> { fn storage(&self, address: H160, index: H256) -> H256 { self.state .get(&address) - .map(|v| v.storage.get(&index).cloned().unwrap_or(H256::default())) - .unwrap_or(H256::default()) + .map(|v| v.storage.get(&index).cloned().unwrap_or_default()) + .unwrap_or_default() } fn original_storage(&self, address: H160, index: H256) -> Option { @@ -155,7 +155,7 @@ impl<'vicinity> ApplyBackend for MemoryBackend<'vicinity> { reset_storage, } => { let is_empty = { - let account = self.state.entry(address).or_insert(Default::default()); + let account = self.state.entry(address).or_insert_with(Default::default); account.balance = basic.balance; account.nonce = basic.nonce; if let Some(code) = code { @@ -170,7 +170,7 @@ impl<'vicinity> ApplyBackend for MemoryBackend<'vicinity> { .storage .iter() .filter(|(_, v)| v == &&H256::default()) - .map(|(k, _)| k.clone()) + .map(|(k, _)| *k) .collect::>(); for zero in zeros { @@ -187,7 +187,7 @@ impl<'vicinity> ApplyBackend for MemoryBackend<'vicinity> { account.balance == U256::zero() && account.nonce == U256::zero() - && account.code.len() == 0 + && account.code.is_empty() }; if is_empty && delete_empty { diff --git a/src/executor/stack/mod.rs b/src/executor/stack/mod.rs index a9029d18d..f60f7b9e5 100644 --- a/src/executor/stack/mod.rs +++ b/src/executor/stack/mod.rs @@ -513,6 +513,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> { } } + #[allow(clippy::too_many_arguments)] fn call_inner( &mut self, code_address: H160, diff --git a/src/executor/stack/state.rs b/src/executor/stack/state.rs index 151bf38b0..9b476fdd2 100644 --- a/src/executor/stack/state.rs +++ b/src/executor/stack/state.rs @@ -197,7 +197,7 @@ impl<'config> MemoryStackSubstate<'config> { return Some( account.basic.balance == U256::zero() && account.basic.nonce == U256::zero() - && code.len() == 0, + && code.is_empty(), ); } } @@ -249,6 +249,7 @@ impl<'config> MemoryStackSubstate<'config> { false } + #[allow(clippy::map_entry)] fn account_mut(&mut self, address: H160, backend: &B) -> &mut MemoryStackAccount { if !self.accounts.contains_key(&address) { let account = self