fix(consensus): calculate the executable_transaction in consensus_context

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs

@@ -39,6 +39,7 @@ use starknet_api::block::{
     GasPrices,
     NonzeroGasPrice,
 };
+use starknet_api::core::ChainId;
 use starknet_api::executable_transaction::Transaction as ExecutableTransaction;
 use starknet_batcher_types::batcher_types::{
     DecisionReachedInput,
@@ -104,6 +105,8 @@ pub struct SequencerConsensusContext {
     outbound_proposal_sender: mpsc::Sender<(u64, mpsc::Receiver<ProposalPart>)>,
     // Used to broadcast votes to other consensus nodes.
     vote_broadcast_client: BroadcastTopicClient<ConsensusMessage>,
+    // Used to convert Transaction to ExecutableTransaction.
+    chain_id: ChainId,
 }
 
 impl SequencerConsensusContext {
@@ -112,6 +115,7 @@ impl SequencerConsensusContext {
         outbound_proposal_sender: mpsc::Sender<(u64, mpsc::Receiver<ProposalPart>)>,
         vote_broadcast_client: BroadcastTopicClient<ConsensusMessage>,
         num_validators: u64,
+        chain_id: ChainId,
     ) -> Self {
         Self {
             batcher,
@@ -127,6 +131,7 @@ impl SequencerConsensusContext {
             current_round: 0,
             active_proposal: None,
             queued_proposals: BTreeMap::new(),
+            chain_id,
         }
     }
 }
@@ -390,6 +395,7 @@ impl SequencerConsensusContext {
 
         let notify = Arc::new(Notify::new());
         let notify_clone = Arc::clone(&notify);
+        let chain_id = self.chain_id.clone();
 
         let handle = tokio::spawn(
             async move {
@@ -400,6 +406,7 @@ impl SequencerConsensusContext {
                     valid_proposals,
                     content_receiver,
                     fin_sender,
+                    chain_id,
                 );
                 tokio::select! {
                     _ = notify_clone.notified() => {}
@@ -513,6 +520,7 @@ async fn stream_validate_proposal(
     valid_proposals: Arc<Mutex<HeightToIdToContent>>,
     mut content_receiver: mpsc::Receiver<ProposalPart>,
     fin_sender: oneshot::Sender<(ProposalContentId, ProposalFin)>,
+    chain_id: ChainId,
 ) {
     let mut content = Vec::new();
     let network_block_id = loop {
@@ -522,11 +530,15 @@ async fn stream_validate_proposal(
             return;
         };
         match prop_part {
-            ProposalPart::Transactions(TransactionBatch { transactions: txs, tx_hashes }) => {
+            ProposalPart::Transactions(TransactionBatch { transactions: txs, tx_hashes: _ }) => {
                 let exe_txs: Vec<ExecutableTransaction> = txs
                     .into_iter()
-                    .zip(tx_hashes.into_iter())
-                    .map(|tx_tup| tx_tup.into())
+                    .map(|tx| {
+                        // An error means we have an invalid chain_id.
+                        (tx, &chain_id)
+                            .try_into()
+                            .expect("Failed to convert transaction to executable_transation.")
+                    })
                     .collect();
                 content.extend_from_slice(&exe_txs[..]);
                 let input = SendProposalContentInput {

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context_test.rs

@@ -22,10 +22,11 @@ use papyrus_protobuf::consensus::{
     TransactionBatch,
 };
 use starknet_api::block::{BlockHash, BlockNumber};
-use starknet_api::core::StateDiffCommitment;
+use starknet_api::core::{ChainId, Nonce, StateDiffCommitment};
 use starknet_api::executable_transaction::Transaction as ExecutableTransaction;
+use starknet_api::felt;
 use starknet_api::hash::PoseidonHash;
-use starknet_api::test_utils::invoke::{executable_invoke_tx, invoke_tx, InvokeTxArgs};
+use starknet_api::test_utils::invoke::{invoke_tx, InvokeTxArgs};
 use starknet_api::transaction::{Transaction, TransactionHash};
 use starknet_batcher_types::batcher_types::{
     GetProposalContent,
@@ -48,21 +49,19 @@ const TIMEOUT: Duration = Duration::from_millis(100);
 const CHANNEL_SIZE: usize = 5000;
 const NUM_VALIDATORS: u64 = 4;
 const STATE_DIFF_COMMITMENT: StateDiffCommitment = StateDiffCommitment(PoseidonHash(Felt::ZERO));
+const CHAIN_ID: ChainId = ChainId::Mainnet;
 
 lazy_static! {
-    static ref TX_BATCH: Vec<ExecutableTransaction> =
-        vec![generate_executable_invoke_tx(Felt::THREE)];
+    static ref TX_BATCH: Vec<ExecutableTransaction> = vec![generate_executable_invoke_tx()];
 }
 
 fn generate_invoke_tx() -> Transaction {
-    Transaction::Invoke(invoke_tx(InvokeTxArgs::default()))
+    Transaction::Invoke(invoke_tx(InvokeTxArgs { nonce: Nonce(felt!(3_u8)), ..Default::default() }))
 }
 
-fn generate_executable_invoke_tx(tx_hash: Felt) -> ExecutableTransaction {
-    ExecutableTransaction::Account(executable_invoke_tx(InvokeTxArgs {
-        tx_hash: TransactionHash(tx_hash),
-        ..Default::default()
-    }))
+fn generate_executable_invoke_tx() -> ExecutableTransaction {
+    let tx = generate_invoke_tx();
+    (tx, &CHAIN_ID).try_into().unwrap()
 }
 
 // Structs which aren't utilized but should not be dropped.
@@ -91,6 +90,7 @@ fn setup(batcher: MockBatcherClient) -> (SequencerConsensusContext, NetworkDepen
         outbound_proposal_stream_sender,
         votes_topic_client,
         NUM_VALIDATORS,
+        CHAIN_ID,
     );
 
     let network_dependencies = NetworkDependencies {

crates/starknet_api/src/transaction.rs

@@ -135,22 +135,25 @@ impl From<executable_transaction::Transaction> for Transaction {
     }
 }
 
-impl From<(Transaction, TransactionHash)> for executable_transaction::Transaction {
-    fn from((tx, tx_hash): (Transaction, TransactionHash)) -> Self {
+impl TryFrom<(Transaction, &ChainId)> for executable_transaction::Transaction {
+    type Error = StarknetApiError;
+
+    fn try_from((tx, chain_id): (Transaction, &ChainId)) -> Result<Self, Self::Error> {
+        let tx_hash = tx.calculate_transaction_hash(chain_id)?;
         match tx {
-            Transaction::Invoke(tx) => executable_transaction::Transaction::Account(
+            Transaction::Invoke(tx) => Ok(executable_transaction::Transaction::Account(
                 executable_transaction::AccountTransaction::Invoke(
                     executable_transaction::InvokeTransaction { tx, tx_hash },
                 ),
-            ),
-            Transaction::L1Handler(tx) => executable_transaction::Transaction::L1Handler(
+            )),
+            Transaction::L1Handler(tx) => Ok(executable_transaction::Transaction::L1Handler(
                 executable_transaction::L1HandlerTransaction {
                     tx,
                     tx_hash,
                     // TODO (yael 1/12/2024): The paid fee should be an input from the l1_handler.
                     paid_fee_on_l1: Fee(1),
                 },
-            ),
+            )),
             _ => {
                 unimplemented!(
                     "Unsupported transaction type. Only Invoke and L1Handler are currently \

crates/starknet_api/src/transaction_test.rs

@@ -1,25 +1,34 @@
-use assert_matches::assert_matches;
 use rstest::{fixture, rstest};
 
-use super::{Transaction, TransactionHash};
+use super::Transaction;
 use crate::block::NonzeroGasPrice;
-use crate::executable_transaction::Transaction as ExecutableTransaction;
+use crate::core::ChainId;
+use crate::executable_transaction::{
+    AccountTransaction,
+    InvokeTransaction,
+    L1HandlerTransaction,
+    Transaction as ExecutableTransaction,
+};
 use crate::execution_resources::GasAmount;
 use crate::test_utils::{read_json_file, TransactionTestData};
 use crate::transaction::Fee;
 
+const CHAIN_ID: ChainId = ChainId::Mainnet;
+
 #[fixture]
 fn transactions_data() -> Vec<TransactionTestData> {
     // The details were taken from Starknet Mainnet. You can find the transactions by hash in:
     // https://alpha-mainnet.starknet.io/feeder_gateway/get_transaction?transactionHash=<transaction_hash>
     serde_json::from_value(read_json_file("transaction_hash.json")).unwrap()
 }
 
-fn verify_transaction_conversion(tx: Transaction, tx_hash: TransactionHash) {
-    let executable_tx: ExecutableTransaction = (tx.clone(), tx_hash).into();
-    let reconverted_tx = Transaction::from(executable_tx);
+fn verify_transaction_conversion(tx: &Transaction, expected_executable_tx: ExecutableTransaction) {
+    let converted_executable_tx: ExecutableTransaction =
+        (tx.clone(), &CHAIN_ID).try_into().unwrap();
+    let reconverted_tx = Transaction::from(converted_executable_tx.clone());
 
-    assert_eq!(tx, reconverted_tx);
+    assert_eq!(converted_executable_tx, expected_executable_tx);
+    assert_eq!(tx, &reconverted_tx);
 }
 
 #[test]
@@ -50,14 +59,17 @@ fn test_fee_div_ceil() {
 fn test_invoke_executable_transaction_conversion(mut transactions_data: Vec<TransactionTestData>) {
     // Extract Invoke transaction data.
     let transaction_data = transactions_data.remove(0);
-    let tx = assert_matches!(
-        transaction_data.transaction,
-        Transaction::Invoke(tx) => Transaction::Invoke(tx),
-        "Transaction_hash.json is expected to have Invoke as the first transaction."
-    );
-    let tx_hash = transaction_data.transaction_hash;
+    let Transaction::Invoke(invoke_tx) = transaction_data.transaction.clone() else {
+        panic!("Transaction_hash.json is expected to have Invoke as the first transaction.")
+    };
+
+    let expected_executable_tx =
+        ExecutableTransaction::Account(AccountTransaction::Invoke(InvokeTransaction {
+            tx: invoke_tx,
+            tx_hash: transaction_data.transaction_hash,
+        }));
 
-    verify_transaction_conversion(tx, tx_hash);
+    verify_transaction_conversion(&transaction_data.transaction, expected_executable_tx);
 }
 
 #[rstest]
@@ -66,12 +78,15 @@ fn test_l1_handler_executable_transaction_conversion(
 ) {
     // Extract L1 Handler transaction data.
     let transaction_data = transactions_data.remove(10);
-    let tx = assert_matches!(
-        transaction_data.transaction,
-        Transaction::L1Handler(tx) => Transaction::L1Handler(tx),
-        "Transaction_hash.json is expected to have L1 handler as the 11th transaction."
-    );
-    let tx_hash = transaction_data.transaction_hash;
+    let Transaction::L1Handler(l1_handler_tx) = transaction_data.transaction.clone() else {
+        panic!("Transaction_hash.json is expected to have L1 Handler as the 11th transaction.")
+    };
+
+    let expected_executable_tx = ExecutableTransaction::L1Handler(L1HandlerTransaction {
+        tx: l1_handler_tx,
+        tx_hash: transaction_data.transaction_hash,
+        paid_fee_on_l1: Fee(1),
+    });
 
-    verify_transaction_conversion(tx, tx_hash);
+    verify_transaction_conversion(&transaction_data.transaction, expected_executable_tx);
 }

crates/starknet_consensus_manager/src/consensus_manager.rs

@@ -68,6 +68,7 @@ impl ConsensusManager {
             outbound_internal_sender,
             votes_broadcast_channels.broadcast_topic_client.clone(),
             self.config.consensus_config.num_validators,
+            self.config.consensus_config.chain_id.clone(),
         );
 
         let mut network_handle = tokio::task::spawn(network_manager.run());