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 all 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 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 @@
}

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 @@
}
}

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

Check warning on line 62 in crates/katana/core/src/backend/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/core/src/backend/mod.rs#L62 

Added line #L62 was not covered by tests
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 @@
) -> 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

Check warning on line 108 in crates/katana/core/src/backend/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/core/src/backend/mod.rs#L107-L108 

Added lines #L107 - L108 were not covered by tests
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,

Check warning on line 116 in crates/katana/core/src/backend/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/core/src/backend/mod.rs#L114-L116 

Added lines #L114 - L116 were not covered by tests
strk: block_env.l1_data_gas_prices.strk,
};

let header = Header {
parent_hash,

Check warning on line 121 in crates/katana/core/src/backend/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/core/src/backend/mod.rs#L120-L121 

Added lines #L120 - L121 were not covered by tests
events_count,
l1_gas_prices,
transaction_count,
l1_data_gas_prices,
state_root: Felt::ZERO,
number: block_env.number,
events_commitment: Felt::ZERO,

Check warning on line 128 in crates/katana/core/src/backend/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/core/src/backend/mod.rs#L124-L128 

Added lines #L124 - L128 were not covered by tests
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
68 changes: 28 additions & 40 deletions crates/katana/primitives/src/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@
#[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,
}

// TODO: change names to wei and fri
Expand Down Expand Up @@ -77,51 +77,49 @@
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,

Check warning on line 92 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L92 

Added line #L92 was not covered by tests
}

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,

Check warning on line 108 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L108 

Added line #L108 was not covered by tests
l1_data_gas_prices: GasPrices::default(),
sequencer_address: ContractAddress::default(),

Check warning on line 110 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L110 

Added line #L110 was not covered by tests
l1_da_mode: L1DataAvailabilityMode::Calldata,
protocol_version: ProtocolVersion::default(),

Check warning on line 112 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L112 

Added line #L112 was not covered by tests
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

Check warning on line 122 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L122 

Added line #L122 was not covered by tests
self.sequencer_address.into(), // sequencer address
self.timestamp.into(), // block timestamp
Felt::ZERO, // transaction commitment
Expand All @@ -131,11 +129,6 @@
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 @@
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 @@
}
}

/// A full Starknet block that has been sealed.

Check warning on line 171 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L171 

Added line #L171 was not covered by tests
#[derive(Debug, Clone)]
pub struct SealedHeader {
/// The hash of the header.
pub struct SealedBlock {
/// The block hash.

Check warning on line 174 in crates/katana/primitives/src/block.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/block.rs#L173-L174 

Added lines #L173 - L174 were not covered by tests
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 @@
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,

Check warning on line 63 in crates/katana/primitives/src/chain_spec.rs

View check run for this annotation

Codecov / codecov/patch

crates/katana/primitives/src/chain_spec.rs#L58-L63 

Added lines #L58 - L63 were not covered by tests
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 @@
};

// 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 @@
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 @@

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 @@
);
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