From f32547958bfb4a3886d6097497ef8170a5c4a742 Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Mon, 22 Jan 2024 16:41:34 +0400 Subject: [PATCH] rename mined_at to proposed_height --- dan_layer/consensus/src/hotstuff/on_propose.rs | 17 ++++++++--------- dan_layer/p2p/src/conversions/consensus.rs | 8 ++++---- .../down.sql | 1 + .../up.sql | 6 ++++++ dan_layer/state_store_sqlite/src/reader.rs | 3 ++- dan_layer/state_store_sqlite/src/schema.rs | 2 +- .../src/sql_models/bookkeeping.rs | 2 +- dan_layer/state_store_sqlite/src/writer.rs | 2 +- .../src/consensus_models/foreign_proposal.rs | 16 ++++++++-------- 9 files changed, 32 insertions(+), 25 deletions(-) create mode 100644 dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/down.sql create mode 100644 dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/up.sql diff --git a/dan_layer/consensus/src/hotstuff/on_propose.rs b/dan_layer/consensus/src/hotstuff/on_propose.rs index 97e3de063..9b8efab8e 100644 --- a/dan_layer/consensus/src/hotstuff/on_propose.rs +++ b/dan_layer/consensus/src/hotstuff/on_propose.rs @@ -190,17 +190,16 @@ where TConsensusSpec: ConsensusSpec let pending_proposals = ForeignProposal::get_all_pending(tx, locked_block.block_id(), parent_block.block_id())?; let commands = ForeignProposal::get_all_new(tx)? .into_iter() - .filter_map(|foreign_proposal| { - if pending_proposals.iter().any(|pending_proposal| { + .filter(|foreign_proposal| { + // If the foreign proposal is already pending, don't propose it again + !pending_proposals.iter().any(|pending_proposal| { pending_proposal.bucket == foreign_proposal.bucket && pending_proposal.block_id == foreign_proposal.block_id - }) { - None - } else { - Some(Ok(Command::ForeignProposal( - foreign_proposal.set_mined_at(parent_block.height().saturating_add(NodeHeight(1))), - ))) - } + }) + }) + .map(|mut foreign_proposal| { + foreign_proposal.set_proposed_height(parent_block.height().saturating_add(NodeHeight(1))); + Ok(Command::ForeignProposal(foreign_proposal)) }) .chain(batch.into_iter().map(|t| match t.current_stage() { // If the transaction is New, propose to Prepare it diff --git a/dan_layer/p2p/src/conversions/consensus.rs b/dan_layer/p2p/src/conversions/consensus.rs index d466e7a03..1419e9091 100644 --- a/dan_layer/p2p/src/conversions/consensus.rs +++ b/dan_layer/p2p/src/conversions/consensus.rs @@ -363,7 +363,7 @@ impl From for proto::consensus::ForeignProposalState { fn from(value: ForeignProposalState) -> Self { match value { ForeignProposalState::New => proto::consensus::ForeignProposalState::New, - ForeignProposalState::Mined => proto::consensus::ForeignProposalState::Mined, + ForeignProposalState::Proposed => proto::consensus::ForeignProposalState::Mined, ForeignProposalState::Deleted => proto::consensus::ForeignProposalState::Deleted, } } @@ -375,7 +375,7 @@ impl TryFrom for ForeignProposalState { fn try_from(value: proto::consensus::ForeignProposalState) -> Result { match value { proto::consensus::ForeignProposalState::New => Ok(ForeignProposalState::New), - proto::consensus::ForeignProposalState::Mined => Ok(ForeignProposalState::Mined), + proto::consensus::ForeignProposalState::Mined => Ok(ForeignProposalState::Proposed), proto::consensus::ForeignProposalState::Deleted => Ok(ForeignProposalState::Deleted), proto::consensus::ForeignProposalState::UnknownState => Err(anyhow!("Foreign proposal state not provided")), } @@ -390,7 +390,7 @@ impl From<&ForeignProposal> for proto::consensus::ForeignProposal { bucket: value.bucket.as_u32(), block_id: value.block_id.as_bytes().to_vec(), state: proto::consensus::ForeignProposalState::from(value.state).into(), - mined_at: value.mined_at.map(|a| a.0).unwrap_or(0), + mined_at: value.proposed_height.map(|a| a.0).unwrap_or(0), } } } @@ -405,7 +405,7 @@ impl TryFrom for ForeignProposal { state: proto::consensus::ForeignProposalState::try_from(value.state) .map_err(|_| anyhow!("Invalid foreign proposal state value {}", value.state))? .try_into()?, - mined_at: if value.mined_at == 0 { + proposed_height: if value.mined_at == 0 { None } else { Some(NodeHeight(value.mined_at)) diff --git a/dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/down.sql b/dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/down.sql new file mode 100644 index 000000000..291a97c5c --- /dev/null +++ b/dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/down.sql @@ -0,0 +1 @@ +-- This file should undo anything in `up.sql` \ No newline at end of file diff --git a/dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/up.sql b/dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/up.sql new file mode 100644 index 000000000..b1a8dec56 --- /dev/null +++ b/dan_layer/state_store_sqlite/migrations/2024-01-22-123711_rename_mined_at_to_proposed_at/up.sql @@ -0,0 +1,6 @@ +ALTER TABLE foreign_proposals + RENAME COLUMN mined_at TO proposed_height; + +UPDATE foreign_proposals + SET state = 'Proposed' + WHERE state = 'Mined'; \ No newline at end of file diff --git a/dan_layer/state_store_sqlite/src/reader.rs b/dan_layer/state_store_sqlite/src/reader.rs index a6967bc6a..c91349ec1 100644 --- a/dan_layer/state_store_sqlite/src/reader.rs +++ b/dan_layer/state_store_sqlite/src/reader.rs @@ -34,6 +34,7 @@ use tari_dan_storage::{ Decision, Evidence, ForeignProposal, + ForeignProposalState, ForeignReceiveCounters, ForeignSendCounters, HighQc, @@ -398,7 +399,7 @@ impl StateStoreReadTransa use crate::schema::foreign_proposals; let foreign_proposals = foreign_proposals::table - .filter(foreign_proposals::state.eq("New")) + .filter(foreign_proposals::state.eq(ForeignProposalState::New.to_string())) .load::(self.connection()) .map_err(|e| SqliteStorageError::DieselError { operation: "foreign_proposal_get_all", diff --git a/dan_layer/state_store_sqlite/src/schema.rs b/dan_layer/state_store_sqlite/src/schema.rs index 573b79e08..fd5643604 100644 --- a/dan_layer/state_store_sqlite/src/schema.rs +++ b/dan_layer/state_store_sqlite/src/schema.rs @@ -27,7 +27,7 @@ diesel::table! { bucket -> Integer, block_id -> Text, state -> Text, - mined_at -> Nullable, + proposed_height -> Nullable, created_at -> Timestamp, } } diff --git a/dan_layer/state_store_sqlite/src/sql_models/bookkeeping.rs b/dan_layer/state_store_sqlite/src/sql_models/bookkeeping.rs index 9e9d33db3..0d3cf17e1 100644 --- a/dan_layer/state_store_sqlite/src/sql_models/bookkeeping.rs +++ b/dan_layer/state_store_sqlite/src/sql_models/bookkeeping.rs @@ -53,7 +53,7 @@ impl TryFrom for consensus_models::ForeignProposal { bucket: Shard::from(value.bucket as u32), block_id: deserialize_hex_try_from(&value.block_id)?, state: parse_from_string(&value.state)?, - mined_at: value.mined_at.map(|mined_at| NodeHeight(mined_at as u64)), + proposed_height: value.mined_at.map(|mined_at| NodeHeight(mined_at as u64)), }) } } diff --git a/dan_layer/state_store_sqlite/src/writer.rs b/dan_layer/state_store_sqlite/src/writer.rs index 2fd43fe70..f17b081b2 100644 --- a/dan_layer/state_store_sqlite/src/writer.rs +++ b/dan_layer/state_store_sqlite/src/writer.rs @@ -577,7 +577,7 @@ impl StateStoreWriteTransaction for SqliteStateStoreWrit foreign_proposals::bucket.eq(foreign_proposal.bucket.as_u32() as i32), foreign_proposals::block_id.eq(serialize_hex(foreign_proposal.block_id)), foreign_proposals::state.eq(foreign_proposal.state.to_string()), - foreign_proposals::mined_at.eq(foreign_proposal.mined_at.map(|h| h.as_u64() as i64)), + foreign_proposals::proposed_height.eq(foreign_proposal.proposed_height.map(|h| h.as_u64() as i64)), ); diesel::insert_into(foreign_proposals::table) diff --git a/dan_layer/storage/src/consensus_models/foreign_proposal.rs b/dan_layer/storage/src/consensus_models/foreign_proposal.rs index 4971bb368..71f0be579 100644 --- a/dan_layer/storage/src/consensus_models/foreign_proposal.rs +++ b/dan_layer/storage/src/consensus_models/foreign_proposal.rs @@ -16,7 +16,7 @@ use crate::{StateStoreReadTransaction, StateStoreWriteTransaction, StorageError} #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd, Ord)] pub enum ForeignProposalState { New, - Mined, + Proposed, Deleted, } @@ -24,7 +24,7 @@ impl Display for ForeignProposalState { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { ForeignProposalState::New => write!(f, "New"), - ForeignProposalState::Mined => write!(f, "Mined"), + ForeignProposalState::Proposed => write!(f, "Proposed"), ForeignProposalState::Deleted => write!(f, "Deleted"), } } @@ -36,7 +36,7 @@ impl FromStr for ForeignProposalState { fn from_str(s: &str) -> Result { match s { "New" => Ok(ForeignProposalState::New), - "Mined" => Ok(ForeignProposalState::Mined), + "Proposed" => Ok(ForeignProposalState::Proposed), "Deleted" => Ok(ForeignProposalState::Deleted), _ => Err(anyhow::anyhow!("Invalid foreign proposal state {}", s)), } @@ -48,7 +48,7 @@ pub struct ForeignProposal { pub bucket: Shard, pub block_id: BlockId, pub state: ForeignProposalState, - pub mined_at: Option, + pub proposed_height: Option, } impl ForeignProposal { @@ -57,13 +57,13 @@ impl ForeignProposal { bucket, block_id, state: ForeignProposalState::New, - mined_at: None, + proposed_height: None, } } - pub fn set_mined_at(mut self, mined_at: NodeHeight) -> Self { - self.mined_at = Some(mined_at); - self.state = ForeignProposalState::Mined; + pub fn set_proposed_height(&mut self, mined_at: NodeHeight) -> &mut Self { + self.proposed_height = Some(mined_at); + self.state = ForeignProposalState::Proposed; self } }