refactor(blockifier): replace Default drive of Bouncer and related st…

…ructs by empty func (#1292)
starkware-libs · Oct 14, 2024 · 16caad8 · 16caad8
1 parent c7c7b1d
commit 16caad8
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 34 deletions.
diff --git a/crates/batcher/src/block_builder.rs b/crates/batcher/src/block_builder.rs
@@ -69,7 +69,7 @@ impl BlockBuilderTrait for BlockBuilder {
     }
 }
 
-#[derive(Default, Debug, PartialEq)]
+#[derive(Debug, PartialEq)]
 pub struct BlockExecutionArtifacts {
     pub execution_infos: IndexMap<TransactionHash, TransactionExecutionInfo>,
     pub commitment_state_diff: CommitmentStateDiff,

diff --git a/crates/batcher/src/proposal_manager_test.rs b/crates/batcher/src/proposal_manager_test.rs
@@ -287,7 +287,7 @@ async fn simulate_block_builder(
         output_sender.send(tx).unwrap();
     }
 
-    Ok(BlockExecutionArtifacts::default())
+    Ok(BlockExecutionArtifacts::create_for_testing())
 }
 
 // A wrapper trait to allow mocking the BlockBuilderTrait in tests.

diff --git a/crates/batcher/src/test_utils.rs b/crates/batcher/src/test_utils.rs
@@ -1,10 +1,16 @@
 use std::ops::Range;
 
+use blockifier::blockifier::transaction_executor::VisitedSegmentsMapping;
+use blockifier::bouncer::BouncerWeights;
+use blockifier::state::cached_state::CommitmentStateDiff;
+use indexmap::IndexMap;
 use starknet_api::executable_transaction::Transaction;
 use starknet_api::felt;
 use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs};
 use starknet_api::transaction::TransactionHash;
 
+use crate::block_builder::BlockExecutionArtifacts;
+
 pub fn test_txs(tx_hash_range: Range<usize>) -> Vec<Transaction> {
     tx_hash_range
         .map(|i| {
@@ -15,3 +21,14 @@ pub fn test_txs(tx_hash_range: Range<usize>) -> Vec<Transaction> {
         })
         .collect()
 }
+
+impl BlockExecutionArtifacts {
+    pub fn create_for_testing() -> Self {
+        Self {
+            execution_infos: IndexMap::default(),
+            commitment_state_diff: CommitmentStateDiff::default(),
+            visited_segments_mapping: VisitedSegmentsMapping::default(),
+            bouncer_weights: BouncerWeights::empty(),
+        }
+    }
+}
diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs
@@ -70,7 +70,7 @@ fn tx_executor_test_body<S: StateReader>(
         state_diff_size: 0,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 #[case::tx_version_1(
@@ -80,7 +80,7 @@ fn tx_executor_test_body<S: StateReader>(
         state_diff_size: 2,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 #[case::tx_version_2(
@@ -90,7 +90,7 @@ fn tx_executor_test_body<S: StateReader>(
         state_diff_size: 4,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 #[case::tx_version_3(
@@ -100,7 +100,7 @@ fn tx_executor_test_body<S: StateReader>(
         state_diff_size: 4,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 fn test_declare(
@@ -150,7 +150,7 @@ fn test_deploy_account(
         state_diff_size: 3,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     };
     tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
 }
@@ -166,7 +166,7 @@ fn test_deploy_account(
         state_diff_size: 2,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 #[case::emit_event_syscall(
@@ -180,7 +180,7 @@ fn test_deploy_account(
         state_diff_size: 2,
         message_segment_length: 0,
         n_events: 1,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 #[case::storage_write_syscall(
@@ -190,7 +190,7 @@ fn test_deploy_account(
         state_diff_size: 6,
         message_segment_length: 0,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     }
 )]
 fn test_invoke(
@@ -233,7 +233,7 @@ fn test_l1_handler(block_context: BlockContext) {
         state_diff_size: 4,
         message_segment_length: 7,
         n_events: 0,
-        ..Default::default()
+        ..BouncerWeights::empty()
     };
     tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
 }
@@ -245,7 +245,7 @@ fn test_l1_handler(block_context: BlockContext) {
 #[case::block_full(
     BouncerWeights {
         n_events: 4,
-        ..Default::default()
+        ..BouncerWeights::empty()
     },
     7
 )]

diff --git a/crates/blockifier/src/bouncer.rs b/crates/blockifier/src/bouncer.rs
@@ -38,12 +38,16 @@ macro_rules! impl_checked_sub {
 
 pub type HashMapWrapper = HashMap<BuiltinName, usize>;
 
-#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
+#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct BouncerConfig {
     pub block_max_capacity: BouncerWeights,
 }
 
 impl BouncerConfig {
+    pub fn empty() -> Self {
+        Self { block_max_capacity: BouncerWeights::empty() }
+    }
+
     pub fn max() -> Self {
         Self { block_max_capacity: BouncerWeights::max() }
     }
@@ -71,7 +75,6 @@ impl BouncerConfig {
     Clone,
     Copy,
     Debug,
-    Default,
     derive_more::Add,
     derive_more::AddAssign,
     derive_more::Sub,
@@ -113,6 +116,17 @@ impl BouncerWeights {
             builtin_count: BuiltinCount::max(),
         }
     }
+
+    pub fn empty() -> Self {
+        Self {
+            n_events: 0,
+            builtin_count: BuiltinCount::empty(),
+            gas: 0,
+            message_segment_length: 0,
+            n_steps: 0,
+            state_diff_size: 0,
+        }
+    }
 }
 
 impl std::fmt::Display for BouncerWeights {
@@ -135,7 +149,6 @@ impl std::fmt::Display for BouncerWeights {
     Clone,
     Copy,
     Debug,
-    Default,
     derive_more::Add,
     derive_more::AddAssign,
     derive_more::Sub,
@@ -199,6 +212,21 @@ impl BuiltinCount {
             range_check96: usize::MAX,
         }
     }
+
+    pub fn empty() -> Self {
+        Self {
+            add_mod: 0,
+            bitwise: 0,
+            ecdsa: 0,
+            ec_op: 0,
+            keccak: 0,
+            mul_mod: 0,
+            pedersen: 0,
+            poseidon: 0,
+            range_check: 0,
+            range_check96: 0,
+        }
+    }
 }
 
 impl From<HashMapWrapper> for BuiltinCount {
@@ -247,7 +275,7 @@ impl std::fmt::Display for BuiltinCount {
     }
 }
 
-#[derive(Debug, Default, PartialEq)]
+#[derive(Debug, PartialEq)]
 #[cfg_attr(test, derive(Clone))]
 pub struct Bouncer {
     // Additional info; maintained and used to calculate the residual contribution of a transaction
@@ -263,7 +291,17 @@ pub struct Bouncer {
 
 impl Bouncer {
     pub fn new(bouncer_config: BouncerConfig) -> Self {
-        Bouncer { bouncer_config, ..Default::default() }
+        Bouncer { bouncer_config, ..Self::empty() }
+    }
+
+    pub fn empty() -> Self {
+        Bouncer {
+            executed_class_hashes: HashSet::default(),
+            visited_storage_entries: HashSet::default(),
+            state_changes_keys: StateChangesKeys::default(),
+            bouncer_config: BouncerConfig::empty(),
+            accumulated_weights: BouncerWeights::empty(),
+        }
     }
 
     pub fn get_accumulated_weights(&self) -> &BouncerWeights {

diff --git a/crates/blockifier/src/bouncer_test.rs b/crates/blockifier/src/bouncer_test.rs
@@ -226,7 +226,7 @@ fn test_bouncer_try_update(#[case] added_ecdsa: usize, #[case] scenario: &'stati
         state_diff_size: 10,
     };
 
-    let mut bouncer = Bouncer { accumulated_weights, bouncer_config, ..Default::default() };
+    let mut bouncer = Bouncer { accumulated_weights, bouncer_config, ..Bouncer::empty() };
 
     // Prepare the resources to be added to the bouncer.
     let execution_summary = ExecutionSummary { ..Default::default() };
@@ -263,7 +263,7 @@ fn test_bouncer_try_update(#[case] added_ecdsa: usize, #[case] scenario: &'stati
     )
     .map_err(TransactionExecutorError::TransactionExecutionError);
     let expected_weights =
-        BouncerWeights { builtin_count: builtin_counter.into(), ..Default::default() };
+        BouncerWeights { builtin_count: builtin_counter.into(), ..BouncerWeights::empty() };
 
     if result.is_ok() {
         // Try to update the bouncer.

diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs
@@ -260,21 +260,8 @@ impl L1HandlerTransaction {
     }
 }
 
-impl BouncerConfig {
-    pub fn empty() -> Self {
-        Self { block_max_capacity: BouncerWeights::empty() }
-    }
-}
-
 impl BouncerWeights {
-    pub fn empty() -> Self {
-        Self {
-            n_events: 0,
-            builtin_count: BuiltinCount::default(),
-            gas: 0,
-            message_segment_length: 0,
-            n_steps: 0,
-            state_diff_size: 0,
-        }
+    pub fn create_for_testing(builtin_count: BuiltinCount) -> Self {
+        Self { builtin_count, ..Self::empty() }
     }
 }