Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: use is_zero for U256 and B256 #1638

Merged
merged 5 commits into from
Jul 18, 2024
Merged

refactor: use is_zero for U256 and B256 #1638

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
39ae687
refactor: use is_zero for U256 and B256
tcoratger Jul 17, 2024
6f68516
fmt
tcoratger Jul 17, 2024
530d0fc
Update crates/interpreter/src/instructions/arithmetic.rs
tcoratger Jul 17, 2024
ee0cd24
fix deref
tcoratger Jul 17, 2024
2c80843
move import
tcoratger Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bins/revme/src/cmd/statetest/merkle_trie.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl TrieAccount {
root_hash: sec_trie_root::<KeccakHasher, _, _, _>(
acc.storage
.iter()
.filter(|(_k, &v)| v != U256::ZERO)
.filter(|(_k, &v)| !v.is_zero())
.map(|(k, v)| (k.to_be_bytes::<32>(), alloy_rlp::encode_fixed_size(v))),
),
code_hash: acc.info.code_hash,
Expand Down
18 changes: 9 additions & 9 deletions crates/interpreter/src/gas/calc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ pub fn sstore_refund(spec_id: SpecId, original: U256, current: U256, new: U256)
if current == new {
0
} else {
if original == current && new == U256::ZERO {
if original == current && new.is_zero() {
sstore_clears_schedule
} else {
let mut refund = 0;

if original != U256::ZERO {
if current == U256::ZERO {
if !original.is_zero() {
if current.is_zero() {
refund -= sstore_clears_schedule;
} else if new == U256::ZERO {
} else if new.is_zero() {
refund += sstore_clears_schedule;
}
}
Expand All @@ -48,7 +48,7 @@ pub fn sstore_refund(spec_id: SpecId, original: U256, current: U256, new: U256)
} else {
(SSTORE_RESET, sload_cost(spec_id, false))
};
if original == U256::ZERO {
if original.is_zero() {
refund += (SSTORE_SET - gas_sload) as i64;
} else {
refund += (gas_sstore_reset - gas_sload) as i64;
Expand All @@ -59,7 +59,7 @@ pub fn sstore_refund(spec_id: SpecId, original: U256, current: U256, new: U256)
}
}
} else {
if current != U256::ZERO && new == U256::ZERO {
if !current.is_zero() && new.is_zero() {
REFUND_SSTORE_CLEARS
} else {
0
Expand Down Expand Up @@ -99,7 +99,7 @@ const fn log2floor(value: U256) -> u64 {
/// `EXP` opcode cost calculation.
#[inline]
pub fn exp_cost(spec_id: SpecId, power: U256) -> Option<u64> {
if power == U256::ZERO {
if power.is_zero() {
Some(EXP)
} else {
// EIP-160: EXP cost increase
Expand Down Expand Up @@ -230,7 +230,7 @@ fn istanbul_sstore_cost<const SLOAD_GAS: u64, const SSTORE_RESET_GAS: u64>(
) -> u64 {
if new == current {
SLOAD_GAS
} else if original == current && original == U256::ZERO {
} else if original == current && original.is_zero() {
SSTORE_SET
} else if original == current {
SSTORE_RESET_GAS
Expand All @@ -242,7 +242,7 @@ fn istanbul_sstore_cost<const SLOAD_GAS: u64, const SSTORE_RESET_GAS: u64>(
/// Frontier sstore cost just had two cases set and reset values.
#[inline]
fn frontier_sstore_cost(current: U256, new: U256) -> u64 {
if current == U256::ZERO && new != U256::ZERO {
if current.is_zero() && !new.is_zero() {
SSTORE_SET
} else {
SSTORE_RESET
Expand Down
4 changes: 2 additions & 2 deletions crates/interpreter/src/instructions/arithmetic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub fn sub<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
pub fn div<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
gas!(interpreter, gas::LOW);
pop_top!(interpreter, op1, op2);
if *op2 != U256::ZERO {
if !(*op2).is_zero() {
tcoratger marked this conversation as resolved.
Show resolved Hide resolved
*op2 = op1.wrapping_div(*op2);
}
}
Expand All @@ -40,7 +40,7 @@ pub fn sdiv<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
pub fn rem<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
gas!(interpreter, gas::LOW);
pop_top!(interpreter, op1, op2);
if *op2 != U256::ZERO {
if !(*op2).is_zero() {
*op2 = op1.wrapping_rem(*op2);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/interpreter/src/instructions/bitwise.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn eq<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
pub fn iszero<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
gas!(interpreter, gas::VERYLOW);
pop_top!(interpreter, op1);
*op1 = U256::from(*op1 == U256::ZERO);
*op1 = U256::from((*op1).is_zero());
}

pub fn bitand<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
Expand Down
8 changes: 4 additions & 4 deletions crates/interpreter/src/instructions/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ pub fn extcall<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, host
};

pop!(interpreter, value);
let has_transfer = value != U256::ZERO;
let has_transfer = !value.is_zero();
if interpreter.is_static && has_transfer {
interpreter.instruction_result = InstructionResult::CallNotAllowedInsideStatic;
return;
Expand Down Expand Up @@ -406,7 +406,7 @@ pub fn call<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, host: &
let local_gas_limit = u64::try_from(local_gas_limit).unwrap_or(u64::MAX);

pop!(interpreter, value);
let has_transfer = value != U256::ZERO;
let has_transfer = !value.is_zero();
if interpreter.is_static && has_transfer {
interpreter.instruction_result = InstructionResult::CallNotAllowedInsideStatic;
return;
Expand Down Expand Up @@ -474,7 +474,7 @@ pub fn call_code<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, ho
let Some(mut gas_limit) = calc_call_gas::<SPEC>(
interpreter,
is_cold,
value != U256::ZERO,
!value.is_zero(),
false,
local_gas_limit,
) else {
Expand All @@ -484,7 +484,7 @@ pub fn call_code<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, ho
gas!(interpreter, gas_limit);

// add call stipend if there is value to be transferred.
if value != U256::ZERO {
if !value.is_zero() {
gas_limit = gas_limit.saturating_add(gas::CALL_STIPEND);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/interpreter/src/instructions/control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub fn jump<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
pub fn jumpi<H: Host + ?Sized>(interpreter: &mut Interpreter, _host: &mut H) {
gas!(interpreter, gas::HIGH);
pop!(interpreter, target, cond);
if cond != U256::ZERO {
if !cond.is_zero() {
jump_inner(interpreter, target);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/interpreter/src/instructions/i256.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn i256_sign(val: &U256) -> Sign {
Sign::Minus
} else {
// SAFETY: false == 0 == Zero, true == 1 == Plus
unsafe { core::mem::transmute::<bool, Sign>(*val != U256::ZERO) }
unsafe { core::mem::transmute::<bool, Sign>(!(*val).is_zero()) }
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/precompile/src/modexp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn berlin_run(input: &Bytes, gas_limit: u64) -> PrecompileResult {
pub fn calculate_iteration_count(exp_length: u64, exp_highp: &U256) -> u64 {
let mut iteration_count: u64 = 0;

if exp_length <= 32 && *exp_highp == U256::ZERO {
if exp_length <= 32 && (*exp_highp).is_zero() {
iteration_count = 0;
} else if exp_length <= 32 {
iteration_count = exp_highp.bit_len() as u64 - 1;
Expand Down
4 changes: 2 additions & 2 deletions crates/primitives/src/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ impl AccountInfo {
/// - balance is zero
/// - nonce is zero
pub fn is_empty(&self) -> bool {
let code_empty = self.is_empty_code_hash() || self.code_hash == B256::ZERO;
code_empty && self.balance == U256::ZERO && self.nonce == 0
let code_empty = self.is_empty_code_hash() || self.code_hash.is_zero();
code_empty && self.balance.is_zero() && self.nonce == 0
}

/// Returns `true` if the account is not empty.
Expand Down
6 changes: 4 additions & 2 deletions crates/revm/src/context/evm_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use revm_interpreter::CallValue;
use revm_precompile::PrecompileErrors;

use super::inner_evm_context::InnerEvmContext;
#[cfg(any(test, feature = "test-utils"))]
use crate::primitives::U256;
use crate::{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did this need to move?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is not used anymore in the main code but still being used under this cfg so that if we don't move it, it will trigger a clippy lint warning

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would move it inside mod test-utils as it is only used there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also used in mod tests so that if I move the import inside mod test-utils, I've to do another equivalent import inside mod tests too. But as I just saw that it was done like that for other similar imports, I just changed the position, tell me what you think?

db::Database,
interpreter::{
Expand All @@ -11,7 +13,7 @@ use crate::{
primitives::{
keccak256, Address, Bytecode, Bytes, CreateScheme, EVMError, Env, Eof,
SpecId::{self, *},
B256, EOF_MAGIC_BYTES, U256,
B256, EOF_MAGIC_BYTES,
},
ContextPrecompiles, FrameOrResult, CALL_STACK_LIMIT,
};
Expand Down Expand Up @@ -185,7 +187,7 @@ impl<DB: Database> EvmContext<DB> {
// Touch address. For "EIP-158 State Clear", this will erase empty accounts.
match inputs.value {
// if transfer value is zero, load account and force the touch.
CallValue::Transfer(value) if value == U256::ZERO => {
CallValue::Transfer(value) if value.is_zero() => {
self.load_account(inputs.target_address)?;
self.journaled_state.touch(&inputs.target_address);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/revm/src/db/in_memory_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl<ExtDB> CacheDB<ExtDB> {
.or_insert_with(|| code.clone());
}
}
if account.code_hash == B256::ZERO {
if account.code_hash.is_zero() {
account.code_hash = KECCAK_EMPTY;
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/revm/src/db/states/bundle_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ impl BundleState {
for (key, slot) in account.storage {
// If storage was destroyed that means that storage was wiped.
// In that case we need to check if present storage value is different then ZERO.
let destroyed_and_not_zero = was_destroyed && slot.present_value != U256::ZERO;
let destroyed_and_not_zero = was_destroyed && !slot.present_value.is_zero();

// If account is not destroyed check if original values was changed,
// so we can update it.
Expand Down
6 changes: 3 additions & 3 deletions crates/revm/src/journaled_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl JournaledState {
had_value,
} => {
let tkey = (address, key);
if had_value == U256::ZERO {
if had_value.is_zero() {
// if previous value is zero, remove it
transient_storage.remove(&tkey);
} else {
Expand Down Expand Up @@ -525,7 +525,7 @@ impl JournaledState {
};

Ok(SelfDestructResult {
had_value: balance != U256::ZERO,
had_value: !balance.is_zero(),
is_cold: load_result.is_cold,
target_exists: !load_result.is_empty,
previously_destroyed,
Expand Down Expand Up @@ -762,7 +762,7 @@ impl JournaledState {
/// EIP-1153: Transient storage opcodes
#[inline]
pub fn tstore(&mut self, address: Address, key: U256, new: U256) {
let had_value = if new == U256::ZERO {
let had_value = if new.is_zero() {
// if new values is zero, remove entry from transient storage.
// if previous values was some insert it inside journal.
// If it is none nothing should be inserted.
Expand Down
2 changes: 1 addition & 1 deletion crates/revm/src/optimism/l1block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl L1BlockInfo {

// Check if the L1 fee scalars are empty. If so, we use the Bedrock cost function. The L1 fee overhead is
// only necessary if `empty_scalars` is true, as it was deprecated in Ecotone.
let empty_scalars = l1_blob_base_fee == U256::ZERO
let empty_scalars = l1_blob_base_fee.is_zero()
&& l1_fee_scalars[BASE_FEE_SCALAR_OFFSET..BLOB_BASE_FEE_SCALAR_OFFSET + 4]
== EMPTY_SCALARS;
let l1_fee_overhead = empty_scalars
Expand Down
Loading