Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(katana): special block hash registry contract #2894

Merged
merged 6 commits into from
Jan 11, 2025
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
42 changes: 39 additions & 3 deletions crates/katana/core/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ use std::sync::Arc;
use gas_oracle::L1GasOracle;
use katana_executor::{ExecutionOutput, ExecutionResult, ExecutorFactory};
use katana_primitives::block::{
FinalityStatus, Header, PartialHeader, SealedBlock, SealedBlockWithStatus,
BlockHash, BlockNumber, FinalityStatus, Header, PartialHeader, SealedBlock,
SealedBlockWithStatus,
};
use katana_primitives::chain_spec::ChainSpec;
use katana_primitives::da::L1DataAvailabilityMode;
use katana_primitives::env::BlockEnv;
use katana_primitives::receipt::{Event, ReceiptWithTxHash};
use katana_primitives::state::{compute_state_diff_hash, StateUpdates};
use katana_primitives::transaction::{TxHash, TxWithHash};
use katana_primitives::Felt;
use katana_primitives::{address, ContractAddress, Felt};
use katana_provider::traits::block::{BlockHashProvider, BlockWriter};
use katana_provider::traits::trie::TrieWriter;
use katana_trie::compute_merkle_root;
Expand Down Expand Up @@ -49,7 +50,7 @@ impl<EF: ExecutorFactory> Backend<EF> {
pub fn do_mine_block(
&self,
block_env: &BlockEnv,
execution_output: ExecutionOutput,
mut execution_output: ExecutionOutput,
) -> Result<MinedBlockOutcome, BlockProductionError> {
// we optimistically allocate the maximum amount possible
let mut txs = Vec::with_capacity(execution_output.transactions.len());
Expand All @@ -68,6 +69,12 @@ impl<EF: ExecutorFactory> Backend<EF> {
let tx_count = txs.len() as u32;
let tx_hashes = txs.iter().map(|tx| tx.hash).collect::<Vec<TxHash>>();

// Update special contract address 0x1
self.update_block_hash_registry_contract(
&mut execution_output.states.state_updates,
block_env.number,
)?;

// create a new block and compute its commitment
let block = self.commit_block(
block_env.clone(),
Expand All @@ -93,6 +100,35 @@ impl<EF: ExecutorFactory> Backend<EF> {
Ok(MinedBlockOutcome { block_number, txs: tx_hashes, stats: execution_output.stats })
}

// TODO: create a dedicated struct for this contract.
// https://docs.starknet.io/architecture-and-concepts/network-architecture/starknet-state/#address_0x1
fn update_block_hash_registry_contract(
&self,
state_updates: &mut StateUpdates,
block_number: BlockNumber,
) -> Result<(), BlockProductionError> {
const STORED_BLOCK_HASH_BUFFER: u64 = 10;

if block_number >= STORED_BLOCK_HASH_BUFFER {
let block_number = block_number - STORED_BLOCK_HASH_BUFFER;
let block_hash = self.blockchain.provider().block_hash_by_num(block_number)?;

// When in forked mode, we might not have the older block hash in the database. This
// could be the case where the `block_number - STORED_BLOCK_HASH_BUFFER` is
// earlier than the forked block, which right now, Katana doesn't
// yet have the ability to fetch older blocks on the database level. So, we default to
// `BlockHash::ZERO` in this case.
//
// TODO: Fix quick!
let block_hash = block_hash.unwrap_or(BlockHash::ZERO);

let storages = state_updates.storage_updates.entry(address!("0x1")).or_default();
storages.insert(block_number.into(), block_hash);
}

Ok(())
}
Comment on lines +103 to +130
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider error handling improvements.

Ohayo, sensei! While the implementation is generally good, there are a few suggestions:

  1. The unwrap on line 123 could be handled more gracefully.
  2. The TODO comment on line 122 should be more descriptive.

Apply this diff to improve error handling:

-            // TODO: Fix quick!
-            let block_hash = block_hash.unwrap_or(BlockHash::ZERO);
+            // TODO: Implement historical block fetching for forked mode to avoid defaulting to ZERO
+            let block_hash = block_hash.unwrap_or_else(|| {
+                tracing::warn!(
+                    target: LOG_TARGET,
+                    block_number = %block_number,
+                    "Block hash not found for historical block in forked mode, defaulting to ZERO"
+                );
+                BlockHash::ZERO
+            });

Additionally, consider adding a constant for the special contract address:

+    /// Special system contract address for block hash registry
+    const BLOCK_HASH_REGISTRY_ADDRESS: ContractAddress = address!("0x1");
+
     fn update_block_hash_registry_contract(

Committable suggestion skipped: line range outside the PR's diff.


pub fn update_block_env(&self, block_env: &mut BlockEnv) {
let mut context_gen = self.block_context_generator.write();
let current_timestamp_secs = get_current_timestamp().as_secs() as i64;
Expand Down
16 changes: 13 additions & 3 deletions crates/katana/rpc/rpc/src/starknet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,12 @@ where
contract_address: ContractAddress,
) -> StarknetApiResult<ClassHash> {
self.on_io_blocking_task(move |this| {
// Contract address 0x1 is special system contract and does not
// have a class. See https://docs.starknet.io/architecture-and-concepts/network-architecture/starknet-state/#address_0x1.
if contract_address.0 == Felt::ONE {
return Ok(ClassHash::ZERO);
}

let state = this.state(&block_id)?;
let class_hash = state.class_hash_of_contract(contract_address)?;
class_hash.ok_or(StarknetApiError::ContractNotFound)
Expand All @@ -305,10 +311,14 @@ where
) -> StarknetApiResult<StorageValue> {
let state = self.state(&block_id)?;

// check that contract exist by checking the class hash of the contract
let Some(_) = state.class_hash_of_contract(contract_address)? else {
// Check that contract exist by checking the class hash of the contract,
// unless its address 0x1 which is special system contract and does not
// have a class. See https://docs.starknet.io/architecture-and-concepts/network-architecture/starknet-state/#address_0x1.
if contract_address.0 != Felt::ONE
&& state.class_hash_of_contract(contract_address)?.is_none()
{
return Err(StarknetApiError::ContractNotFound);
};
}

let value = state.storage(contract_address, storage_key)?;
Ok(value.unwrap_or_default())
Expand Down
48 changes: 24 additions & 24 deletions crates/katana/storage/provider/tests/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ mod latest {
#[rstest::rstest]
#[case(
vec![
(ContractAddress::from(felt!("1")), Some(felt!("22")), Some(felt!("3"))),
(ContractAddress::from(felt!("2")), Some(felt!("33")), Some(felt!("2"))),
(ContractAddress::from(felt!("1337")), Some(felt!("22")), Some(felt!("3"))),
(ContractAddress::from(felt!("80085")), Some(felt!("33")), Some(felt!("2"))),
]
)]
fn test_latest_contract_info_read<Db>(
Expand Down Expand Up @@ -95,31 +95,31 @@ mod historical {
#[case::storage_at_block_0(
0,
vec![
(ContractAddress::from(felt!("1")), None, None),
(ContractAddress::from(felt!("2")), None, None)
])
]
(ContractAddress::from(felt!("1337")), None, None),
(ContractAddress::from(felt!("80085")), None, None)
])
]
#[case::storage_at_block_1(
1,
vec![
(ContractAddress::from(felt!("1")), Some(felt!("11")), Some(felt!("1"))),
(ContractAddress::from(felt!("2")), Some(felt!("11")), Some(felt!("1"))),
])
]
1,
vec![
(ContractAddress::from(felt!("1337")), Some(felt!("11")), Some(felt!("1"))),
(ContractAddress::from(felt!("80085")), Some(felt!("11")), Some(felt!("1"))),
])
]
#[case::storage_at_block_4(
4,
vec![
(ContractAddress::from(felt!("1")), Some(felt!("11")), Some(felt!("2"))),
(ContractAddress::from(felt!("2")), Some(felt!("22")), Some(felt!("1"))),
])
]
4,
vec![
(ContractAddress::from(felt!("1337")), Some(felt!("11")), Some(felt!("2"))),
(ContractAddress::from(felt!("80085")), Some(felt!("22")), Some(felt!("1"))),
])
]
#[case::storage_at_block_5(
5,
vec![
(ContractAddress::from(felt!("1")), Some(felt!("22")), Some(felt!("3"))),
(ContractAddress::from(felt!("2")), Some(felt!("33")), Some(felt!("2"))),
])
]
5,
vec![
(ContractAddress::from(felt!("1337")), Some(felt!("22")), Some(felt!("3"))),
(ContractAddress::from(felt!("80085")), Some(felt!("33")), Some(felt!("2"))),
])
]
fn test_historical_storage_read(
#[from(provider_with_states)] provider: BlockchainProvider<InMemoryProvider>,
#[case] block_num: BlockNumber,
Expand Down
4 changes: 2 additions & 2 deletions crates/katana/storage/provider/tests/fixtures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ pub fn db_provider() -> BlockchainProvider<DbProvider> {

#[rstest::fixture]
pub fn mock_state_updates() -> [StateUpdatesWithClasses; 3] {
let address_1 = address!("1");
let address_2 = address!("2");
let address_1 = address!("1337");
let address_2 = address!("80085");

let class_hash_1 = felt!("11");
let compiled_class_hash_1 = felt!("1000");
Expand Down
44 changes: 22 additions & 22 deletions crates/katana/storage/provider/tests/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ mod latest {
#[rstest::rstest]
#[case(
vec![
(ContractAddress::from(felt!("1")), felt!("1"), Some(felt!("111"))),
(ContractAddress::from(felt!("1")), felt!("2"), Some(felt!("222"))),
(ContractAddress::from(felt!("1")), felt!("3"), Some(felt!("77"))),
(ContractAddress::from(felt!("2")), felt!("1"), Some(felt!("12"))),
(ContractAddress::from(felt!("2")), felt!("2"), Some(felt!("13")))
(ContractAddress::from(felt!("1337")), felt!("1"), Some(felt!("111"))),
(ContractAddress::from(felt!("1337")), felt!("2"), Some(felt!("222"))),
(ContractAddress::from(felt!("1337")), felt!("3"), Some(felt!("77"))),
(ContractAddress::from(felt!("80085")), felt!("1"), Some(felt!("12"))),
(ContractAddress::from(felt!("80085")), felt!("2"), Some(felt!("13")))
]
)]
fn test_latest_storage_read<Db>(
Expand Down Expand Up @@ -95,38 +95,38 @@ mod historical {
#[case::storage_at_block_0(
0,
vec![
(ContractAddress::from(felt!("1")), felt!("1"), None),
(ContractAddress::from(felt!("1")), felt!("2"), None),
(ContractAddress::from(felt!("2")), felt!("1"), None),
(ContractAddress::from(felt!("2")), felt!("2"), None)
(ContractAddress::from(felt!("1337")), felt!("1"), None),
(ContractAddress::from(felt!("1337")), felt!("2"), None),
(ContractAddress::from(felt!("80085")), felt!("1"), None),
(ContractAddress::from(felt!("80085")), felt!("2"), None)
])
]
#[case::storage_at_block_1(
1,
vec![
(ContractAddress::from(felt!("1")), felt!("1"), Some(felt!("100"))),
(ContractAddress::from(felt!("1")), felt!("2"), Some(felt!("101"))),
(ContractAddress::from(felt!("2")), felt!("1"), Some(felt!("200"))),
(ContractAddress::from(felt!("2")), felt!("2"), Some(felt!("201"))),
(ContractAddress::from(felt!("1337")), felt!("1"), Some(felt!("100"))),
(ContractAddress::from(felt!("1337")), felt!("2"), Some(felt!("101"))),
(ContractAddress::from(felt!("80085")), felt!("1"), Some(felt!("200"))),
(ContractAddress::from(felt!("80085")), felt!("2"), Some(felt!("201"))),
])
]
#[case::storage_at_block_4(
4,
vec![
(ContractAddress::from(felt!("1")), felt!("1"), Some(felt!("111"))),
(ContractAddress::from(felt!("1")), felt!("2"), Some(felt!("222"))),
(ContractAddress::from(felt!("2")), felt!("1"), Some(felt!("200"))),
(ContractAddress::from(felt!("2")), felt!("2"), Some(felt!("201"))),
(ContractAddress::from(felt!("1337")), felt!("1"), Some(felt!("111"))),
(ContractAddress::from(felt!("1337")), felt!("2"), Some(felt!("222"))),
(ContractAddress::from(felt!("80085")), felt!("1"), Some(felt!("200"))),
(ContractAddress::from(felt!("80085")), felt!("2"), Some(felt!("201"))),
])
]
#[case::storage_at_block_5(
5,
vec![
(ContractAddress::from(felt!("1")), felt!("1"), Some(felt!("111"))),
(ContractAddress::from(felt!("1")), felt!("2"), Some(felt!("222"))),
(ContractAddress::from(felt!("1")), felt!("3"), Some(felt!("77"))),
(ContractAddress::from(felt!("2")), felt!("1"), Some(felt!("12"))),
(ContractAddress::from(felt!("2")), felt!("2"), Some(felt!("13"))),
(ContractAddress::from(felt!("1337")), felt!("1"), Some(felt!("111"))),
(ContractAddress::from(felt!("1337")), felt!("2"), Some(felt!("222"))),
(ContractAddress::from(felt!("1337")), felt!("3"), Some(felt!("77"))),
(ContractAddress::from(felt!("80085")), felt!("1"), Some(felt!("12"))),
(ContractAddress::from(felt!("80085")), felt!("2"), Some(felt!("13"))),
])
]
fn test_historical_storage_read(
Expand Down
Loading