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.

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

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

Merged
merged 3 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
91 changes: 70 additions & 21 deletions crates/client/db/src/block_db.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand All @@ -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),
}
}
Expand All @@ -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),
}
}
Expand Down Expand Up @@ -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 _))))
}
Expand All @@ -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 _))))
}
}
Expand Down
9 changes: 7 additions & 2 deletions crates/client/db/tests/test_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}
}
18 changes: 18 additions & 0 deletions scripts/e2e-tests.sh
Original file line number Diff line number Diff line change
@@ -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
Loading