Skip to content

Commit

Permalink
refactor: replace U256 with u64 in BLOCKHASH (bluealloy#1505)
Browse files Browse the repository at this point in the history
* replace U256 -> u64

* cargo fmt

* refactor type cast to prevent panic
  • Loading branch information
TropicalDog17 authored Jun 21, 2024
1 parent 99367b1 commit 784a065
Show file tree
Hide file tree
Showing 12 changed files with 37 additions and 43 deletions.
2 changes: 1 addition & 1 deletion crates/interpreter/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub trait Host {
fn load_account(&mut self, address: Address) -> Option<LoadAccountResult>;

/// Get the block hash of the given block `number`.
fn block_hash(&mut self, number: U256) -> Option<B256>;
fn block_hash(&mut self, number: u64) -> Option<B256>;

/// Get balance of `address` and if the account is cold.
fn balance(&mut self, address: Address) -> Option<(U256, bool)>;
Expand Down
2 changes: 1 addition & 1 deletion crates/interpreter/src/host/dummy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl Host for DummyHost {
}

#[inline]
fn block_hash(&mut self, _number: U256) -> Option<B256> {
fn block_hash(&mut self, _number: u64) -> Option<B256> {
Some(B256::ZERO)
}

Expand Down
3 changes: 2 additions & 1 deletion crates/interpreter/src/instructions/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ pub fn blockhash<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, ho
gas!(interpreter, gas::BLOCKHASH);
pop_top!(interpreter, number);

let Some(hash) = host.block_hash(*number) else {
let number_u64 = as_u64_saturated!(number);
let Some(hash) = host.block_hash(number_u64) else {
interpreter.instruction_result = InstructionResult::FatalExternalError;
return;
};
Expand Down
6 changes: 3 additions & 3 deletions crates/primitives/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub trait Database {
fn storage(&mut self, address: Address, index: U256) -> Result<U256, Self::Error>;

/// Get block hash by block number.
fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error>;
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error>;
}

/// EVM database commit interface.
Expand Down Expand Up @@ -53,7 +53,7 @@ pub trait DatabaseRef {
fn storage_ref(&self, address: Address, index: U256) -> Result<U256, Self::Error>;

/// Get block hash by block number.
fn block_hash_ref(&self, number: U256) -> Result<B256, Self::Error>;
fn block_hash_ref(&self, number: u64) -> Result<B256, Self::Error>;
}

/// Wraps a [`DatabaseRef`] to provide a [`Database`] implementation.
Expand Down Expand Up @@ -86,7 +86,7 @@ impl<T: DatabaseRef> Database for WrapDatabaseRef<T> {
}

#[inline]
fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
self.0.block_hash_ref(number)
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/primitives/src/db/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl<S: State, BH: BlockHash> Database for DatabaseComponents<S, BH> {
.map_err(Self::Error::State)
}

fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
self.block_hash
.block_hash(number)
.map_err(Self::Error::BlockHash)
Expand All @@ -69,7 +69,7 @@ impl<S: StateRef, BH: BlockHashRef> DatabaseRef for DatabaseComponents<S, BH> {
.map_err(Self::Error::State)
}

fn block_hash_ref(&self, number: U256) -> Result<B256, Self::Error> {
fn block_hash_ref(&self, number: u64) -> Result<B256, Self::Error> {
self.block_hash
.block_hash(number)
.map_err(Self::Error::BlockHash)
Expand Down
10 changes: 5 additions & 5 deletions crates/primitives/src/db/components/block_hash.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! BlockHash database component from [`crate::db::Database`]
//! it is used inside [`crate::db::DatabaseComponents`]

use crate::{B256, U256};
use crate::B256;
use auto_impl::auto_impl;
use core::ops::Deref;
use std::sync::Arc;
Expand All @@ -11,15 +11,15 @@ pub trait BlockHash {
type Error;

/// Get block hash by block number
fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error>;
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error>;
}

#[auto_impl(&, &mut, Box, Rc, Arc)]
pub trait BlockHashRef {
type Error;

/// Get block hash by block number
fn block_hash(&self, number: U256) -> Result<B256, Self::Error>;
fn block_hash(&self, number: u64) -> Result<B256, Self::Error>;
}

impl<T> BlockHash for &T
Expand All @@ -28,7 +28,7 @@ where
{
type Error = <T as BlockHashRef>::Error;

fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
BlockHashRef::block_hash(*self, number)
}
}
Expand All @@ -39,7 +39,7 @@ where
{
type Error = <T as BlockHashRef>::Error;

fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
self.deref().block_hash(number)
}
}
4 changes: 2 additions & 2 deletions crates/revm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ impl<EXT, DB: Database> Host for Context<EXT, DB> {
&mut self.evm.env
}

fn block_hash(&mut self, number: U256) -> Option<B256> {
fn block_hash(&mut self, number: u64) -> Option<B256> {
let block_number = as_usize_saturated!(self.env().block.number);
let requested_number = as_usize_saturated!(number);
let requested_number = usize::try_from(number).unwrap_or(usize::MAX);

let Some(diff) = block_number.checked_sub(requested_number) else {
return Some(B256::ZERO);
Expand Down
2 changes: 1 addition & 1 deletion crates/revm/src/context/inner_evm_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<DB: Database> InnerEvmContext<DB> {

/// Fetch block hash from database.
#[inline]
pub fn block_hash(&mut self, number: U256) -> Result<B256, EVMError<DB::Error>> {
pub fn block_hash(&mut self, number: u64) -> Result<B256, EVMError<DB::Error>> {
self.db.block_hash(number).map_err(EVMError::Database)
}

Expand Down
10 changes: 5 additions & 5 deletions crates/revm/src/db/emptydb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<E> Database for EmptyDBTyped<E> {
}

#[inline]
fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
<Self as DatabaseRef>::block_hash_ref(self, number)
}
}
Expand All @@ -96,7 +96,7 @@ impl<E> DatabaseRef for EmptyDBTyped<E> {
}

#[inline]
fn block_hash_ref(&self, number: U256) -> Result<B256, Self::Error> {
fn block_hash_ref(&self, number: u64) -> Result<B256, Self::Error> {
Ok(keccak256(number.to_string().as_bytes()))
}
}
Expand All @@ -110,21 +110,21 @@ mod tests {
fn conform_block_hash_calculation() {
let db = EmptyDB::new();
assert_eq!(
db.block_hash_ref(U256::from(0)),
db.block_hash_ref(0u64),
Ok(b256!(
"044852b2a670ade5407e78fb2863c51de9fcb96542a07186fe3aeda6bb8a116d"
))
);

assert_eq!(
db.block_hash_ref(U256::from(1)),
db.block_hash_ref(1u64),
Ok(b256!(
"c89efdaa54c0f20c7adf612882df0950f5a951637e0307cdcb4c672f298b8bc6"
))
);

assert_eq!(
db.block_hash_ref(U256::from(100)),
db.block_hash_ref(100u64),
Ok(b256!(
"8c18210df0d9514f2d2e5d8ca7c100978219ee80d3968ad850ab5ead208287b3"
))
Expand Down
13 changes: 4 additions & 9 deletions crates/revm/src/db/ethersdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ethers_core::types::{Block, BlockId, TxHash, H160 as eH160, H256, U64 as eU6
use ethers_providers::Middleware;
use tokio::runtime::{Builder, Handle, RuntimeFlavor};

use crate::primitives::{AccountInfo, Address, Bytecode, B256, KECCAK_EMPTY, U256};
use crate::primitives::{AccountInfo, Address, Bytecode, B256, U256};
use crate::{Database, DatabaseRef};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -103,13 +103,8 @@ impl<M: Middleware> DatabaseRef for EthersDB<M> {
Ok(U256::from_be_bytes(slot_value.to_fixed_bytes()))
}

fn block_hash_ref(&self, number: U256) -> Result<B256, Self::Error> {
// saturate usize
if number > U256::from(u64::MAX) {
return Ok(KECCAK_EMPTY);
}
// We know number <= u64::MAX so unwrap is safe
let number = eU64::from(u64::try_from(number).unwrap());
fn block_hash_ref(&self, number: u64) -> Result<B256, Self::Error> {
let number = eU64::from(number);
let block: Option<Block<TxHash>> =
Self::block_on(self.client.get_block(BlockId::from(number)))?;
// If number is given, the block is supposed to be finalized so unwrap is safe too.
Expand All @@ -136,7 +131,7 @@ impl<M: Middleware> Database for EthersDB<M> {
}

#[inline]
fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
<Self as DatabaseRef>::block_hash_ref(self, number)
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/revm/src/db/in_memory_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ impl<ExtDB: DatabaseRef> Database for CacheDB<ExtDB> {
}
}

fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
match self.block_hashes.entry(number) {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
match self.block_hashes.entry(U256::from(number)) {
Entry::Occupied(entry) => Ok(*entry.get()),
Entry::Vacant(entry) => {
let hash = self.db.block_hash_ref(number)?;
Expand Down Expand Up @@ -282,8 +282,8 @@ impl<ExtDB: DatabaseRef> DatabaseRef for CacheDB<ExtDB> {
}
}

fn block_hash_ref(&self, number: U256) -> Result<B256, Self::Error> {
match self.block_hashes.get(&number) {
fn block_hash_ref(&self, number: u64) -> Result<B256, Self::Error> {
match self.block_hashes.get(&U256::from(number)) {
Some(entry) => Ok(*entry),
None => self.db.block_hash_ref(number),
}
Expand Down Expand Up @@ -403,7 +403,7 @@ impl Database for BenchmarkDB {
}

// History related
fn block_hash(&mut self, _number: U256) -> Result<B256, Self::Error> {
fn block_hash(&mut self, _number: u64) -> Result<B256, Self::Error> {
Ok(B256::default())
}
}
Expand Down
14 changes: 6 additions & 8 deletions crates/revm/src/db/states/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,16 +268,14 @@ impl<DB: Database> Database for State<DB> {
}
}

fn block_hash(&mut self, number: U256) -> Result<B256, Self::Error> {
// block number is never bigger then u64::MAX.
let u64num: u64 = number.to();
match self.block_hashes.entry(u64num) {
fn block_hash(&mut self, number: u64) -> Result<B256, Self::Error> {
match self.block_hashes.entry(number) {
btree_map::Entry::Occupied(entry) => Ok(*entry.get()),
btree_map::Entry::Vacant(entry) => {
let ret = *entry.insert(self.database.block_hash(number)?);

// prune all hashes that are older then BLOCK_HASH_HISTORY
let last_block = u64num.saturating_sub(BLOCK_HASH_HISTORY as u64);
let last_block = number.saturating_sub(BLOCK_HASH_HISTORY as u64);
while let Some(entry) = self.block_hashes.first_entry() {
if *entry.key() < last_block {
entry.remove();
Expand Down Expand Up @@ -311,8 +309,8 @@ mod tests {
#[test]
fn block_hash_cache() {
let mut state = State::builder().build();
state.block_hash(U256::from(1)).unwrap();
state.block_hash(U256::from(2)).unwrap();
state.block_hash(1u64).unwrap();
state.block_hash(2u64).unwrap();

let test_number = BLOCK_HASH_HISTORY as u64 + 2;

Expand All @@ -325,7 +323,7 @@ mod tests {
BTreeMap::from([(1, block1_hash), (2, block2_hash)])
);

state.block_hash(U256::from(test_number)).unwrap();
state.block_hash(test_number).unwrap();
assert_eq!(
state.block_hashes,
BTreeMap::from([(test_number, block_test_hash), (2, block2_hash)])
Expand Down

0 comments on commit 784a065

Please sign in to comment.