refactor(blockifier): move BlockExecutionArtifacts to the blockifier …

…crate

crates/blockifier/src/execution_artifacts.rs

@@ -0,0 +1,57 @@
+use std::collections::{HashMap, HashSet};
+
+use indexmap::IndexMap;
+use starknet_api::block_hash::state_diff_hash::calculate_state_diff_hash;
+use starknet_api::core::{ContractAddress, Nonce, StateDiffCommitment};
+use starknet_api::execution_resources::GasAmount;
+use starknet_api::state::ThinStateDiff;
+use starknet_api::transaction::TransactionHash;
+
+use crate::blockifier::transaction_executor::VisitedSegmentsMapping;
+use crate::bouncer::BouncerWeights;
+use crate::state::cached_state::CommitmentStateDiff;
+use crate::transaction::objects::TransactionExecutionInfo;
+
+#[cfg_attr(any(test, feature = "testing"), derive(Clone))]
+#[derive(Debug, PartialEq)]
+pub struct BlockExecutionArtifacts {
+    pub execution_infos: IndexMap<TransactionHash, TransactionExecutionInfo>,
+    pub commitment_state_diff: CommitmentStateDiff,
+    pub visited_segments_mapping: VisitedSegmentsMapping,
+    pub bouncer_weights: BouncerWeights,
+    pub l2_gas_used: GasAmount,
+}
+
+impl BlockExecutionArtifacts {
+    pub fn address_to_nonce(&self) -> HashMap<ContractAddress, Nonce> {
+        HashMap::from_iter(
+            self.commitment_state_diff
+                .address_to_nonce
+                .iter()
+                .map(|(address, nonce)| (*address, *nonce)),
+        )
+    }
+
+    pub fn tx_hashes(&self) -> HashSet<TransactionHash> {
+        HashSet::from_iter(self.execution_infos.keys().copied())
+    }
+
+    pub fn state_diff(&self) -> ThinStateDiff {
+        // TODO(Ayelet): Remove the clones.
+        let storage_diffs = self.commitment_state_diff.storage_updates.clone();
+        let nonces = self.commitment_state_diff.address_to_nonce.clone();
+        ThinStateDiff {
+            deployed_contracts: IndexMap::new(),
+            storage_diffs,
+            declared_classes: IndexMap::new(),
+            nonces,
+            // TODO: Remove this when the structure of storage diffs changes.
+            deprecated_declared_classes: Vec::new(),
+            replaced_classes: IndexMap::new(),
+        }
+    }
+
+    pub fn commitment(&self) -> StateDiffCommitment {
+        calculate_state_diff_hash(&self.state_diff())
+    }
+}

crates/blockifier/src/lib.rs

@@ -22,3 +22,5 @@ pub mod test_utils;
 pub mod transaction;
 pub mod utils;
 pub mod versioned_constants;
+// TODO(yael): create a new crate with the type in this module (and the types it depends on)
+pub mod execution_artifacts;

crates/starknet_batcher/Cargo.toml

@@ -30,6 +30,7 @@ validator.workspace = true
 
 [dev-dependencies]
 assert_matches.workspace = true
+blockifier = { workspace = true, features = ["testing"] }
 chrono = { workspace = true }
 futures.workspace = true
 mempool_test_utils.workspace = true

crates/starknet_batcher/src/batcher.rs

@@ -1,6 +1,7 @@
 use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
 
+use blockifier::execution_artifacts::BlockExecutionArtifacts;
 use blockifier::state::contract_class_manager::ContractClassManager;
 #[cfg(test)]
 use mockall::automock;
@@ -43,7 +44,6 @@ use crate::block_builder::{
     BlockBuilderFactory,
     BlockBuilderFactoryTrait,
     BlockBuilderTrait,
-    BlockExecutionArtifacts,
     BlockMetadata,
 };
 use crate::config::BatcherConfig;
@@ -522,7 +522,9 @@ impl Batcher {
         let guard = self.executed_proposals.lock().await;
         let proposal_result = guard.get(&proposal_id);
         match proposal_result {
-            Some(Ok(artifacts)) => Some(Ok(artifacts.commitment())),
+            Some(Ok(artifacts)) => {
+                Some(Ok(ProposalCommitment { state_diff_commitment: artifacts.commitment() }))
+            }
             Some(Err(e)) => Some(Err(e.clone())),
             None => None,
         }

crates/starknet_batcher/src/batcher_test.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 
 use assert_matches::assert_matches;
 use blockifier::abi::constants;
+use blockifier::execution_artifacts::BlockExecutionArtifacts;
 use indexmap::indexmap;
 use mockall::predicate::eq;
 use rstest::rstest;
@@ -39,12 +40,16 @@ use crate::block_builder::{
     AbortSignalSender,
     BlockBuilderError,
     BlockBuilderResult,
-    BlockExecutionArtifacts,
     FailOnErrorCause,
     MockBlockBuilderFactoryTrait,
 };
 use crate::config::BatcherConfig;
-use crate::test_utils::{test_txs, FakeProposeBlockBuilder, FakeValidateBlockBuilder};
+use crate::test_utils::{
+    test_txs,
+    testing_block_execution_artifacts,
+    FakeProposeBlockBuilder,
+    FakeValidateBlockBuilder,
+};
 
 const INITIAL_HEIGHT: BlockNumber = BlockNumber(3);
 const STREAMING_CHUNK_SIZE: usize = 3;
@@ -54,7 +59,7 @@ const BUILD_BLOCK_FAIL_ON_ERROR: BlockBuilderError =
     BlockBuilderError::FailOnError(FailOnErrorCause::BlockFull);
 
 fn proposal_commitment() -> ProposalCommitment {
-    BlockExecutionArtifacts::create_for_testing().commitment()
+    ProposalCommitment { state_diff_commitment: testing_block_execution_artifacts().commitment() }
 }
 
 fn propose_block_input(proposal_id: ProposalId) -> ProposeBlockInput {
@@ -224,7 +229,7 @@ async fn consecutive_heights_success() {
         mock_create_builder_for_propose_block(
             &mut block_builder_factory,
             vec![],
-            Ok(BlockExecutionArtifacts::create_for_testing()),
+            Ok(testing_block_execution_artifacts()),
         );
     }
 
@@ -256,7 +261,7 @@ async fn consecutive_heights_success() {
 #[tokio::test]
 async fn validate_block_full_flow() {
     let mut batcher =
-        batcher_with_active_validate_block(Ok(BlockExecutionArtifacts::create_for_testing())).await;
+        batcher_with_active_validate_block(Ok(testing_block_execution_artifacts())).await;
 
     let send_proposal_input_txs = SendProposalContentInput {
         proposal_id: PROPOSAL_ID,
@@ -320,7 +325,7 @@ async fn send_proposal_content_after_finish_or_abort(
     #[case] content: SendProposalContent,
 ) {
     let mut batcher =
-        batcher_with_active_validate_block(Ok(BlockExecutionArtifacts::create_for_testing())).await;
+        batcher_with_active_validate_block(Ok(testing_block_execution_artifacts())).await;
 
     // End the proposal.
     let end_proposal =
@@ -338,7 +343,7 @@ async fn send_proposal_content_after_finish_or_abort(
 #[tokio::test]
 async fn send_proposal_content_abort() {
     let mut batcher =
-        batcher_with_active_validate_block(Ok(BlockExecutionArtifacts::create_for_testing())).await;
+        batcher_with_active_validate_block(Ok(testing_block_execution_artifacts())).await;
 
     let send_abort_proposal =
         SendProposalContentInput { proposal_id: PROPOSAL_ID, content: SendProposalContent::Abort };
@@ -358,7 +363,7 @@ async fn propose_block_full_flow() {
     mock_create_builder_for_propose_block(
         &mut block_builder_factory,
         expected_streamed_txs.clone(),
-        Ok(BlockExecutionArtifacts::create_for_testing()),
+        Ok(testing_block_execution_artifacts()),
     );
 
     let mut batcher =
@@ -444,11 +449,11 @@ async fn consecutive_proposal_generation_success() {
         mock_create_builder_for_propose_block(
             &mut block_builder_factory,
             vec![],
-            Ok(BlockExecutionArtifacts::create_for_testing()),
+            Ok(testing_block_execution_artifacts()),
         );
         mock_create_builder_for_validate_block(
             &mut block_builder_factory,
-            Ok(BlockExecutionArtifacts::create_for_testing()),
+            Ok(testing_block_execution_artifacts()),
         );
     }
     let mut batcher =
@@ -475,7 +480,7 @@ async fn consecutive_proposal_generation_success() {
 #[tokio::test]
 async fn concurrent_proposals_generation_fail() {
     let mut batcher =
-        batcher_with_active_validate_block(Ok(BlockExecutionArtifacts::create_for_testing())).await;
+        batcher_with_active_validate_block(Ok(testing_block_execution_artifacts())).await;
 
     // Make sure another proposal can't be generated while the first one is still active.
     let result = batcher.propose_block(propose_block_input(ProposalId(1))).await;
@@ -538,7 +543,7 @@ async fn add_sync_block_mismatch_block_number() {
 #[tokio::test]
 async fn decision_reached() {
     let mut mock_dependencies = MockDependencies::default();
-    let expected_artifacts = BlockExecutionArtifacts::create_for_testing();
+    let expected_artifacts = testing_block_execution_artifacts();
 
     mock_dependencies
         .mempool_client
@@ -560,7 +565,7 @@ async fn decision_reached() {
     mock_create_builder_for_propose_block(
         &mut mock_dependencies.block_builder_factory,
         vec![],
-        Ok(BlockExecutionArtifacts::create_for_testing()),
+        Ok(testing_block_execution_artifacts()),
     );
 
     let mut batcher = create_batcher(mock_dependencies);

crates/starknet_batcher/src/block_builder.rs

@@ -10,6 +10,7 @@ use blockifier::blockifier::transaction_executor::{
 };
 use blockifier::bouncer::{BouncerConfig, BouncerWeights};
 use blockifier::context::{BlockContext, ChainInfo};
+use blockifier::execution_artifacts::BlockExecutionArtifacts;
 use blockifier::state::cached_state::CommitmentStateDiff;
 use blockifier::state::contract_class_manager::ContractClassManager;
 use blockifier::state::errors::StateError;
@@ -66,50 +67,6 @@ pub enum FailOnErrorCause {
     TransactionFailed(BlockifierTransactionExecutorError),
 }
 
-#[cfg_attr(test, derive(Clone))]
-#[derive(Debug, PartialEq)]
-pub struct BlockExecutionArtifacts {
-    pub execution_infos: IndexMap<TransactionHash, TransactionExecutionInfo>,
-    pub commitment_state_diff: CommitmentStateDiff,
-    pub visited_segments_mapping: VisitedSegmentsMapping,
-    pub bouncer_weights: BouncerWeights,
-    pub l2_gas_used: GasAmount,
-}
-
-impl BlockExecutionArtifacts {
-    pub fn address_to_nonce(&self) -> HashMap<ContractAddress, Nonce> {
-        HashMap::from_iter(
-            self.commitment_state_diff
-                .address_to_nonce
-                .iter()
-                .map(|(address, nonce)| (*address, *nonce)),
-        )
-    }
-
-    pub fn tx_hashes(&self) -> HashSet<TransactionHash> {
-        HashSet::from_iter(self.execution_infos.keys().copied())
-    }
-
-    pub fn state_diff(&self) -> ThinStateDiff {
-        // TODO(Ayelet): Remove the clones.
-        let storage_diffs = self.commitment_state_diff.storage_updates.clone();
-        let nonces = self.commitment_state_diff.address_to_nonce.clone();
-        ThinStateDiff {
-            deployed_contracts: IndexMap::new(),
-            storage_diffs,
-            declared_classes: IndexMap::new(),
-            nonces,
-            // TODO: Remove this when the structure of storage diffs changes.
-            deprecated_declared_classes: Vec::new(),
-            replaced_classes: IndexMap::new(),
-        }
-    }
-
-    pub fn commitment(&self) -> ProposalCommitment {
-        ProposalCommitment { state_diff_commitment: calculate_state_diff_hash(&self.state_diff()) }
-    }
-}
-
 /// The BlockBuilderTrait is responsible for building a new block from transactions provided by the
 /// tx_provider. The block building will stop at time deadline.
 /// The transactions that were added to the block will be streamed to the output_content_sender.

crates/starknet_batcher/src/block_builder_test.rs

@@ -1,6 +1,7 @@
 use assert_matches::assert_matches;
 use blockifier::blockifier::transaction_executor::TransactionExecutorError;
 use blockifier::bouncer::BouncerWeights;
+use blockifier::execution_artifacts::BlockExecutionArtifacts;
 use blockifier::fee::fee_checks::FeeCheckError;
 use blockifier::fee::receipt::TransactionReceipt;
 use blockifier::state::errors::StateError;
@@ -23,7 +24,6 @@ use crate::block_builder::{
     BlockBuilderExecutionParams,
     BlockBuilderResult,
     BlockBuilderTrait,
-    BlockExecutionArtifacts,
     FailOnErrorCause,
 };
 use crate::test_utils::test_txs;

crates/starknet_batcher/src/test_utils.rs

@@ -3,6 +3,7 @@ use std::ops::Range;
 use async_trait::async_trait;
 use blockifier::blockifier::transaction_executor::VisitedSegmentsMapping;
 use blockifier::bouncer::BouncerWeights;
+use blockifier::execution_artifacts::BlockExecutionArtifacts;
 use blockifier::state::cached_state::CommitmentStateDiff;
 use indexmap::IndexMap;
 use starknet_api::executable_transaction::Transaction;
@@ -11,7 +12,7 @@ use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs};
 use starknet_api::{class_hash, contract_address, nonce, tx_hash};
 use tokio::sync::mpsc::UnboundedSender;
 
-use crate::block_builder::{BlockBuilderResult, BlockBuilderTrait, BlockExecutionArtifacts};
+use crate::block_builder::{BlockBuilderResult, BlockBuilderTrait};
 use crate::transaction_provider::{NextTxs, TransactionProvider};
 
 // A fake block builder for validate flow, that fetches transactions from the transaction provider
@@ -70,23 +71,21 @@ pub fn test_txs(tx_hash_range: Range<usize>) -> Vec<Transaction> {
         .collect()
 }
 
-impl BlockExecutionArtifacts {
-    pub fn create_for_testing() -> Self {
-        // Use a non-empty commitment_state_diff to make the tests more realistic.
-        Self {
-            execution_infos: IndexMap::default(),
-            commitment_state_diff: CommitmentStateDiff {
-                address_to_class_hash: IndexMap::from_iter([(
-                    contract_address!("0x7"),
-                    class_hash!("0x11111111"),
-                )]),
-                storage_updates: IndexMap::new(),
-                class_hash_to_compiled_class_hash: IndexMap::new(),
-                address_to_nonce: IndexMap::from_iter([(contract_address!("0x7"), nonce!(1_u64))]),
-            },
-            visited_segments_mapping: VisitedSegmentsMapping::default(),
-            bouncer_weights: BouncerWeights::empty(),
-            l2_gas_used: GasAmount::default(),
-        }
+pub fn testing_block_execution_artifacts() -> BlockExecutionArtifacts {
+    // Use a non-empty commitment_state_diff to make the tests more realistic.
+    BlockExecutionArtifacts {
+        execution_infos: IndexMap::default(),
+        commitment_state_diff: CommitmentStateDiff {
+            address_to_class_hash: IndexMap::from_iter([(
+                contract_address!("0x7"),
+                class_hash!("0x11111111"),
+            )]),
+            storage_updates: IndexMap::new(),
+            class_hash_to_compiled_class_hash: IndexMap::new(),
+            address_to_nonce: IndexMap::from_iter([(contract_address!("0x7"), nonce!(1_u64))]),
+        },
+        visited_segments_mapping: VisitedSegmentsMapping::default(),
+        bouncer_weights: BouncerWeights::empty(),
+        l2_gas_used: GasAmount::default(),
     }
 }