Skip to content

Commit

Permalink
chore(consensus): remove the tx_hash from transactionBatch
Browse files Browse the repository at this point in the history
  • Loading branch information
Yael-Starkware committed Dec 17, 2024
1 parent 64b08ed commit 2f37611
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 68 deletions.
5 changes: 1 addition & 4 deletions crates/papyrus_protobuf/src/consensus.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::core::ContractAddress;
use starknet_api::transaction::{Transaction, TransactionHash};
use starknet_api::transaction::Transaction;

use crate::converters::ProtobufConversionError;

Expand Down Expand Up @@ -77,9 +77,6 @@ pub struct ProposalInit {
pub struct TransactionBatch {
/// The transactions in the batch.
pub transactions: Vec<Transaction>,
// TODO(guyn): remove this once we know how to get hashes as part of the compilation.
/// The transaction's hashes.
pub tx_hashes: Vec<TransactionHash>,
}

/// The proposal is done when receiving this fin message, which contains the block hash.
Expand Down
15 changes: 3 additions & 12 deletions crates/papyrus_protobuf/src/converters/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use std::convert::{TryFrom, TryInto};
use prost::Message;
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::hash::StarkHash;
use starknet_api::transaction::{Transaction, TransactionHash};
use starknet_types_core::felt::Felt;
use starknet_api::transaction::Transaction;

use crate::consensus::{
ConsensusMessage,
Expand Down Expand Up @@ -227,8 +226,6 @@ impl From<ProposalInit> for protobuf::ProposalInit {

auto_impl_into_and_try_from_vec_u8!(ProposalInit, protobuf::ProposalInit);

// TODO(guyn): remove tx_hashes once we know how to compile the hashes
// when making the executable transactions.
impl TryFrom<protobuf::TransactionBatch> for TransactionBatch {
type Error = ProtobufConversionError;
fn try_from(value: protobuf::TransactionBatch) -> Result<Self, Self::Error> {
Expand All @@ -237,20 +234,14 @@ impl TryFrom<protobuf::TransactionBatch> for TransactionBatch {
.into_iter()
.map(|tx| tx.try_into())
.collect::<Result<Vec<Transaction>, ProtobufConversionError>>()?;
let tx_hashes = value
.tx_hashes
.into_iter()
.map(|x| Felt::try_from(x).map(TransactionHash))
.collect::<Result<_, Self::Error>>()?;
Ok(TransactionBatch { transactions, tx_hashes })
Ok(TransactionBatch { transactions })
}
}

impl From<TransactionBatch> for protobuf::TransactionBatch {
fn from(value: TransactionBatch) -> Self {
let transactions = value.transactions.into_iter().map(Into::into).collect();
let tx_hashes = value.tx_hashes.into_iter().map(|hash| hash.0.into()).collect();
protobuf::TransactionBatch { transactions, tx_hashes }
protobuf::TransactionBatch { transactions }
}
}

Expand Down
3 changes: 1 addition & 2 deletions crates/papyrus_protobuf/src/converters/test_instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use papyrus_test_utils::{auto_impl_get_test_instance, get_number_of_variants, Ge
use rand::Rng;
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::core::ContractAddress;
use starknet_api::transaction::{Transaction, TransactionHash};
use starknet_api::transaction::Transaction;

use crate::consensus::{
ConsensusMessage,
Expand Down Expand Up @@ -52,7 +52,6 @@ auto_impl_get_test_instance! {
}
pub struct TransactionBatch {
pub transactions: Vec<Transaction>,
pub tx_hashes: Vec<TransactionHash>,
}
pub enum ProposalPart {
Init(ProposalInit) = 0,
Expand Down
2 changes: 0 additions & 2 deletions crates/papyrus_protobuf/src/proto/p2p/proto/consensus.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ message ProposalInit {

message TransactionBatch {
repeated Transaction transactions = 1;
// TODO(guyn): remove this once we know how to calculate hashes
repeated Felt252 tx_hashes = 2;
}

message ProposalFin {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ impl ConsensusContext for PapyrusConsensusContext {
proposal_sender
.send(ProposalPart::Transactions(TransactionBatch {
transactions: transactions.clone(),
tx_hashes: vec![],
}))
.await
.expect("Failed to send transactions");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ async fn validate_proposal_success() {

let (mut validate_sender, validate_receiver) = mpsc::channel(TEST_CHANNEL_SIZE);
for tx in block.body.transactions.clone() {
let tx_part = ProposalPart::Transactions(TransactionBatch {
transactions: vec![tx],
tx_hashes: vec![],
});
let tx_part = ProposalPart::Transactions(TransactionBatch { transactions: vec![tx] });
validate_sender.try_send(tx_part).unwrap();
}
let fin_part = ProposalPart::Fin(ProposalFin { proposal_content_id: block.header.block_hash });
Expand Down Expand Up @@ -89,10 +86,7 @@ async fn validate_proposal_fail() {
let different_block = get_test_block(4, None, None, None);
let (mut validate_sender, validate_receiver) = mpsc::channel(5000);
for tx in different_block.body.transactions.clone() {
let tx_part = ProposalPart::Transactions(TransactionBatch {
transactions: vec![tx],
tx_hashes: vec![],
});
let tx_part = ProposalPart::Transactions(TransactionBatch { transactions: vec![tx] });
validate_sender.try_send(tx_part).unwrap();
}
validate_sender.close_channel();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use starknet_api::block::{
};
use starknet_api::core::ChainId;
use starknet_api::executable_transaction::Transaction as ExecutableTransaction;
use starknet_api::transaction::{Transaction, TransactionHash};
use starknet_batcher_types::batcher_types::{
DecisionReachedInput,
GetProposalContent,
Expand Down Expand Up @@ -459,19 +460,16 @@ async fn stream_build_proposal(
// TODO: Broadcast the transactions to the network.
// TODO(matan): Convert to protobuf and make sure this isn't too large for a single
// proto message (could this be a With adapter added to the channel in `new`?).
let mut transaction_hashes = Vec::with_capacity(txs.len());
let mut transactions = Vec::with_capacity(txs.len());
for tx in txs.into_iter() {
transaction_hashes.push(tx.tx_hash());
transactions.push(tx.into());
}
let transaction_hashes =
txs.iter().map(|tx| tx.tx_hash()).collect::<Vec<TransactionHash>>();
debug!("Broadcasting proposal content: {transaction_hashes:?}");

let transactions =
txs.into_iter().map(|tx| tx.into()).collect::<Vec<Transaction>>();
trace!("Broadcasting proposal content: {transactions:?}");

proposal_sender
.send(ProposalPart::Transactions(TransactionBatch {
transactions,
tx_hashes: transaction_hashes,
}))
.send(ProposalPart::Transactions(TransactionBatch { transactions }))
.await
.expect("Failed to broadcast proposal content");
}
Expand Down Expand Up @@ -530,7 +528,7 @@ async fn stream_validate_proposal(
return;
};
match prop_part {
ProposalPart::Transactions(TransactionBatch { transactions: txs, tx_hashes: _ }) => {
ProposalPart::Transactions(TransactionBatch { transactions: txs }) => {
let exe_txs: Vec<ExecutableTransaction> = txs
.into_iter()
.map(|tx| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use starknet_api::executable_transaction::Transaction as ExecutableTransaction;
use starknet_api::felt;
use starknet_api::hash::PoseidonHash;
use starknet_api::test_utils::invoke::{invoke_tx, InvokeTxArgs};
use starknet_api::transaction::{Transaction, TransactionHash};
use starknet_api::transaction::Transaction;
use starknet_batcher_types::batcher_types::{
GetProposalContent,
GetProposalContentResponse,
Expand All @@ -52,16 +52,19 @@ const STATE_DIFF_COMMITMENT: StateDiffCommitment = StateDiffCommitment(PoseidonH
const CHAIN_ID: ChainId = ChainId::Mainnet;

lazy_static! {
static ref TX_BATCH: Vec<ExecutableTransaction> = vec![generate_executable_invoke_tx()];
static ref TX_BATCH: Vec<Transaction> = (0..3).map(generate_invoke_tx).collect();
}

fn generate_invoke_tx() -> Transaction {
Transaction::Invoke(invoke_tx(InvokeTxArgs { nonce: Nonce(felt!(3_u8)), ..Default::default() }))
lazy_static! {
static ref EXECUTABLE_TX_BATCH: Vec<ExecutableTransaction> =
TX_BATCH.iter().map(|tx| (tx.clone(), &CHAIN_ID).try_into().unwrap()).collect();
}

fn generate_executable_invoke_tx() -> ExecutableTransaction {
let tx = generate_invoke_tx();
(tx, &CHAIN_ID).try_into().unwrap()
fn generate_invoke_tx(nonce: u8) -> Transaction {
Transaction::Invoke(invoke_tx(InvokeTxArgs {
nonce: Nonce(felt!(nonce)),
..Default::default()
}))
}

// Structs which aren't utilized but should not be dropped.
Expand Down Expand Up @@ -117,7 +120,9 @@ async fn build_proposal() {
let proposal_id_clone = Arc::clone(&proposal_id);
batcher.expect_get_proposal_content().times(1).returning(move |input| {
assert_eq!(input.proposal_id, *proposal_id_clone.get().unwrap());
Ok(GetProposalContentResponse { content: GetProposalContent::Txs(TX_BATCH.clone()) })
Ok(GetProposalContentResponse {
content: GetProposalContent::Txs(EXECUTABLE_TX_BATCH.clone()),
})
});
let proposal_id_clone = Arc::clone(&proposal_id);
batcher.expect_get_proposal_content().times(1).returning(move |input| {
Expand Down Expand Up @@ -157,7 +162,7 @@ async fn validate_proposal_success() {
let SendProposalContent::Txs(txs) = input.content else {
panic!("Expected SendProposalContent::Txs, got {:?}", input.content);
};
assert_eq!(txs, *TX_BATCH);
assert_eq!(txs, *EXECUTABLE_TX_BATCH);
Ok(SendProposalContentResponse { response: ProposalStatus::Processing })
},
);
Expand All @@ -179,14 +184,8 @@ async fn validate_proposal_success() {
context.set_height_and_round(BlockNumber(0), 0).await;

let (mut content_sender, content_receiver) = mpsc::channel(CHANNEL_SIZE);
let tx_hash = TX_BATCH.first().unwrap().tx_hash();
let txs =
TX_BATCH.clone().into_iter().map(starknet_api::transaction::Transaction::from).collect();
content_sender
.send(ProposalPart::Transactions(TransactionBatch {
transactions: txs,
tx_hashes: vec![tx_hash],
}))
.send(ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() }))
.await
.unwrap();
content_sender
Expand Down Expand Up @@ -242,8 +241,7 @@ async fn repropose() {
let (mut content_sender, content_receiver) = mpsc::channel(CHANNEL_SIZE);
content_sender
.send(ProposalPart::Transactions(TransactionBatch {
transactions: vec![generate_invoke_tx()],
tx_hashes: vec![TransactionHash(Felt::TWO)],
transactions: vec![generate_invoke_tx(2)],
}))
.await
.unwrap();
Expand Down Expand Up @@ -294,7 +292,7 @@ async fn proposals_from_different_rounds() {
let SendProposalContent::Txs(txs) = input.content else {
panic!("Expected SendProposalContent::Txs, got {:?}", input.content);
};
assert_eq!(txs, *TX_BATCH);
assert_eq!(txs, *EXECUTABLE_TX_BATCH);
Ok(SendProposalContentResponse { response: ProposalStatus::Processing })
},
);
Expand All @@ -316,10 +314,8 @@ async fn proposals_from_different_rounds() {
context.set_height_and_round(BlockNumber(0), 1).await;

// Proposal parts sent in the proposals.
let prop_part_txs = ProposalPart::Transactions(TransactionBatch {
transactions: TX_BATCH.clone().into_iter().map(Transaction::from).collect(),
tx_hashes: vec![TX_BATCH[0].tx_hash()],
});
let prop_part_txs =
ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() });
let prop_part_fin = ProposalPart::Fin(ProposalFin {
proposal_content_id: BlockHash(STATE_DIFF_COMMITMENT.0.0),
});
Expand Down Expand Up @@ -394,7 +390,7 @@ async fn interrupt_active_proposal() {
.expect_send_proposal_content()
.withf(|input| {
input.proposal_id == ProposalId(1)
&& input.content == SendProposalContent::Txs(TX_BATCH.clone())
&& input.content == SendProposalContent::Txs(EXECUTABLE_TX_BATCH.clone())
})
.times(1)
.returning(move |_| {
Expand Down Expand Up @@ -433,10 +429,7 @@ async fn interrupt_active_proposal() {

let (mut content_sender_1, content_receiver) = mpsc::channel(CHANNEL_SIZE);
content_sender_1
.send(ProposalPart::Transactions(TransactionBatch {
transactions: TX_BATCH.clone().into_iter().map(Transaction::from).collect(),
tx_hashes: vec![TX_BATCH[0].tx_hash()],
}))
.send(ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() }))
.await
.unwrap();
content_sender_1
Expand Down

0 comments on commit 2f37611

Please sign in to comment.