From bd1211a310c5e43556156da89a5d31b9bbdf6475 Mon Sep 17 00:00:00 2001 From: Guy Nir Date: Mon, 9 Dec 2024 11:18:49 +0200 Subject: [PATCH] chore(sequencing): remove ConsensusMessage and Proposal from proto --- crates/papyrus_protobuf/src/consensus.rs | 27 ------- .../src/converters/consensus.rs | 80 ------------------- .../src/converters/consensus_test.rs | 36 +-------- .../src/converters/test_instances.rs | 15 ---- .../src/proto/p2p/proto/consensus.proto | 18 ----- 5 files changed, 2 insertions(+), 174 deletions(-) diff --git a/crates/papyrus_protobuf/src/consensus.rs b/crates/papyrus_protobuf/src/consensus.rs index feb40ee908..8ca665fb04 100644 --- a/crates/papyrus_protobuf/src/consensus.rs +++ b/crates/papyrus_protobuf/src/consensus.rs @@ -4,17 +4,6 @@ use starknet_api::transaction::Transaction; use crate::converters::ProtobufConversionError; -// TODO(guyn): remove this once we integrate ProposalPart everywhere. -#[derive(Debug, Default, Hash, Clone, Eq, PartialEq)] -pub struct Proposal { - pub height: u64, - pub round: u32, - pub proposer: ContractAddress, - pub transactions: Vec, - pub block_hash: BlockHash, - pub valid_round: Option, -} - #[derive(Debug, Default, Hash, Clone, Eq, PartialEq)] pub enum VoteType { Prevote, @@ -31,22 +20,6 @@ pub struct Vote { pub voter: ContractAddress, } -// TODO: remove this once we are sure everything works using just Vote. -#[derive(Debug, Clone, Hash, Eq, PartialEq)] -pub enum ConsensusMessage { - Proposal(Proposal), // To be deprecated - Vote(Vote), -} - -impl ConsensusMessage { - pub fn height(&self) -> u64 { - match self { - ConsensusMessage::Proposal(proposal) => proposal.height, - ConsensusMessage::Vote(vote) => vote.height, - } - } -} - #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub enum StreamMessageBody { Content(T), diff --git a/crates/papyrus_protobuf/src/converters/consensus.rs b/crates/papyrus_protobuf/src/converters/consensus.rs index 6e431e7411..1dad9045b8 100644 --- a/crates/papyrus_protobuf/src/converters/consensus.rs +++ b/crates/papyrus_protobuf/src/converters/consensus.rs @@ -9,8 +9,6 @@ use starknet_api::hash::StarkHash; use starknet_api::transaction::Transaction; use crate::consensus::{ - ConsensusMessage, - Proposal, ProposalFin, ProposalInit, ProposalPart, @@ -23,51 +21,6 @@ use crate::consensus::{ use crate::converters::ProtobufConversionError; use crate::{auto_impl_into_and_try_from_vec_u8, protobuf}; -// TODO(guyn): remove this once we integrate ProposalPart everywhere. -impl TryFrom for Proposal { - type Error = ProtobufConversionError; - - fn try_from(value: protobuf::Proposal) -> Result { - let transactions = value - .transactions - .into_iter() - .map(|tx| tx.try_into()) - .collect::, ProtobufConversionError>>()?; - - let height = value.height; - let round = value.round; - let proposer = value - .proposer - .ok_or(ProtobufConversionError::MissingField { field_description: "proposer" })? - .try_into()?; - let block_hash: StarkHash = value - .block_hash - .ok_or(ProtobufConversionError::MissingField { field_description: "block_hash" })? - .try_into()?; - let block_hash = BlockHash(block_hash); - let valid_round = value.valid_round; - - Ok(Proposal { height, round, proposer, transactions, block_hash, valid_round }) - } -} - -impl From for protobuf::Proposal { - fn from(value: Proposal) -> Self { - let transactions = value.transactions.into_iter().map(Into::into).collect(); - - protobuf::Proposal { - height: value.height, - round: value.round, - proposer: Some(value.proposer.into()), - transactions, - block_hash: Some(value.block_hash.0.into()), - valid_round: value.valid_round, - } - } -} - -auto_impl_into_and_try_from_vec_u8!(Proposal, protobuf::Proposal); - impl TryFrom for VoteType { type Error = ProtobufConversionError; @@ -304,36 +257,3 @@ impl From for protobuf::ProposalPart { } auto_impl_into_and_try_from_vec_u8!(ProposalPart, protobuf::ProposalPart); - -// TODO(guyn): remove this once we are happy with how proposals are sent separate from votes. -impl TryFrom for ConsensusMessage { - type Error = ProtobufConversionError; - - fn try_from(value: protobuf::ConsensusMessage) -> Result { - use protobuf::consensus_message::Message; - - let Some(message) = value.message else { - return Err(ProtobufConversionError::MissingField { field_description: "message" }); - }; - - match message { - Message::Proposal(proposal) => Ok(ConsensusMessage::Proposal(proposal.try_into()?)), - Message::Vote(vote) => Ok(ConsensusMessage::Vote(vote.try_into()?)), - } - } -} - -impl From for protobuf::ConsensusMessage { - fn from(value: ConsensusMessage) -> Self { - match value { - ConsensusMessage::Proposal(proposal) => protobuf::ConsensusMessage { - message: Some(protobuf::consensus_message::Message::Proposal(proposal.into())), - }, - ConsensusMessage::Vote(vote) => protobuf::ConsensusMessage { - message: Some(protobuf::consensus_message::Message::Vote(vote.into())), - }, - } - } -} - -auto_impl_into_and_try_from_vec_u8!(ConsensusMessage, protobuf::ConsensusMessage); diff --git a/crates/papyrus_protobuf/src/converters/consensus_test.rs b/crates/papyrus_protobuf/src/converters/consensus_test.rs index 51f34a3f70..d2c465706d 100644 --- a/crates/papyrus_protobuf/src/converters/consensus_test.rs +++ b/crates/papyrus_protobuf/src/converters/consensus_test.rs @@ -12,8 +12,6 @@ use starknet_api::transaction::{ }; use crate::consensus::{ - ConsensusMessage, // TODO: remove this - Proposal, // TODO: remove this ProposalFin, ProposalInit, ProposalPart, @@ -66,23 +64,6 @@ fn convert_stream_message_to_vec_u8_and_back() { assert_eq!(stream_message, res_data); } -// TODO(guyn): this can be removed once ConsensusMessage is taken out. -#[test] -fn convert_consensus_message_to_vec_u8_and_back() { - let mut rng = get_rng(); - - // Test that we can convert a ConsensusMessage to bytes and back. - let mut message = ConsensusMessage::get_test_instance(&mut rng); - - if let ConsensusMessage::Proposal(proposal) = &mut message { - add_gas_values_to_transaction(&mut proposal.transactions); - } - - let bytes_data: Vec = message.clone().into(); - let res_data = ConsensusMessage::try_from(bytes_data).unwrap(); - assert_eq!(message, res_data); -} - #[test] fn convert_vote_to_vec_u8_and_back() { let mut rng = get_rng(); @@ -94,19 +75,6 @@ fn convert_vote_to_vec_u8_and_back() { assert_eq!(vote, res_data); } -#[test] -fn convert_proposal_to_vec_u8_and_back() { - let mut rng = get_rng(); - - let mut proposal = Proposal::get_test_instance(&mut rng); - - add_gas_values_to_transaction(&mut proposal.transactions); - - let bytes_data: Vec = proposal.clone().into(); - let res_data = Proposal::try_from(bytes_data).unwrap(); - assert_eq!(proposal, res_data); -} - #[test] fn convert_proposal_init_to_vec_u8_and_back() { let mut rng = get_rng(); @@ -162,7 +130,7 @@ fn stream_message_display() { let mut rng = get_rng(); let stream_id = 42; let message_id = 127; - let proposal = Proposal::get_test_instance(&mut rng); + let proposal = ProposalPart::get_test_instance(&mut rng); let proposal_bytes: Vec = proposal.clone().into(); let proposal_length = proposal_bytes.len(); let content = StreamMessageBody::Content(proposal); @@ -177,7 +145,7 @@ fn stream_message_display() { ) ); - let content: StreamMessageBody = StreamMessageBody::Fin; + let content: StreamMessageBody = StreamMessageBody::Fin; let message = StreamMessage { message: content, stream_id, message_id }; let txt = message.to_string(); assert_eq!( diff --git a/crates/papyrus_protobuf/src/converters/test_instances.rs b/crates/papyrus_protobuf/src/converters/test_instances.rs index c870e1f8e8..be55534a75 100644 --- a/crates/papyrus_protobuf/src/converters/test_instances.rs +++ b/crates/papyrus_protobuf/src/converters/test_instances.rs @@ -5,8 +5,6 @@ use starknet_api::core::ContractAddress; use starknet_api::transaction::Transaction; use crate::consensus::{ - ConsensusMessage, // TODO: remove this - Proposal, // TODO: remove this ProposalFin, ProposalInit, ProposalPart, @@ -18,19 +16,6 @@ use crate::consensus::{ }; auto_impl_get_test_instance! { - // TODO(guyn): remove this once we integrate ProposalPart everywhere. - pub enum ConsensusMessage { - Proposal(Proposal) = 0, - Vote(Vote) = 1, - } - pub struct Proposal { - pub height: u64, - pub round: u32, - pub proposer: ContractAddress, - pub transactions: Vec, - pub block_hash: BlockHash, - pub valid_round: Option, - } pub struct Vote { pub vote_type: VoteType, pub height: u64, diff --git a/crates/papyrus_protobuf/src/proto/p2p/proto/consensus.proto b/crates/papyrus_protobuf/src/proto/p2p/proto/consensus.proto index 2761f48735..155a995755 100644 --- a/crates/papyrus_protobuf/src/proto/p2p/proto/consensus.proto +++ b/crates/papyrus_protobuf/src/proto/p2p/proto/consensus.proto @@ -2,16 +2,6 @@ syntax = "proto3"; import "p2p/proto/transaction.proto"; import "p2p/proto/common.proto"; -// To be deprecated -message Proposal { - uint64 height = 1; - uint32 round = 2; - Address proposer = 3; - repeated Transaction transactions = 4; - Hash block_hash = 5; - optional uint32 valid_round = 6; -} - message Vote { enum VoteType { Prevote = 0; @@ -29,14 +19,6 @@ message Vote { Address voter = 6; } -// TODO(guyn): remove this after we have integrated streams for the proposal -message ConsensusMessage { - oneof message { - Proposal proposal = 1; - Vote vote = 2; - } -} - message StreamMessage { oneof message { bytes content = 1;