From 42c5799b660ae1246de7c2a5aef23f860b9aa8b6 Mon Sep 17 00:00:00 2001 From: Yael Doweck Date: Wed, 4 Dec 2024 15:45:51 +0200 Subject: [PATCH] feat(consensus): calculate the executable_transaction in consensus_context --- .../src/sequencer_consensus_context.rs | 18 +++++- .../src/sequencer_consensus_context_test.rs | 20 +++---- crates/starknet_api/src/transaction.rs | 15 +++-- crates/starknet_api/src/transaction_test.rs | 57 ++++++++++++------- .../src/consensus_manager.rs | 1 + 5 files changed, 71 insertions(+), 40 deletions(-) diff --git a/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs b/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs index 92e45cd85d..12ba86891a 100644 --- a/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs +++ b/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)>, // Used to broadcast votes to other consensus nodes. vote_broadcast_client: BroadcastTopicClient, + // 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)>, vote_broadcast_client: BroadcastTopicClient, 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(¬ify); + 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>, mut content_receiver: mpsc::Receiver, 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 = 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 { diff --git a/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context_test.rs b/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context_test.rs index 085cfed39d..f72bde01da 100644 --- a/crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context_test.rs +++ b/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 = - vec![generate_executable_invoke_tx(Felt::THREE)]; + static ref TX_BATCH: Vec = 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 { diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index b2af549672..b2bf7e3808 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -135,22 +135,25 @@ impl From 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 { + 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 \ diff --git a/crates/starknet_api/src/transaction_test.rs b/crates/starknet_api/src/transaction_test.rs index 5eb6e04ed2..9ff28c5858 100644 --- a/crates/starknet_api/src/transaction_test.rs +++ b/crates/starknet_api/src/transaction_test.rs @@ -1,13 +1,20 @@ -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 { // The details were taken from Starknet Mainnet. You can find the transactions by hash in: @@ -15,11 +22,13 @@ fn transactions_data() -> Vec { 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) { // 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); } diff --git a/crates/starknet_consensus_manager/src/consensus_manager.rs b/crates/starknet_consensus_manager/src/consensus_manager.rs index d043566607..9a6991e797 100644 --- a/crates/starknet_consensus_manager/src/consensus_manager.rs +++ b/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());