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.

Sign up for GitHub

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

feat(katana): commitment fields in block header #2560

Merged
merged 6 commits into from
Oct 19, 2024
Merged

feat(katana): commitment fields in block header #2560

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b10f124
add new fields to header
kariy Oct 19, 2024
c1618a7
refactor: simplify block structure at its relationships
kariy Oct 19, 2024
b3fe894
Merge branch 'main' into katana/block-header
kariy Oct 19, 2024
fb55875
update test database
kariy Oct 19, 2024
6e2a8de
breaking: bump katana db version to `3`
kariy Oct 19, 2024
d022ef1
rename back to partial header
kariy Oct 19, 2024
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
85 changes: 51 additions & 34 deletions crates/katana/core/src/backend/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ use std::sync::Arc;

use katana_executor::{ExecutionOutput, ExecutionResult, ExecutorFactory};
use katana_primitives::block::{
Block, FinalityStatus, GasPrices, Header, PartialHeader, SealedBlockWithStatus,
Block, FinalityStatus, GasPrices, Header, SealedBlock, SealedBlockWithStatus,
};
use katana_primitives::chain_spec::ChainSpec;
use katana_primitives::da::L1DataAvailabilityMode;
use katana_primitives::env::BlockEnv;
use katana_primitives::transaction::TxHash;
use katana_primitives::receipt::Receipt;
use katana_primitives::transaction::{TxHash, TxWithHash};
use katana_primitives::Felt;
use katana_provider::traits::block::{BlockHashProvider, BlockWriter};
use parking_lot::RwLock;
Expand Down Expand Up @@ -35,6 +36,7 @@ pub struct Backend<EF: ExecutorFactory> {
}

impl<EF: ExecutorFactory> Backend<EF> {
// TODO: add test for this function
pub fn do_mine_block(
&self,
block_env: &BlockEnv,
Expand All @@ -54,47 +56,22 @@ impl<EF: ExecutorFactory> Backend<EF> {
}
}

let prev_hash = BlockHashProvider::latest_hash(self.blockchain.provider())?;
let block_number = block_env.number;
let tx_count = txs.len();

let partial_header = PartialHeader {
number: block_number,
parent_hash: prev_hash,
version: self.chain_spec.version.clone(),
timestamp: block_env.timestamp,
sequencer_address: block_env.sequencer_address,
l1_da_mode: L1DataAvailabilityMode::Calldata,
l1_gas_prices: GasPrices {
eth: block_env.l1_gas_prices.eth,
strk: block_env.l1_gas_prices.strk,
},
l1_data_gas_prices: GasPrices {
eth: block_env.l1_data_gas_prices.eth,
strk: block_env.l1_data_gas_prices.strk,
},
};

let tx_count = txs.len() as u32;
let tx_hashes = txs.iter().map(|tx| tx.hash).collect::<Vec<TxHash>>();
let header = Header::new(partial_header, Felt::ZERO);
let block = Block { header, body: txs }.seal();

// create a new block and compute its commitment
let block = self.commit_block(block_env, txs, &receipts)?;
let block = SealedBlockWithStatus { block, status: FinalityStatus::AcceptedOnL2 };
let block_number = block.block.header.number;

BlockWriter::insert_block_with_states_and_receipts(
self.blockchain.provider(),
self.blockchain.provider().insert_block_with_states_and_receipts(
block,
execution_output.states,
receipts,
traces,
)?;

info!(
target: LOG_TARGET,
block_number = %block_number,
tx_count = %tx_count,
"Block mined.",
);

info!(target: LOG_TARGET, %block_number, %tx_count, "Block mined.");
Ok(MinedBlockOutcome { block_number, txs: tx_hashes, stats: execution_output.stats })
}

Expand All @@ -121,4 +98,44 @@ impl<EF: ExecutorFactory> Backend<EF> {
) -> Result<MinedBlockOutcome, BlockProductionError> {
self.do_mine_block(block_env, Default::default())
}

fn commit_block(
&self,
block_env: &BlockEnv,
transactions: Vec<TxWithHash>,
receipts: &[Receipt],
) -> Result<SealedBlock, BlockProductionError> {
// get the hash of the latest committed block
let parent_hash = self.blockchain.provider().latest_hash()?;
let events_count = receipts.iter().map(|r| r.events().len() as u32).sum::<u32>();
let transaction_count = transactions.len() as u32;

let l1_gas_prices =
GasPrices { eth: block_env.l1_gas_prices.eth, strk: block_env.l1_gas_prices.strk };
let l1_data_gas_prices = GasPrices {
eth: block_env.l1_data_gas_prices.eth,
strk: block_env.l1_data_gas_prices.strk,
};

let header = Header {
parent_hash,
events_count,
l1_gas_prices,
transaction_count,
l1_data_gas_prices,
state_root: Felt::ZERO,
number: block_env.number,
events_commitment: Felt::ZERO,
timestamp: block_env.timestamp,
receipts_commitment: Felt::ZERO,
state_diff_commitment: Felt::ZERO,
transactions_commitment: Felt::ZERO,
l1_da_mode: L1DataAvailabilityMode::Calldata,
sequencer_address: block_env.sequencer_address,
protocol_version: self.chain_spec.version.clone(),
};

let sealed = Block { header, body: transactions }.seal();
Ok(sealed)
}
}
11 changes: 4 additions & 7 deletions crates/katana/core/src/backend/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,18 +353,15 @@ mod tests {

let block_number = blockchain.provider().latest_number().unwrap();
let block_hash = blockchain.provider().latest_hash().unwrap();
let block = blockchain
.provider()
.block_by_hash(dummy_block.block.header.hash)
.unwrap()
.unwrap();
let block =
blockchain.provider().block_by_hash(dummy_block.block.hash).unwrap().unwrap();

kariy marked this conversation as resolved.
Show resolved Hide resolved
let tx = blockchain.provider().transaction_by_hash(dummy_tx.hash).unwrap().unwrap();
let tx_exec =
blockchain.provider().transaction_execution(dummy_tx.hash).unwrap().unwrap();

assert_eq!(block_hash, dummy_block.block.header.hash);
assert_eq!(block_number, dummy_block.block.header.header.number);
assert_eq!(block_hash, dummy_block.block.hash);
assert_eq!(block_number, dummy_block.block.header.number);
assert_eq!(block, dummy_block.block.unseal());
assert_eq!(tx, dummy_tx);
assert_eq!(tx_exec, TxExecInfo::default());
Expand Down
2 changes: 1 addition & 1 deletion crates/katana/core/src/service/block_producer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ impl<EF: ExecutorFactory> InstantBlockProducer<EF> {
parent_hash,
number: block_env.number,
timestamp: block_env.timestamp,
version: backend.chain_spec.version.clone(),
protocol_version: backend.chain_spec.version.clone(),
sequencer_address: block_env.sequencer_address,
l1_da_mode: L1DataAvailabilityMode::Calldata,
l1_gas_prices: block_env.l1_gas_prices.clone(),
Expand Down
8 changes: 4 additions & 4 deletions crates/katana/executor/tests/fixtures/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn state_provider(chain: &ChainSpec) -> Box<dyn StateProvider> {
/// [state_provider].
#[rstest::fixture]
pub fn valid_blocks() -> [ExecutableBlock; 3] {
let version = CURRENT_STARKNET_VERSION;
let protocol_version = CURRENT_STARKNET_VERSION;
let chain_id = ChainId::parse("KATANA").unwrap();
let sequencer_address = ContractAddress(1u64.into());

Expand All @@ -103,7 +103,7 @@ pub fn valid_blocks() -> [ExecutableBlock; 3] {
[
ExecutableBlock {
header: PartialHeader {
version: version.clone(),
protocol_version: protocol_version.clone(),
number: 1,
timestamp: 100,
sequencer_address,
Expand Down Expand Up @@ -153,7 +153,7 @@ pub fn valid_blocks() -> [ExecutableBlock; 3] {
},
ExecutableBlock {
header: PartialHeader {
version: version.clone(),
protocol_version: protocol_version.clone(),
number: 2,
timestamp: 200,
sequencer_address,
Expand Down Expand Up @@ -186,7 +186,7 @@ pub fn valid_blocks() -> [ExecutableBlock; 3] {
},
ExecutableBlock {
header: PartialHeader {
version,
protocol_version,
number: 3,
timestamp: 300,
sequencer_address,
Expand Down
70 changes: 29 additions & 41 deletions crates/katana/primitives/src/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,18 @@ pub enum FinalityStatus {
AcceptedOnL1,
}

/// Represents a partial block header.
/// Represents a pending block header.
#[derive(Debug, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct PartialHeader {
pub parent_hash: BlockHash,
pub number: BlockNumber,
pub parent_hash: Felt,
pub timestamp: u64,
pub sequencer_address: ContractAddress,
pub version: ProtocolVersion,
pub l1_gas_prices: GasPrices,
pub l1_data_gas_prices: GasPrices,
pub l1_da_mode: L1DataAvailabilityMode,
pub protocol_version: ProtocolVersion,
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Suggestion: Rename PartialHeader to PendingHeader for Consistency

Ohayo, sensei! The documentation comment at line 41 refers to a "pending block header," but the struct is still named PartialHeader. To maintain consistency and clarity, consider renaming the struct to PendingHeader.

Apply this diff to rename the struct:

 /// Represents a pending block header.
-pub struct PartialHeader {
+pub struct PendingHeader {
     pub parent_hash: BlockHash,
     pub number: BlockNumber,
     pub timestamp: u64,
     pub sequencer_address: ContractAddress,
     pub l1_gas_prices: GasPrices,
     pub l1_data_gas_prices: GasPrices,
     pub l1_da_mode: L1DataAvailabilityMode,
     pub protocol_version: ProtocolVersion,
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Represents a pending block header.
#[derive(Debug, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct PartialHeader {
pub parent_hash: BlockHash,
pub number: BlockNumber,
pub parent_hash: Felt,
pub timestamp: u64,
pub sequencer_address: ContractAddress,
pub version: ProtocolVersion,
pub l1_gas_prices: GasPrices,
pub l1_data_gas_prices: GasPrices,
pub l1_da_mode: L1DataAvailabilityMode,
pub protocol_version: ProtocolVersion,
/// Represents a pending block header.
#[derive(Debug, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct PendingHeader {
pub parent_hash: BlockHash,
pub number: BlockNumber,
pub timestamp: u64,
pub sequencer_address: ContractAddress,
pub l1_gas_prices: GasPrices,
pub l1_data_gas_prices: GasPrices,
pub l1_da_mode: L1DataAvailabilityMode,
pub protocol_version: ProtocolVersion,

}

// TODO: change names to wei and fri
Expand Down Expand Up @@ -77,51 +77,49 @@ impl GasPrices {
pub struct Header {
pub parent_hash: BlockHash,
pub number: BlockNumber,
pub timestamp: u64,
pub state_diff_commitment: Felt,
pub transactions_commitment: Felt,
pub receipts_commitment: Felt,
pub events_commitment: Felt,
pub state_root: Felt,
pub timestamp: u64,
pub transaction_count: u32,
pub events_count: u32,
Comment on lines +80 to +87
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

🚨 Issues Found: Missing Initialization of New Header Fields

Ohayo, sensei! It looks like there are several instances where the new fields in the Header struct aren't being initialized properly. Please review and update the following locations to include all required fields:

  • crates/saya/provider/src/rpc/mod.rs: header: Header {
  • crates/katana/storage/provider/tests/utils.rs: let header = Header { parent_hash, number: i, ..Default::default() };
  • crates/katana/storage/provider/tests/fixtures.rs: header: Header { number: i, ..Default::default() },
  • crates/katana/storage/provider/src/providers/db/mod.rs: let header = Header { parent_hash: 199u8.into(), number: 0, ..Default::default() };
  • crates/katana/core/src/backend/mod.rs: let header = Header { parent_hash, l1_gas_prices, l1_data_gas_prices, state_root: Felt::ZERO, number: block_env.number, timestamp: block_env.timestamp, l1_da_mode: L1DataAvailabilityMode::Calldata, sequencer_address: block_env.sequencer_address, protocol_version: self.chain_spec.version.clone(), };
  • crates/katana/core/src/backend/storage.rs: header: Header { parent_hash: Felt::ZERO, number: 1, l1_gas_prices: GasPrices::default(), l1_data_gas_prices: GasPrices::default(), l1_da_mode: L1DataAvailabilityMode::Calldata, timestamp: 123456, ..Default::default() }
  • crates/katana/executor/tests/fixtures/mod.rs: header: PartialHeader { protocol_version: protocol_version.clone(), number: 1, timestamp: 100, sequencer_address, parent_hash: 123u64.into(), l1_gas_prices: gas_prices.clone(), l1_data_gas_prices: gas_prices.clone(), l1_da_mode: L1DataAvailabilityMode::Calldata, },
  • crates/katana/executor/tests/fixtures/mod.rs: header: PartialHeader { protocol_version: protocol_version.clone(), number: 2, timestamp: 200, sequencer_address, parent_hash: 1234u64.into(), l1_gas_prices: gas_prices.clone(), l1_data_gas_prices: gas_prices.clone(), l1_da_mode: L1DataAvailabilityMode::Calldata, },
  • crates/katana/executor/tests/fixtures/mod.rs: header: PartialHeader { protocol_version: protocol_version, number: 3, timestamp: 300, sequencer_address, parent_hash: 12345u64.into(), l1_gas_prices: gas_prices.clone(), l1_data_gas_prices: gas_prices.clone(), l1_da_mode: L1DataAvailabilityMode::Calldata, },

Please ensure that all Header initializations include the new fields to maintain consistency and avoid potential runtime issues.

🔗 Analysis chain

Verify Initialization of New Fields in Header

Ohayo, sensei! The added fields in the Header struct enhance the block header's data representation. Please ensure that these new fields are properly initialized and computed throughout the codebase where Header is used.

To check for proper initialization and usage of the new fields, you can run:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all instances of `Header` initialization include the new fields.

# Search for `Header` initializations that might be missing the new fields.
rg --type rust 'Header\s*\{[^}]*\}' -U --pcre2 -r '$0' \
| grep -v 'state_diff_commitment' \
| grep -v 'transactions_commitment' \
| grep -v 'receipts_commitment' \
| grep -v 'events_commitment' \
| grep -v 'transaction_count' \
| grep -v 'events_count'

Length of output: 22799

pub sequencer_address: ContractAddress,
pub protocol_version: ProtocolVersion,
pub l1_gas_prices: GasPrices,
pub l1_data_gas_prices: GasPrices,
pub l1_da_mode: L1DataAvailabilityMode,
pub protocol_version: ProtocolVersion,
}

impl Default for Header {
fn default() -> Self {
Self {
timestamp: 0,
events_count: 0,
transaction_count: 0,
state_root: Felt::ZERO,
events_commitment: Felt::ZERO,
number: BlockNumber::default(),
state_root: Felt::default(),
receipts_commitment: Felt::ZERO,
state_diff_commitment: Felt::ZERO,
parent_hash: BlockHash::default(),
l1_gas_prices: GasPrices::default(),
protocol_version: ProtocolVersion::default(),
sequencer_address: ContractAddress::default(),
transactions_commitment: Felt::ZERO,
l1_data_gas_prices: GasPrices::default(),
sequencer_address: ContractAddress::default(),
l1_da_mode: L1DataAvailabilityMode::Calldata,
protocol_version: ProtocolVersion::default(),
Comment on lines +99 to +112
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Appropriate Default Values for New Fields

Ohayo, sensei! In the Default implementation for Header, the new fields are initialized with default values. Please confirm that these defaults align with the expected initial state to prevent any unintended behavior.

If necessary, adjust the default values to match the intended use cases of these fields.

}
}
}

impl Header {
pub fn new(partial_header: PartialHeader, state_root: Felt) -> Self {
Self {
state_root,
number: partial_header.number,
protocol_version: partial_header.version,
timestamp: partial_header.timestamp,
parent_hash: partial_header.parent_hash,
sequencer_address: partial_header.sequencer_address,
l1_gas_prices: partial_header.l1_gas_prices,
l1_da_mode: partial_header.l1_da_mode,
l1_data_gas_prices: partial_header.l1_data_gas_prices,
}
}

/// Computes the hash of the header.
pub fn compute_hash(&self) -> Felt {
compute_hash_on_elements(&vec![
self.number.into(), // block number
self.state_root, // state root
Felt::ZERO, // state root
self.sequencer_address.into(), // sequencer address
self.timestamp.into(), // block timestamp
Felt::ZERO, // transaction commitment
Expand All @@ -131,11 +129,6 @@ impl Header {
self.parent_hash, // parent hash
])
}

fn seal(self) -> SealedHeader {
let hash = self.compute_hash();
SealedHeader { hash, header: self }
}
}

/// Represents a Starknet full block.
Expand All @@ -156,12 +149,13 @@ pub struct BlockWithTxHashes {
impl Block {
/// Seals the block. This computes the hash of the block.
pub fn seal(self) -> SealedBlock {
SealedBlock { header: self.header.seal(), body: self.body }
let hash = self.header.compute_hash();
SealedBlock { hash, header: self.header, body: self.body }
}

/// Seals the block with a given hash.
pub fn seal_with_hash(self, hash: BlockHash) -> SealedBlock {
SealedBlock { header: SealedHeader { hash, header: self.header }, body: self.body }
SealedBlock { hash, header: self.header, body: self.body }
}

/// Seals the block with a given block hash and status.
Expand All @@ -174,27 +168,21 @@ impl Block {
}
}

/// A full Starknet block that has been sealed.
#[derive(Debug, Clone)]
pub struct SealedHeader {
/// The hash of the header.
pub struct SealedBlock {
/// The block hash.
pub hash: BlockHash,
/// The block header.
pub header: Header,
}

/// A full Starknet block that has been sealed.
#[derive(Debug, Clone)]
pub struct SealedBlock {
/// The sealed block header.
pub header: SealedHeader,
/// The block body.
/// The block transactions.
pub body: Vec<TxWithHash>,
}

impl SealedBlock {
/// Unseal the block.
pub fn unseal(self) -> Block {
Block { header: self.header.header, body: self.body }
Block { header: self.header, body: self.body }
}
}

Expand Down
16 changes: 14 additions & 2 deletions crates/katana/primitives/src/chain_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ impl ChainSpec {
protocol_version: self.version.clone(),
number: self.genesis.number,
timestamp: self.genesis.timestamp,
events_count: 0,
transaction_count: 0,
events_commitment: Felt::ZERO,
receipts_commitment: Felt::ZERO,
state_diff_commitment: Felt::ZERO,
transactions_commitment: Felt::ZERO,
state_root: self.genesis.state_root,
parent_hash: self.genesis.parent_hash,
l1_da_mode: L1DataAvailabilityMode::Calldata,
Expand Down Expand Up @@ -338,9 +344,12 @@ mod tests {
};

// setup expected storage values

let expected_block = Block {
header: Header {
events_commitment: Felt::ZERO,
receipts_commitment: Felt::ZERO,
state_diff_commitment: Felt::ZERO,
transactions_commitment: Felt::ZERO,
number: chain_spec.genesis.number,
timestamp: chain_spec.genesis.timestamp,
state_root: chain_spec.genesis.state_root,
Expand All @@ -350,6 +359,8 @@ mod tests {
l1_data_gas_prices: chain_spec.genesis.gas_prices.clone(),
l1_da_mode: L1DataAvailabilityMode::Calldata,
protocol_version: CURRENT_STARKNET_VERSION,
transaction_count: 0,
events_count: 0,
},
body: Vec::new(),
};
Expand All @@ -361,7 +372,6 @@ mod tests {

assert_eq!(actual_block.header.number, expected_block.header.number);
assert_eq!(actual_block.header.timestamp, expected_block.header.timestamp);
assert_eq!(actual_block.header.state_root, expected_block.header.state_root);
assert_eq!(actual_block.header.parent_hash, expected_block.header.parent_hash);
assert_eq!(actual_block.header.sequencer_address, expected_block.header.sequencer_address);
assert_eq!(actual_block.header.l1_gas_prices, expected_block.header.l1_gas_prices);
Expand All @@ -371,6 +381,8 @@ mod tests {
);
assert_eq!(actual_block.header.l1_da_mode, expected_block.header.l1_da_mode);
assert_eq!(actual_block.header.protocol_version, expected_block.header.protocol_version);
assert_eq!(actual_block.header.transaction_count, expected_block.header.transaction_count);
assert_eq!(actual_block.header.events_count, expected_block.header.events_count);
assert_eq!(actual_block.body, expected_block.body);

if cfg!(feature = "slot") {
Expand Down
Loading
Loading