Skip to content

Commit

Permalink
chore: unify the two types of ProposalInit structs (#1792)
Browse files Browse the repository at this point in the history
  • Loading branch information
guy-starkware authored Nov 10, 2024
1 parent 913e137 commit 1097d48
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 84 deletions.
32 changes: 16 additions & 16 deletions crates/papyrus_protobuf/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ pub struct StreamMessage<T: Into<Vec<u8>> + TryFrom<Vec<u8>, Error = ProtobufCon
}

/// This message must be sent first when proposing a new block.
#[derive(Debug, Clone, PartialEq)]
#[derive(Default, Debug, Clone, PartialEq)]
pub struct ProposalInit {
/// The height of the consensus (block number).
pub height: u64,
pub height: BlockNumber,
/// The current round of the consensus.
pub round: u32,
/// The last round that was valid.
Expand Down Expand Up @@ -129,16 +129,16 @@ where
pub struct ProposalWrapper(pub Proposal);

impl From<ProposalWrapper>
for (
(BlockNumber, u32, ContractAddress, Option<u32>),
mpsc::Receiver<Transaction>,
oneshot::Receiver<BlockHash>,
)
for (ProposalInit, mpsc::Receiver<Transaction>, oneshot::Receiver<BlockHash>)
{
fn from(val: ProposalWrapper) -> Self {
let transactions: Vec<Transaction> = val.0.transactions.into_iter().collect();
let proposal_init =
(BlockNumber(val.0.height), val.0.round, val.0.proposer, val.0.valid_round);
let proposal_init = ProposalInit {
height: BlockNumber(val.0.height),
round: val.0.round,
proposer: val.0.proposer,
valid_round: val.0.valid_round,
};
let (mut content_sender, content_receiver) = mpsc::channel(transactions.len());
for tx in transactions {
content_sender.try_send(tx).expect("Send should succeed");
Expand All @@ -153,15 +153,15 @@ impl From<ProposalWrapper>
}

impl From<ProposalWrapper>
for (
(BlockNumber, u32, ContractAddress, Option<u32>),
mpsc::Receiver<Vec<ExecutableTransaction>>,
oneshot::Receiver<BlockHash>,
)
for (ProposalInit, mpsc::Receiver<Vec<ExecutableTransaction>>, oneshot::Receiver<BlockHash>)
{
fn from(val: ProposalWrapper) -> Self {
let proposal_init =
(BlockNumber(val.0.height), val.0.round, val.0.proposer, val.0.valid_round);
let proposal_init = ProposalInit {
height: BlockNumber(val.0.height),
round: val.0.round,
proposer: val.0.proposer,
valid_round: val.0.valid_round,
};

let (_, content_receiver) = mpsc::channel(0);
// This should only be used for Milestone 1, and then removed once streaming is supported.
Expand Down
6 changes: 3 additions & 3 deletions crates/papyrus_protobuf/src/converters/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ mod consensus_test;
use std::convert::{TryFrom, TryInto};

use prost::Message;
use starknet_api::block::BlockHash;
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::hash::StarkHash;
use starknet_api::transaction::Transaction;

Expand Down Expand Up @@ -209,14 +209,14 @@ impl TryFrom<protobuf::ProposalInit> for ProposalInit {
.proposer
.ok_or(ProtobufConversionError::MissingField { field_description: "proposer" })?
.try_into()?;
Ok(ProposalInit { height, round, valid_round, proposer })
Ok(ProposalInit { height: BlockNumber(height), round, valid_round, proposer })
}
}

impl From<ProposalInit> for protobuf::ProposalInit {
fn from(value: ProposalInit) -> Self {
protobuf::ProposalInit {
height: value.height,
height: value.height.0,
round: value.round,
valid_round: value.valid_round,
proposer: Some(value.proposer.into()),
Expand Down
4 changes: 2 additions & 2 deletions crates/papyrus_protobuf/src/converters/test_instances.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use papyrus_test_utils::{auto_impl_get_test_instance, get_number_of_variants, GetTestInstance};
use rand::Rng;
use starknet_api::block::BlockHash;
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::core::ContractAddress;
use starknet_api::transaction::Transaction;

Expand Down Expand Up @@ -42,7 +42,7 @@ auto_impl_get_test_instance! {
Precommit = 1,
}
pub struct ProposalInit {
pub height: u64,
pub height: BlockNumber,
pub round: u32,
pub valid_round: Option<u32>,
pub proposer: ContractAddress,
Expand Down
16 changes: 6 additions & 10 deletions crates/sequencing/papyrus_consensus/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ use futures::stream::FuturesUnordered;
use futures::{Stream, StreamExt};
use papyrus_common::metrics::{PAPYRUS_CONSENSUS_HEIGHT, PAPYRUS_CONSENSUS_SYNC_COUNT};
use papyrus_network::network_manager::BroadcastTopicClientTrait;
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalWrapper};
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalInit, ProposalWrapper};
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::core::ContractAddress;
use tracing::{debug, info, instrument};

use crate::config::TimeoutsConfig;
Expand Down Expand Up @@ -43,11 +42,8 @@ pub async fn run_consensus<ContextT, SyncReceiverT>(
where
ContextT: ConsensusContext,
SyncReceiverT: Stream<Item = BlockNumber> + Unpin,
ProposalWrapper: Into<(
(BlockNumber, u32, ContractAddress, Option<u32>),
mpsc::Receiver<ContextT::ProposalChunk>,
oneshot::Receiver<BlockHash>,
)>,
ProposalWrapper:
Into<(ProposalInit, mpsc::Receiver<ContextT::ProposalChunk>, oneshot::Receiver<BlockHash>)>,
{
info!(
"Running consensus, start_height={}, validator_id={}, consensus_delay={}, timeouts={:?}",
Expand Down Expand Up @@ -114,7 +110,7 @@ impl MultiHeightManager {
where
ContextT: ConsensusContext,
ProposalWrapper: Into<(
(BlockNumber, u32, ContractAddress, Option<u32>),
ProposalInit,
mpsc::Receiver<ContextT::ProposalChunk>,
oneshot::Receiver<BlockHash>,
)>,
Expand Down Expand Up @@ -171,7 +167,7 @@ impl MultiHeightManager {
where
ContextT: ConsensusContext,
ProposalWrapper: Into<(
(BlockNumber, u32, ContractAddress, Option<u32>),
ProposalInit,
mpsc::Receiver<ContextT::ProposalChunk>,
oneshot::Receiver<BlockHash>,
)>,
Expand All @@ -193,7 +189,7 @@ impl MultiHeightManager {
let (proposal_init, content_receiver, fin_receiver) =
ProposalWrapper(proposal).into();
let res = shc
.handle_proposal(context, proposal_init.into(), content_receiver, fin_receiver)
.handle_proposal(context, proposal_init, content_receiver, fin_receiver)
.await?;
Ok(res)
}
Expand Down
11 changes: 2 additions & 9 deletions crates/sequencing/papyrus_consensus/src/manager_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use papyrus_network::network_manager::test_utils::{
TestSubscriberChannels,
};
use papyrus_network_types::network_types::BroadcastedMessageMetadata;
use papyrus_protobuf::consensus::{ConsensusMessage, Vote};
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalInit, Vote};
use papyrus_test_utils::{get_rng, GetTestInstance};
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::transaction::Transaction;
Expand All @@ -22,14 +22,7 @@ use starknet_types_core::felt::Felt;
use super::{run_consensus, MultiHeightManager};
use crate::config::TimeoutsConfig;
use crate::test_utils::{precommit, prevote, proposal};
use crate::types::{
ConsensusContext,
ConsensusError,
ProposalContentId,
ProposalInit,
Round,
ValidatorId,
};
use crate::types::{ConsensusContext, ConsensusError, ProposalContentId, Round, ValidatorId};

lazy_static! {
static ref PROPOSER_ID: ValidatorId = 0_u32.into();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::time::Duration;
#[cfg(test)]
use enum_as_inner::EnumAsInner;
use futures::channel::{mpsc, oneshot};
use papyrus_protobuf::consensus::{ConsensusMessage, Vote, VoteType};
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalInit, Vote, VoteType};
use starknet_api::block::BlockNumber;
use tracing::{debug, info, instrument, trace, warn};

Expand All @@ -20,7 +20,6 @@ use crate::types::{
ConsensusError,
Decision,
ProposalContentId,
ProposalInit,
Round,
ValidatorId,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use futures::channel::{mpsc, oneshot};
use lazy_static::lazy_static;
use papyrus_protobuf::consensus::ConsensusMessage;
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalInit};
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_types_core::felt::Felt;
use test_case::test_case;
Expand All @@ -11,7 +11,7 @@ use crate::config::TimeoutsConfig;
use crate::single_height_consensus::{ShcEvent, ShcReturn, ShcTask};
use crate::state_machine::StateMachineEvent;
use crate::test_utils::{precommit, prevote, MockTestContext, TestBlock};
use crate::types::{ConsensusError, ProposalInit, ValidatorId};
use crate::types::{ConsensusError, ValidatorId};

lazy_static! {
static ref PROPOSER_ID: ValidatorId = 0_u32.into();
Expand Down
11 changes: 2 additions & 9 deletions crates/sequencing/papyrus_consensus/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,11 @@ use std::time::Duration;
use async_trait::async_trait;
use futures::channel::{mpsc, oneshot};
use mockall::mock;
use papyrus_protobuf::consensus::{ConsensusMessage, Proposal, Vote, VoteType};
use papyrus_protobuf::consensus::{ConsensusMessage, Proposal, ProposalInit, Vote, VoteType};
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_types_core::felt::Felt;

use crate::types::{
ConsensusContext,
ConsensusError,
ProposalContentId,
ProposalInit,
Round,
ValidatorId,
};
use crate::types::{ConsensusContext, ConsensusError, ProposalContentId, Round, ValidatorId};

/// Define a consensus block which can be used to enable auto mocking Context.
#[derive(Debug, PartialEq, Clone)]
Expand Down
17 changes: 1 addition & 16 deletions crates/sequencing/papyrus_consensus/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use papyrus_network::network_manager::{
GenericReceiver,
};
use papyrus_network_types::network_types::BroadcastedMessageMetadata;
use papyrus_protobuf::consensus::{ConsensusMessage, Vote};
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalInit, Vote};
use papyrus_protobuf::converters::ProtobufConversionError;
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::core::ContractAddress;
Expand Down Expand Up @@ -118,21 +118,6 @@ impl Debug for Decision {
}
}

#[derive(PartialEq, Debug, Default, Clone)]
pub struct ProposalInit {
pub height: BlockNumber,
pub round: Round,
pub proposer: ValidatorId,
pub valid_round: Option<Round>,
}

// TODO(Guy): Remove after implementing broadcast streams.
impl From<(BlockNumber, u32, ContractAddress, Option<u32>)> for ProposalInit {
fn from(val: (BlockNumber, u32, ContractAddress, Option<u32>)) -> Self {
ProposalInit { height: val.0, round: val.1, proposer: val.2, valid_round: val.3 }
}
}

pub struct BroadcastConsensusMessageChannel {
pub broadcasted_messages_receiver: GenericReceiver<(
Result<ConsensusMessage, ProtobufConversionError>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ use papyrus_consensus::types::{
ConsensusContext,
ConsensusError,
ProposalContentId,
ProposalInit,
Round,
ValidatorId,
};
use papyrus_network::network_manager::{BroadcastTopicClient, BroadcastTopicClientTrait};
use papyrus_protobuf::consensus::{ConsensusMessage, Proposal, Vote};
use papyrus_protobuf::consensus::{ConsensusMessage, Proposal, ProposalInit, Vote};
use papyrus_storage::body::BodyStorageReader;
use papyrus_storage::header::HeaderStorageReader;
use papyrus_storage::{StorageError, StorageReader};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ use std::time::Duration;

use futures::channel::{mpsc, oneshot};
use futures::StreamExt;
use papyrus_consensus::types::{ConsensusContext, ProposalInit};
use papyrus_consensus::types::ConsensusContext;
use papyrus_network::network_manager::test_utils::{
mock_register_broadcast_topic,
BroadcastNetworkMock,
};
use papyrus_protobuf::consensus::{ConsensusMessage, Vote};
use papyrus_protobuf::consensus::{ConsensusMessage, ProposalInit, Vote};
use papyrus_storage::body::BodyStorageWriter;
use papyrus_storage::header::HeaderStorageWriter;
use papyrus_storage::test_utils::get_test_storage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ use papyrus_consensus::types::{
ConsensusContext,
ConsensusError,
ProposalContentId,
ProposalInit,
Round,
ValidatorId,
};
use papyrus_network::network_manager::{BroadcastTopicClient, BroadcastTopicClientTrait};
use papyrus_protobuf::consensus::{
ConsensusMessage,
ProposalInit as ProtobufProposalInit,
ProposalInit,
ProposalPart,
TransactionBatch,
Vote,
Expand Down Expand Up @@ -125,15 +124,9 @@ impl ConsensusContext for SequencerConsensusContext {
.build_proposal(build_proposal_input)
.await
.expect("Failed to initiate proposal build");
let protobuf_consensus_init = ProtobufProposalInit {
height: proposal_init.height.0,
round: proposal_init.round,
proposer: proposal_init.proposer,
valid_round: proposal_init.valid_round,
};
debug!("Broadcasting proposal init: {protobuf_consensus_init:?}");
debug!("Broadcasting proposal init: {proposal_init:?}");
self.network_broadcast_client
.broadcast_message(ProposalPart::Init(protobuf_consensus_init))
.broadcast_message(ProposalPart::Init(proposal_init.clone()))
.await
.expect("Failed to broadcast proposal init");
let broadcast_client = self.network_broadcast_client.clone();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ use std::vec;
use futures::channel::mpsc;
use futures::SinkExt;
use lazy_static::lazy_static;
use papyrus_consensus::types::{ConsensusContext, ProposalInit};
use papyrus_consensus::types::ConsensusContext;
use papyrus_network::network_manager::test_utils::{
mock_register_broadcast_topic,
TestSubscriberChannels,
};
use papyrus_network::network_manager::BroadcastTopicChannels;
use papyrus_protobuf::consensus::ProposalInit;
use starknet_api::block::{BlockHash, BlockNumber};
use starknet_api::core::{ContractAddress, StateDiffCommitment};
use starknet_api::executable_transaction::{AccountTransaction, Transaction};
Expand Down

0 comments on commit 1097d48

Please sign in to comment.