Refactor Transaction enum (0xSpaceShard#411)

* refactor starknet response variants to be created based on return type not on method * added todo statement to not forget * rename Transaction enum to TransactionWithHash * refactor transaction with hash to be a struct and contain two types: transaction_hash and transaction * remove transaction hash from transaction variants except for l1 handler transaction * clippy + test fixes * refactor l1 transactions * clippy * added deny_unknown_fields * test fixes * chain id returns felt type instead of string * clippy fix * rename enum variants from Version<N> to V<N>
Hanae1963 · Apr 4, 2024 · 0a0d6ec · 0a0d6ec
1 parent 8131d0a
commit 0a0d6ec
Show file tree

Hide file tree

Showing 29 changed files with 308 additions and 424 deletions.
diff --git a/crates/starknet-devnet-core/src/messaging/mod.rs b/crates/starknet-devnet-core/src/messaging/mod.rs
@@ -35,7 +35,7 @@ use std::collections::HashMap;
 use starknet_rs_core::types::{BlockId, ExecutionResult, Hash256};
 use starknet_types::felt::TransactionHash;
 use starknet_types::rpc::messaging::{MessageToL1, MessageToL2};
-use starknet_types::rpc::transactions::L1HandlerTransaction;
+use starknet_types::rpc::transactions::l1_handler_transaction::L1HandlerTransaction;
 
 use crate::error::{DevnetResult, Error, MessagingError};
 use crate::starknet::Starknet;
@@ -225,15 +225,13 @@ impl Starknet {
         &mut self,
         messages: &[MessageToL2],
     ) -> DevnetResult<Vec<TransactionHash>> {
-        let chain_id = self.chain_id().to_felt();
         let mut transactions_hashes = vec![];
 
         for message in messages {
-            let transaction =
-                L1HandlerTransaction::try_from_message_to_l2(message.clone())?.with_hash(chain_id);
-            transactions_hashes.push(transaction.transaction_hash);
+            let transaction = L1HandlerTransaction::try_from_message_to_l2(message.clone())?;
 
-            self.add_l1_handler_transaction(transaction)?;
+            let transaction_hash = self.add_l1_handler_transaction(transaction)?;
+            transactions_hashes.push(transaction_hash);
         }
 
         Ok(transactions_hashes)

diff --git a/crates/starknet-devnet-core/src/starknet/add_declare_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_declare_transaction.rs
@@ -3,9 +3,11 @@ use starknet_types::felt::{ClassHash, TransactionHash};
 use starknet_types::rpc::transactions::broadcasted_declare_transaction_v1::BroadcastedDeclareTransactionV1;
 use starknet_types::rpc::transactions::broadcasted_declare_transaction_v2::BroadcastedDeclareTransactionV2;
 use starknet_types::rpc::transactions::broadcasted_declare_transaction_v3::BroadcastedDeclareTransactionV3;
+use starknet_types::rpc::transactions::declare_transaction_v0v1::DeclareTransactionV0V1;
+use starknet_types::rpc::transactions::declare_transaction_v2::DeclareTransactionV2;
 use starknet_types::rpc::transactions::declare_transaction_v3::DeclareTransactionV3;
 use starknet_types::rpc::transactions::{
-    BroadcastedDeclareTransaction, DeclareTransaction, Transaction,
+    BroadcastedDeclareTransaction, DeclareTransaction, Transaction, TransactionWithHash,
 };
 
 use super::dump::DumpEvent;
@@ -26,12 +28,13 @@ pub fn add_declare_transaction_v3(
     let transaction_hash = blockifier_declare_transaction.tx_hash().0.into();
     let class_hash = blockifier_declare_transaction.class_hash().0.into();
 
-    let transaction =
-        Transaction::Declare(DeclareTransaction::Version3(DeclareTransactionV3::new(
-            broadcasted_declare_transaction.clone(),
+    let transaction = TransactionWithHash::new(
+        transaction_hash,
+        Transaction::Declare(DeclareTransaction::V3(DeclareTransactionV3::new(
+            &broadcasted_declare_transaction,
             class_hash,
-            transaction_hash,
-        )));
+        ))),
+    );
 
     let blockifier_execution_result =
         blockifier::transaction::account_transaction::AccountTransaction::Declare(
@@ -65,9 +68,13 @@ pub fn add_declare_transaction_v2(
     let transaction_hash = blockifier_declare_transaction.tx_hash().0.into();
     let class_hash = blockifier_declare_transaction.class_hash().0.into();
 
-    let transaction = Transaction::Declare(DeclareTransaction::Version2(
-        broadcasted_declare_transaction.create_declare(class_hash, transaction_hash),
-    ));
+    let transaction = TransactionWithHash::new(
+        transaction_hash,
+        Transaction::Declare(DeclareTransaction::V2(DeclareTransactionV2::new(
+            &broadcasted_declare_transaction,
+            class_hash,
+        ))),
+    );
 
     let blockifier_execution_result =
         blockifier::transaction::account_transaction::AccountTransaction::Declare(
@@ -100,8 +107,12 @@ pub fn add_declare_transaction_v1(
         .calculate_transaction_hash(&starknet.config.chain_id.to_felt(), &class_hash)?;
 
     let declare_transaction =
-        broadcasted_declare_transaction.create_declare(class_hash, transaction_hash);
-    let transaction = Transaction::Declare(DeclareTransaction::Version1(declare_transaction));
+        DeclareTransactionV0V1::new(&broadcasted_declare_transaction, class_hash);
+
+    let transaction = TransactionWithHash::new(
+        transaction_hash,
+        Transaction::Declare(DeclareTransaction::V1(declare_transaction)),
+    );
 
     let blockifier_declare_transaction =
         broadcasted_declare_transaction.create_blockifier_declare(class_hash, transaction_hash)?;

diff --git a/crates/starknet-devnet-core/src/starknet/add_deploy_account_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_deploy_account_transaction.rs
@@ -3,9 +3,10 @@ use starknet_types::contract_address::ContractAddress;
 use starknet_types::felt::TransactionHash;
 use starknet_types::rpc::transactions::broadcasted_deploy_account_transaction_v1::BroadcastedDeployAccountTransactionV1;
 use starknet_types::rpc::transactions::broadcasted_deploy_account_transaction_v3::BroadcastedDeployAccountTransactionV3;
+use starknet_types::rpc::transactions::deploy_account_transaction_v1::DeployAccountTransactionV1;
 use starknet_types::rpc::transactions::deploy_account_transaction_v3::DeployAccountTransactionV3;
 use starknet_types::rpc::transactions::{
-    BroadcastedDeployAccountTransaction, DeployAccountTransaction, Transaction,
+    BroadcastedDeployAccountTransaction, DeployAccountTransaction, Transaction, TransactionWithHash,
 };
 
 use super::dump::DumpEvent;
@@ -32,16 +33,16 @@ pub fn add_deploy_account_transaction_v3(
 
     let transaction_hash = blockifier_deploy_account_transaction.tx_hash.0.into();
     let address: ContractAddress = blockifier_deploy_account_transaction.contract_address.into();
-    let deploy_account_transaction_v3 = DeployAccountTransactionV3::new(
-        broadcasted_deploy_account_transaction.clone(),
-        address,
+    let deploy_account_transaction_v3 =
+        DeployAccountTransactionV3::new(&broadcasted_deploy_account_transaction, address);
+
+    let transaction = TransactionWithHash::new(
         transaction_hash,
+        Transaction::DeployAccount(DeployAccountTransaction::V3(Box::new(
+            deploy_account_transaction_v3,
+        ))),
     );
 
-    let transaction = Transaction::DeployAccount(DeployAccountTransaction::Version3(Box::new(
-        deploy_account_transaction_v3,
-    )));
-
     let blockifier_execution_result =
         blockifier::transaction::account_transaction::AccountTransaction::DeployAccount(
             blockifier_deploy_account_transaction,
@@ -75,12 +76,15 @@ pub fn add_deploy_account_transaction_v1(
 
     let transaction_hash = blockifier_deploy_account_transaction.tx_hash.0.into();
     let address: ContractAddress = blockifier_deploy_account_transaction.contract_address.into();
-    let deploy_account_transaction_v1 = broadcasted_deploy_account_transaction
-        .compile_deploy_account_transaction_v1(&transaction_hash, address);
+    let deploy_account_transaction_v1 =
+        DeployAccountTransactionV1::new(&broadcasted_deploy_account_transaction, address);
 
-    let transaction = Transaction::DeployAccount(DeployAccountTransaction::Version1(Box::new(
-        deploy_account_transaction_v1,
-    )));
+    let transaction = TransactionWithHash::new(
+        transaction_hash,
+        Transaction::DeployAccount(DeployAccountTransaction::V1(Box::new(
+            deploy_account_transaction_v1,
+        ))),
+    );
 
     let blockifier_execution_result =
         blockifier::transaction::account_transaction::AccountTransaction::DeployAccount(

diff --git a/crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs
@@ -2,9 +2,10 @@ use blockifier::transaction::transactions::ExecutableTransaction;
 use starknet_types::felt::TransactionHash;
 use starknet_types::rpc::transactions::broadcasted_invoke_transaction_v1::BroadcastedInvokeTransactionV1;
 use starknet_types::rpc::transactions::broadcasted_invoke_transaction_v3::BroadcastedInvokeTransactionV3;
+use starknet_types::rpc::transactions::invoke_transaction_v1::InvokeTransactionV1;
 use starknet_types::rpc::transactions::invoke_transaction_v3::InvokeTransactionV3;
 use starknet_types::rpc::transactions::{
-    BroadcastedInvokeTransaction, InvokeTransaction, Transaction,
+    BroadcastedInvokeTransaction, InvokeTransaction, Transaction, TransactionWithHash,
 };
 
 use super::dump::DumpEvent;
@@ -23,9 +24,11 @@ pub fn add_invoke_transaction_v1(
         .create_blockifier_invoke_transaction(starknet.chain_id().to_felt(), false)?;
     let transaction_hash = blockifier_invoke_transaction.tx_hash.0.into();
 
-    let invoke_transaction =
-        broadcasted_invoke_transaction.create_invoke_transaction(transaction_hash);
-    let transaction = Transaction::Invoke(InvokeTransaction::Version1(invoke_transaction));
+    let invoke_transaction = InvokeTransactionV1::new(&broadcasted_invoke_transaction);
+    let transaction = TransactionWithHash::new(
+        transaction_hash,
+        Transaction::Invoke(InvokeTransaction::V1(invoke_transaction)),
+    );
 
     let blockifier_execution_result =
         blockifier::transaction::account_transaction::AccountTransaction::Invoke(
@@ -60,10 +63,12 @@ pub fn add_invoke_transaction_v3(
         )
         .execute(&mut starknet.state.state, &starknet.block_context, true, true);
 
-    let transaction = Transaction::Invoke(InvokeTransaction::Version3(InvokeTransactionV3::new(
-        broadcasted_invoke_transaction.clone(),
+    let transaction = TransactionWithHash::new(
         transaction_hash,
-    )));
+        Transaction::Invoke(InvokeTransaction::V3(InvokeTransactionV3::new(
+            &broadcasted_invoke_transaction,
+        ))),
+    );
 
     starknet.handle_transaction_result(transaction, None, blockifier_execution_result)?;
     starknet.handle_dump_event(DumpEvent::AddInvokeTransaction(

diff --git a/crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_l1_handler_transaction.rs
@@ -1,6 +1,7 @@
 use blockifier::transaction::transactions::ExecutableTransaction;
 use starknet_types::felt::TransactionHash;
-use starknet_types::rpc::transactions::{L1HandlerTransaction, Transaction};
+use starknet_types::rpc::transactions::l1_handler_transaction::L1HandlerTransaction;
+use starknet_types::rpc::transactions::{Transaction, TransactionWithHash};
 use tracing::trace;
 
 use super::Starknet;
@@ -11,10 +12,10 @@ pub fn add_l1_handler_transaction(
     starknet: &mut Starknet,
     transaction: L1HandlerTransaction,
 ) -> DevnetResult<TransactionHash> {
-    let transaction_hash = transaction.transaction_hash;
-    trace!("Executing L1 handler transaction [{:#064x}]", transaction.transaction_hash);
-
-    let blockifier_transaction = transaction.create_blockifier_transaction()?;
+    let blockifier_transaction =
+        transaction.create_blockifier_transaction(starknet.chain_id().to_felt())?;
+    let transaction_hash = blockifier_transaction.tx_hash.0.into();
+    trace!("Executing L1 handler transaction [{:#064x}]", transaction_hash);
 
     // Fees are charges on L1 as `L1HandlerTransaction` is not executed by an
     // account, but directly by the sequencer.
@@ -30,7 +31,7 @@ pub fn add_l1_handler_transaction(
     );
 
     starknet.handle_transaction_result(
-        Transaction::L1Handler(transaction.clone()),
+        TransactionWithHash::new(transaction_hash, Transaction::L1Handler(transaction.clone())),
         None,
         blockifier_execution_result,
     )?;
@@ -55,7 +56,7 @@ mod tests {
     use starknet_types::contract_class::{Cairo0ContractClass, ContractClass};
     use starknet_types::felt::Felt;
     use starknet_types::rpc::state::Balance;
-    use starknet_types::rpc::transactions::L1HandlerTransaction;
+    use starknet_types::rpc::transactions::l1_handler_transaction::L1HandlerTransaction;
     use starknet_types::traits::HashProducer;
 
     use crate::account::Account;
@@ -91,16 +92,17 @@ mod tests {
             nonce: nonce.into(),
             paid_fee_on_l1: fee,
             ..Default::default()
-        }
-        .with_hash(ChainId::Testnet.to_felt());
+        };
+
+        let l1_handler_transaction_hash = transaction.compute_hash(ChainId::Testnet.to_felt());
 
         let transaction_hash = Felt::from_prefixed_hex_str(
             "0x1b24ea8dd9e0cb603043958b27a8569635ea13568883cc155130591b7ffe37a",
         )
         .unwrap();
 
         assert_eq!(transaction.version, Felt::from(0));
-        assert_eq!(transaction.transaction_hash, transaction_hash);
+        assert_eq!(l1_handler_transaction_hash, transaction_hash);
     }
 
     #[test]
@@ -174,7 +176,6 @@ mod tests {
             paid_fee_on_l1: fee,
             ..Default::default()
         }
-        .with_hash(ChainId::Testnet.to_felt())
     }
 
     /// Initializes a starknet object with: l1l2 dummy contract that has two functions for

diff --git a/crates/starknet-devnet-core/src/starknet/dump.rs b/crates/starknet-devnet-core/src/starknet/dump.rs
@@ -3,9 +3,10 @@ use std::io::{Read, Seek, SeekFrom, Write};
 use std::path::Path;
 
 use serde::{Deserialize, Serialize};
+use starknet_types::rpc::transactions::l1_handler_transaction::L1HandlerTransaction;
 use starknet_types::rpc::transactions::{
     BroadcastedDeclareTransaction, BroadcastedDeployAccountTransaction,
-    BroadcastedInvokeTransaction, L1HandlerTransaction,
+    BroadcastedInvokeTransaction,
 };
 
 use super::{DumpOn, Starknet};

diff --git a/crates/starknet-devnet-core/src/starknet/events.rs b/crates/starknet-devnet-core/src/starknet/events.rs
@@ -130,7 +130,6 @@ mod tests {
     use starknet_types::contract_address::ContractAddress;
     use starknet_types::emitted_event::Event;
     use starknet_types::felt::Felt;
-    use starknet_types::rpc::transactions::{DeclareTransaction, Transaction};
 
     use super::{check_if_filter_applies_for_event, get_events};
     use crate::starknet::events::check_if_filter_applies_for_event_keys;
@@ -394,10 +393,9 @@ mod tests {
         // each transaction should have events count equal to the order of the transaction
         let mut starknet = Starknet::new(&StarknetConfig::default()).unwrap();
 
-        for idx in 0..5 {
-            let transaction =
-                Transaction::Declare(DeclareTransaction::Version1(dummy_declare_transaction_v1()));
+        let transaction = dummy_declare_transaction_v1();
 
+        for idx in 0..5 {
             let txn_info = blockifier::transaction::objects::TransactionExecutionInfo {
                 execute_call_info: Some(dummy_call_info(idx + 1)),
                 ..Default::default()