fix: behavior when pending block is not present in db (#258)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Next release
 
+- fix: pending block must always be returned in rpc even if none is in db
 - fix: fixed the starting block arg with an ignore_block_order argument
 - docs: fixed Docker Compose instructions
 - fix: removed unused dependencies with udeps and machete

crates/client/db/src/block_db.rs

@@ -1,4 +1,8 @@
+use crate::db_block_id::{DbBlockId, DbBlockIdResolvable};
+use crate::MadaraStorageError;
+use crate::{Column, DatabaseExt, MadaraBackend, WriteBatchWithTransaction};
 use anyhow::Context;
+use mp_block::header::{GasPrices, PendingHeader};
 use mp_block::{
     BlockId, BlockTag, MadaraBlock, MadaraBlockInfo, MadaraBlockInner, MadaraMaybePendingBlock,
     MadaraMaybePendingBlockInfo, MadaraPendingBlock, MadaraPendingBlockInfo,
@@ -8,10 +12,6 @@ use rocksdb::WriteOptions;
 use starknet_api::core::ChainId;
 use starknet_types_core::felt::Felt;
 
-use crate::db_block_id::{DbBlockId, DbBlockIdResolvable};
-use crate::MadaraStorageError;
-use crate::{Column, DatabaseExt, MadaraBackend, WriteBatchWithTransaction};
-
 type Result<T, E = MadaraStorageError> = std::result::Result<T, E>;
 
 #[derive(serde::Serialize, serde::Deserialize, Debug, Clone)]
@@ -110,30 +110,79 @@ impl MadaraBackend {
         Ok(Some(res))
     }
 
-    fn get_pending_block_info(&self) -> Result<Option<MadaraPendingBlockInfo>> {
+    // Pending block quirk: We should act as if there is always a pending block in db, to match
+    //  juno and pathfinder's handling of pending blocks.
+
+    fn get_pending_block_info(&self) -> Result<MadaraPendingBlockInfo> {
         let col = self.db.get_column(Column::BlockStorageMeta);
-        let Some(res) = self.db.get_cf(&col, ROW_PENDING_INFO)? else { return Ok(None) };
+        let Some(res) = self.db.get_cf(&col, ROW_PENDING_INFO)? else {
+            // See pending block quirk
+
+            let Some(latest_block_id) = self.get_latest_block_n()? else {
+                // Second quirk: if there is not even a genesis block in db, make up the gas prices and everything else
+                return Ok(MadaraPendingBlockInfo {
+                    header: PendingHeader {
+                        parent_block_hash: Felt::ZERO,
+                        // Sequencer address is ZERO for chains where we don't produce blocks. This means that trying to simulate/trace a transaction on Pending when
+                        // genesis has not been loaded yet will return an error. That probably fine because the ERC20 fee contracts are not even deployed yet - it
+                        // will error somewhere else anyway.
+                        sequencer_address: **self.chain_config().sequencer_address,
+                        block_timestamp: 0, // Junk timestamp: unix epoch
+                        protocol_version: self.chain_config.latest_protocol_version,
+                        l1_gas_price: GasPrices {
+                            eth_l1_gas_price: 1,
+                            strk_l1_gas_price: 1,
+                            eth_l1_data_gas_price: 1,
+                            strk_l1_data_gas_price: 1,
+                        },
+                        l1_da_mode: mp_block::header::L1DataAvailabilityMode::Blob,
+                    },
+                    tx_hashes: vec![],
+                });
+            };
+
+            let latest_block_info =
+                self.get_block_info_from_block_n(latest_block_id)?.ok_or(MadaraStorageError::MissingChainInfo)?;
+
+            return Ok(MadaraPendingBlockInfo {
+                header: PendingHeader {
+                    parent_block_hash: latest_block_info.block_hash,
+                    sequencer_address: latest_block_info.header.sequencer_address,
+                    block_timestamp: latest_block_info.header.block_timestamp,
+                    protocol_version: latest_block_info.header.protocol_version,
+                    l1_gas_price: latest_block_info.header.l1_gas_price.clone(),
+                    l1_da_mode: latest_block_info.header.l1_da_mode,
+                },
+                tx_hashes: vec![],
+            });
+        };
         let res = bincode::deserialize(&res)?;
-        Ok(Some(res))
+        Ok(res)
     }
 
-    fn get_pending_block_inner(&self) -> Result<Option<MadaraBlockInner>> {
+    fn get_pending_block_inner(&self) -> Result<MadaraBlockInner> {
         let col = self.db.get_column(Column::BlockStorageMeta);
-        let Some(res) = self.db.get_cf(&col, ROW_PENDING_INNER)? else { return Ok(None) };
+        let Some(res) = self.db.get_cf(&col, ROW_PENDING_INNER)? else {
+            // See pending block quirk
+            return Ok(MadaraBlockInner::default());
+        };
         let res = bincode::deserialize(&res)?;
-        Ok(Some(res))
+        Ok(res)
     }
 
-    pub fn get_l1_last_confirmed_block(&self) -> Result<Option<u64>> {
+    pub fn get_pending_block_state_update(&self) -> Result<StateDiff> {
         let col = self.db.get_column(Column::BlockStorageMeta);
-        let Some(res) = self.db.get_cf(&col, ROW_L1_LAST_CONFIRMED_BLOCK)? else { return Ok(None) };
+        let Some(res) = self.db.get_cf(&col, ROW_PENDING_STATE_UPDATE)? else {
+            // See pending block quirk
+            return Ok(StateDiff::default());
+        };
         let res = bincode::deserialize(&res)?;
-        Ok(Some(res))
+        Ok(res)
     }
 
-    pub fn get_pending_block_state_update(&self) -> Result<Option<StateDiff>> {
+    pub fn get_l1_last_confirmed_block(&self) -> Result<Option<u64>> {
         let col = self.db.get_column(Column::BlockStorageMeta);
-        let Some(res) = self.db.get_cf(&col, ROW_PENDING_STATE_UPDATE)? else { return Ok(None) };
+        let Some(res) = self.db.get_cf(&col, ROW_L1_LAST_CONFIRMED_BLOCK)? else { return Ok(None) };
         let res = bincode::deserialize(&res)?;
         Ok(Some(res))
     }
@@ -224,7 +273,7 @@ impl MadaraBackend {
 
     fn storage_to_info(&self, id: &DbBlockId) -> Result<Option<MadaraMaybePendingBlockInfo>> {
         match id {
-            DbBlockId::Pending => Ok(self.get_pending_block_info()?.map(MadaraMaybePendingBlockInfo::Pending)),
+            DbBlockId::Pending => Ok(Some(MadaraMaybePendingBlockInfo::Pending(self.get_pending_block_info()?))),
             DbBlockId::BlockN(block_n) => {
                 Ok(self.get_block_info_from_block_n(*block_n)?.map(MadaraMaybePendingBlockInfo::NotPending))
             }
@@ -233,7 +282,7 @@ impl MadaraBackend {
 
     fn storage_to_inner(&self, id: &DbBlockId) -> Result<Option<MadaraBlockInner>> {
         match id {
-            DbBlockId::Pending => self.get_pending_block_inner(),
+            DbBlockId::Pending => Ok(Some(self.get_pending_block_inner()?)),
             DbBlockId::BlockN(block_n) => self.get_block_inner_from_block_n(*block_n),
         }
     }
@@ -260,7 +309,7 @@ impl MadaraBackend {
     pub fn get_block_state_diff(&self, id: &impl DbBlockIdResolvable) -> Result<Option<StateDiff>> {
         let Some(ty) = id.resolve_db_block_id(self)? else { return Ok(None) };
         match ty {
-            DbBlockId::Pending => self.get_pending_block_state_update(),
+            DbBlockId::Pending => Ok(Some(self.get_pending_block_state_update()?)),
             DbBlockId::BlockN(block_n) => self.get_state_update(block_n),
         }
     }
@@ -299,7 +348,7 @@ impl MadaraBackend {
                 Ok(Some((info.into(), TxIndex(tx_index as _))))
             }
             None => {
-                let Some(info) = self.get_pending_block_info()? else { return Ok(None) };
+                let info = self.get_pending_block_info()?;
                 let Some(tx_index) = info.tx_hashes.iter().position(|a| a == tx_hash) else { return Ok(None) };
                 Ok(Some((info.into(), TxIndex(tx_index as _))))
             }
@@ -316,9 +365,9 @@ impl MadaraBackend {
                 Ok(Some((MadaraMaybePendingBlock { info: info.into(), inner }, TxIndex(tx_index as _))))
             }
             None => {
-                let Some(info) = self.get_pending_block_info()? else { return Ok(None) };
+                let info = self.get_pending_block_info()?;
                 let Some(tx_index) = info.tx_hashes.iter().position(|a| a == tx_hash) else { return Ok(None) };
-                let Some(inner) = self.get_pending_block_inner()? else { return Ok(None) };
+                let inner = self.get_pending_block_inner()?;
                 Ok(Some((MadaraMaybePendingBlock { info: info.into(), inner }, TxIndex(tx_index as _))))
             }
         }

crates/client/db/tests/test_block.rs

@@ -81,7 +81,7 @@ async fn test_store_pending_block() {
     let db = temp_db().await;
     let backend = db.backend();
 
-    assert!(backend.get_block(&BLOCK_ID_PENDING).unwrap().is_none());
+    assert!(backend.get_block(&BLOCK_ID_PENDING).unwrap().is_some()); // Pending block should always be there
 
     let block = pending_block_one();
     let state_diff = pending_state_diff_one();
@@ -106,7 +106,12 @@ async fn test_erase_pending_block() {
     backend.store_block(pending_block_one(), pending_state_diff_one(), vec![]).unwrap();
     backend.clear_pending_block().unwrap();
 
-    assert!(backend.get_block(&BLOCK_ID_PENDING).unwrap().is_none());
+    assert!(backend.get_block(&BLOCK_ID_PENDING).unwrap().unwrap().inner.transactions.is_empty());
+    assert!(
+        backend.get_block(&BLOCK_ID_PENDING).unwrap().unwrap().info.as_pending().unwrap().header.parent_block_hash
+            == finalized_block_zero().info.as_nonpending().unwrap().block_hash,
+        "fake pending block parent hash must match with latest block in db"
+    );
 
     backend.store_block(finalized_block_one(), finalized_state_diff_one(), vec![]).unwrap();
 

crates/client/rpc/src/versions/v0_7_1/methods/read/get_block_with_receipts.rs

@@ -70,10 +70,19 @@ mod tests {
     use super::*;
     use crate::{
         errors::StarknetRpcApiError,
-        test_utils::{sample_chain_for_block_getters, SampleChainForBlockGetters},
+        test_utils::{rpc_test_setup, sample_chain_for_block_getters, SampleChainForBlockGetters},
     };
+    use mc_db::MadaraBackend;
+    use mp_block::{header::GasPrices, Header, MadaraBlockInfo, MadaraBlockInner, MadaraMaybePendingBlock};
+    use mp_chain_config::StarknetVersion;
+    use mp_receipt::{
+        ExecutionResources, ExecutionResult, FeePayment, InvokeTransactionReceipt, PriceUnit, TransactionReceipt,
+    };
+    use mp_state_update::StateDiff;
+    use mp_transactions::{InvokeTransaction, InvokeTransactionV0, Transaction};
     use rstest::rstest;
     use starknet_core::types::{BlockTag, Felt, L1DataAvailabilityMode, ResourcePrice};
+    use std::sync::Arc;
 
     #[rstest]
     fn test_get_block_with_receipts(sample_chain_for_block_getters: (SampleChainForBlockGetters, Starknet)) {
@@ -169,4 +178,72 @@ mod tests {
             Err(StarknetRpcApiError::BlockNotFound)
         );
     }
+
+    #[rstest]
+    fn test_get_block_with_receipts_pending_always_present(rpc_test_setup: (Arc<MadaraBackend>, Starknet)) {
+        let (backend, rpc) = rpc_test_setup;
+        backend
+            .store_block(
+                MadaraMaybePendingBlock {
+                    info: MadaraMaybePendingBlockInfo::NotPending(MadaraBlockInfo {
+                        header: Header {
+                            parent_block_hash: Felt::ZERO,
+                            block_number: 0,
+                            transaction_count: 1,
+                            global_state_root: Felt::from_hex_unchecked("0x88912"),
+                            sequencer_address: Felt::from_hex_unchecked("0xbabaa"),
+                            block_timestamp: 43,
+                            transaction_commitment: Felt::from_hex_unchecked("0xbabaa0"),
+                            event_count: 0,
+                            event_commitment: Felt::from_hex_unchecked("0xb"),
+                            state_diff_length: 5,
+                            state_diff_commitment: Felt::from_hex_unchecked("0xb1"),
+                            receipt_commitment: Felt::from_hex_unchecked("0xb4"),
+                            protocol_version: StarknetVersion::V0_13_1_1,
+                            l1_gas_price: GasPrices {
+                                eth_l1_gas_price: 123,
+                                strk_l1_gas_price: 12,
+                                eth_l1_data_gas_price: 44,
+                                strk_l1_data_gas_price: 52,
+                            },
+                            l1_da_mode: mp_block::header::L1DataAvailabilityMode::Blob,
+                        },
+                        block_hash: Felt::from_hex_unchecked("0x1777177171"),
+                        tx_hashes: vec![Felt::from_hex_unchecked("0x8888888")],
+                    }),
+                    inner: MadaraBlockInner {
+                        transactions: vec![Transaction::Invoke(InvokeTransaction::V0(InvokeTransactionV0 {
+                            max_fee: Felt::from_hex_unchecked("0x12"),
+                            signature: vec![],
+                            contract_address: Felt::from_hex_unchecked("0x4343"),
+                            entry_point_selector: Felt::from_hex_unchecked("0x1212"),
+                            calldata: vec![Felt::from_hex_unchecked("0x2828")],
+                        }))],
+                        receipts: vec![TransactionReceipt::Invoke(InvokeTransactionReceipt {
+                            transaction_hash: Felt::from_hex_unchecked("0x8888888"),
+                            actual_fee: FeePayment { amount: Felt::from_hex_unchecked("0x9"), unit: PriceUnit::Wei },
+                            messages_sent: vec![],
+                            events: vec![],
+                            execution_resources: ExecutionResources::default(),
+                            execution_result: ExecutionResult::Succeeded,
+                        })],
+                    },
+                },
+                StateDiff::default(),
+                vec![],
+            )
+            .unwrap();
+
+        let res = MaybePendingBlockWithReceipts::PendingBlock(PendingBlockWithReceipts {
+            parent_hash: Felt::from_hex_unchecked("0x1777177171"),
+            sequencer_address: Felt::from_hex_unchecked("0xbabaa"),
+            timestamp: 43,
+            starknet_version: StarknetVersion::V0_13_1_1.to_string(),
+            l1_data_gas_price: ResourcePrice { price_in_fri: 52.into(), price_in_wei: 44.into() },
+            l1_gas_price: ResourcePrice { price_in_fri: 12.into(), price_in_wei: 123.into() },
+            l1_da_mode: starknet_core::types::L1DataAvailabilityMode::Blob,
+            transactions: vec![],
+        });
+        assert_eq!(get_block_with_receipts(&rpc, BlockId::Tag(BlockTag::Pending)).unwrap(), res);
+    }
 }

scripts/e2e-tests.sh

@@ -0,0 +1,18 @@
+#!/bin/bash
+# This script is the same as `e2e-coverage` but does not run with llvm-cov. Use this for faster testing, because
+# `e2e-coverage` will likely rebuild everything.
+set -e
+
+# will also launch anvil and automatically close it down on error or success
+
+anvil --fork-url https://eth.merkle.io --fork-block-number 20395662 &
+
+subshell() {
+    set -e
+    cargo build --bin madara --profile dev
+    cargo test --profile dev
+}
+
+(subshell && r=$?) || r=$?
+pkill -P $$
+exit $r