Merge branch 'main' into edr/change-edr-npm-org

Author: fvictorio

.changeset/cold-cobras-carry.md

@@ -197,6 +197,13 @@ jobs:
 
       - uses: Swatinem/rust-cache@v2
 
+      - name: Cache EDR RPC cache
+        uses: actions/cache@v2
+        with:
+          path: |
+            **/edr-cache
+          key: edr-rs-rpc-cache-v1
+
       - name: Run cargo test
         uses: actions-rs/cargo@v1
         env:

.changeset/proud-peas-sort.md

@@ -33,6 +33,7 @@ url = "2.4.1"
 [dev-dependencies]
 anyhow = "1.0.75"
 assert-json-diff = "2.0.2"
+edr_defaults = { version = "0.2.0-dev", path = "../edr_defaults" }
 lazy_static = "1.4.0"
 mockito = { version = "1.0.2", default-features = false }
 paste = { version = "1.0.14", default-features = false }

.github/workflows/hardhat-core-ci.yml

@@ -1,13 +1,6 @@
 #[cfg(feature = "test-remote")]
 mod remote {
-    use lazy_static::lazy_static;
     use serial_test::serial;
-    use tempfile::TempDir;
-
-    lazy_static! {
-        // Use same cache dir for all tests
-        static ref CACHE_DIR: TempDir = TempDir::new().unwrap();
-    }
 
     macro_rules! impl_test_remote_block_receipt_root {
         ($(
@@ -21,7 +14,7 @@ mod remote {
                         use edr_eth::{remote::{RpcClient, PreEip1898BlockSpec}, trie::ordered_trie_root};
                         use edr_test_utils::env::get_alchemy_url;
 
-                        let client = RpcClient::new(&get_alchemy_url(), CACHE_DIR.path().into(), None).expect("url ok");
+                        let client = RpcClient::new(&get_alchemy_url(), edr_defaults::CACHE_DIR.into(), None).expect("url ok");
 
                         let block = client
                             .get_block_by_number_with_transaction_data(PreEip1898BlockSpec::Number($block_number))

Cargo.lock

@@ -34,6 +34,9 @@ pub enum BlockchainError {
     /// Forked blockchain error
     #[error(transparent)]
     Forked(#[from] ForkedBlockchainError),
+    /// An error that occurs when trying to insert a block into storage.
+    #[error(transparent)]
+    Insert(#[from] storage::InsertError),
     /// Invalid block number
     #[error("Invalid block number: {actual}. Expected: {expected}.")]
     InvalidBlockNumber {

crates/edr_eth/Cargo.toml

@@ -16,8 +16,9 @@ use revm::{
 use tokio::runtime;
 
 use super::{
-    compute_state_at_block, remote::RemoteBlockchain, storage::ReservableSparseBlockchainStorage,
-    validate_next_block, Blockchain, BlockchainError, BlockchainMut,
+    compute_state_at_block, remote::RemoteBlockchain, storage,
+    storage::ReservableSparseBlockchainStorage, validate_next_block, Blockchain, BlockchainError,
+    BlockchainMut,
 };
 use crate::{
     state::{ForkState, StateDiff, StateError, StateOverride, SyncState},
@@ -60,6 +61,9 @@ pub enum ForkedBlockchainError {
     /// Remote blocks cannot be deleted
     #[error("Cannot delete remote block.")]
     CannotDeleteRemote,
+    /// An error that occurs when trying to insert a block into storage.
+    #[error(transparent)]
+    Insert(#[from] storage::InsertError),
     /// Rpc client error
     #[error(transparent)]
     RpcClient(#[from] RpcClientError),
@@ -194,7 +198,7 @@ impl BlockHashRef for ForkedBlockchain {
             .map(|block| Ok(*block.hash()))?
         } else {
             self.local_storage
-                .block_by_number(number)
+                .block_by_number(number)?
                 .map(|block| *block.hash())
                 .ok_or(BlockchainError::UnknownBlockNumber)
         }
@@ -235,7 +239,7 @@ impl Blockchain for ForkedBlockchain {
             })
             .map(|block| Ok(Some(block)))?
         } else {
-            Ok(self.local_storage.block_by_number(number))
+            Ok(self.local_storage.block_by_number(number)?)
         }
     }
 
@@ -271,7 +275,7 @@ impl Blockchain for ForkedBlockchain {
         if self.fork_block_number < last_block_number {
             Ok(self
                 .local_storage
-                .block_by_number(last_block_number)
+                .block_by_number(last_block_number)?
                 .expect("Block must exist since block number is less than the last block number"))
         } else {
             Ok(tokio::task::block_in_place(move || {
@@ -467,12 +471,9 @@ impl BlockchainMut for ForkedBlockchain {
 
         let total_difficulty = previous_total_difficulty + block.header().difficulty;
 
-        // SAFETY: The block number is guaranteed to be unique, so the block hash must
-        // be too.
-        let block = unsafe {
-            self.local_storage
-                .insert_block_unchecked(block, state_diff, total_difficulty)
-        };
+        let block = self
+            .local_storage
+            .insert_block(block, state_diff, total_difficulty)?;
 
         Ok(BlockAndTotalDifficulty {
             block: block.clone(),

crates/edr_eth/tests/receipt.rs

@@ -222,7 +222,7 @@ impl Blockchain for LocalBlockchain {
         number: u64,
     ) -> Result<Option<Arc<dyn SyncBlock<Error = Self::BlockchainError>>>, Self::BlockchainError>
     {
-        Ok(self.storage.block_by_number(number))
+        Ok(self.storage.block_by_number(number)?)
     }
 
     #[cfg_attr(feature = "tracing", tracing::instrument(skip_all))]
@@ -245,7 +245,7 @@ impl Blockchain for LocalBlockchain {
     ) -> Result<Arc<dyn SyncBlock<Error = Self::BlockchainError>>, Self::BlockchainError> {
         Ok(self
             .storage
-            .block_by_number(self.storage.last_block_number())
+            .block_by_number(self.storage.last_block_number())?
             .expect("Block must exist"))
     }
 
@@ -331,12 +331,9 @@ impl BlockchainMut for LocalBlockchain {
 
         let total_difficulty = previous_total_difficulty + block.header().difficulty;
 
-        // SAFETY: The block number is guaranteed to be unique, so the block hash must
-        // be too.
-        let block = unsafe {
-            self.storage
-                .insert_block_unchecked(block, state_diff, total_difficulty)
-        };
+        let block = self
+            .storage
+            .insert_block(block, state_diff, total_difficulty)?;
 
         Ok(BlockAndTotalDifficulty {
             block: block.clone(),
@@ -390,7 +387,7 @@ impl BlockHashRef for LocalBlockchain {
             u64::try_from(number).map_err(|_error| BlockchainError::BlockNumberTooLarge)?;
 
         self.storage
-            .block_by_number(number)
+            .block_by_number(number)?
             .map(|block| *block.hash())
             .ok_or(BlockchainError::UnknownBlockNumber)
     }

crates/edr_evm/src/blockchain.rs

@@ -171,8 +171,7 @@ impl<BlockT: Block + Clone + From<RemoteBlock>, const FORCE_CACHING: bool>
         {
             Ok(Some({
                 let mut cache = RwLockUpgradableReadGuard::upgrade(cache).await;
-                // SAFETY: the receipt with this hash didn't exist yet, so it must be unique
-                unsafe { cache.insert_receipt_unchecked(receipt) }.clone()
+                cache.insert_receipt(receipt)?.clone()
             }))
         } else {
             Ok(None)
@@ -235,8 +234,7 @@ impl<BlockT: Block + Clone + From<RemoteBlock>, const FORCE_CACHING: bool>
         if is_cacheable {
             let mut remote_cache = RwLockUpgradableReadGuard::upgrade(cache).await;
 
-            // SAFETY: the block with this number didn't exist yet, so it must be unique
-            Ok(unsafe { remote_cache.insert_block_unchecked(block, total_difficulty) }.clone())
+            Ok(remote_cache.insert_block(block, total_difficulty)?.clone())
         } else {
             Ok(block)
         }

crates/edr_evm/src/blockchain/forked.rs

@@ -16,6 +16,12 @@ pub enum InsertError {
         /// The block's number
         block_number: u64,
     },
+    /// Receipt already exists
+    #[error("A receipt with transaction hash {transaction_hash} already exists.")]
+    DuplicateReceipt {
+        /// Transaction hash of duplicated receipt
+        transaction_hash: B256,
+    },
     /// Transaction already exists
     #[error("A transaction with hash {hash} already exists.")]
     DuplicateTransaction {

crates/edr_evm/src/blockchain/local.rs

@@ -6,7 +6,7 @@ use edr_eth::{
 use parking_lot::{RwLock, RwLockUpgradableReadGuard, RwLockWriteGuard};
 use revm::primitives::{HashMap, HashSet};
 
-use super::{sparse, SparseBlockchainStorage};
+use super::{sparse, InsertError, SparseBlockchainStorage};
 use crate::{state::StateDiff, Block, LocalBlock};
 
 /// A reservation for a sequence of blocks that have not yet been inserted into
@@ -225,24 +225,21 @@ impl<BlockT: Block + Clone> ReservableSparseBlockchainStorage<BlockT> {
 impl<BlockT: Block + Clone + From<LocalBlock>> ReservableSparseBlockchainStorage<BlockT> {
     /// Retrieves the block by number, if it exists.
     #[cfg_attr(feature = "tracing", tracing::instrument(skip_all))]
-    pub fn block_by_number(&self, number: u64) -> Option<BlockT> {
-        self.try_fulfilling_reservation(number)
-            .or_else(|| self.storage.read().block_by_number(number).cloned())
+    pub fn block_by_number(&self, number: u64) -> Result<Option<BlockT>, InsertError> {
+        Ok(self
+            .try_fulfilling_reservation(number)?
+            .or_else(|| self.storage.read().block_by_number(number).cloned()))
     }
 
-    /// Inserts a block without checking its validity.
-    ///
-    /// # Safety
-    ///
-    /// Ensure that the instance does not contain a block with the same hash or
-    /// number, nor any transactions with the same hash.
+    /// Insert a block into the storage. Errors if a block with the same hash or
+    /// number already exists.
     #[cfg_attr(feature = "tracing", tracing::instrument(skip_all))]
-    pub unsafe fn insert_block_unchecked(
+    pub fn insert_block(
         &mut self,
         block: LocalBlock,
         state_diff: StateDiff,
         total_difficulty: U256,
-    ) -> &BlockT {
+    ) -> Result<&BlockT, InsertError> {
         self.last_block_number = block.header().number;
         self.number_to_diff_index
             .insert(self.last_block_number, self.state_diffs.len());
@@ -252,17 +249,13 @@ impl<BlockT: Block + Clone + From<LocalBlock>> ReservableSparseBlockchainStorage
         let receipts: Vec<_> = block.transaction_receipts().to_vec();
         let block = BlockT::from(block);
 
-        self.storage
-            .get_mut()
-            .insert_receipts_unchecked(receipts, block.clone());
+        self.storage.get_mut().insert_receipts(receipts)?;
 
-        self.storage
-            .get_mut()
-            .insert_block_unchecked(block, total_difficulty)
+        self.storage.get_mut().insert_block(block, total_difficulty)
     }
 
     #[cfg_attr(feature = "tracing", tracing::instrument(skip_all))]
-    fn try_fulfilling_reservation(&self, block_number: u64) -> Option<BlockT> {
+    fn try_fulfilling_reservation(&self, block_number: u64) -> Result<Option<BlockT>, InsertError> {
         let reservations = self.reservations.upgradable_read();
 
         reservations
@@ -321,17 +314,14 @@ impl<BlockT: Block + Clone + From<LocalBlock>> ReservableSparseBlockchainStorage
                     },
                 );
 
-                let mut storage = RwLockUpgradableReadGuard::upgrade(storage);
-
-                // SAFETY: Reservations are guaranteed to not overlap with other reservations or
-                // blocks, so the generated block must have a unique block
-                // number and thus hash.
-                unsafe {
-                    storage
-                        .insert_block_unchecked(block.into(), reservation.previous_total_difficulty)
+                {
+                    let mut storage = RwLockUpgradableReadGuard::upgrade(storage);
+                    Ok(storage
+                        .insert_block(block.into(), reservation.previous_total_difficulty)?
+                        .clone())
                 }
-                .clone()
             })
+            .transpose()
     }
 }