From 8aa965fd8a723584f892573285b4e8d2625d72aa Mon Sep 17 00:00:00 2001 From: Brian Cho Date: Thu, 30 Jan 2025 11:01:39 -0800 Subject: [PATCH 1/3] [consensus observer] add new BlockTransactionPayload variants to prepare for block gas limit override on blocks --- .../network/observer_message.rs | 121 +++++++++++++----- consensus/src/payload_manager.rs | 4 +- 2 files changed, 93 insertions(+), 32 deletions(-) diff --git a/consensus/src/consensus_observer/network/observer_message.rs b/consensus/src/consensus_observer/network/observer_message.rs index d72397fc4fb17..3fe7711cb09c4 100644 --- a/consensus/src/consensus_observer/network/observer_message.rs +++ b/consensus/src/consensus_observer/network/observer_message.rs @@ -159,7 +159,7 @@ impl Display for ConsensusObserverDirectSend { "BlockPayload: {}. Number of transactions: {}, limit: {:?}, proofs: {:?}", block_payload.block, block_payload.transaction_payload.transactions().len(), - block_payload.transaction_payload.limit(), + block_payload.transaction_payload.transaction_limit(), block_payload.transaction_payload.payload_proofs(), ) }, @@ -336,6 +336,38 @@ impl PayloadWithProof { } } +/// The transaction payload and proof of each block with a transaction and block gas limit +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct PayloadWithProofAndLimits { + payload_with_proof: PayloadWithProof, + transaction_limit: Option, + gas_limit: Option, +} + +impl PayloadWithProofAndLimits { + pub fn new( + payload_with_proof: PayloadWithProof, + transaction_limit: Option, + gas_limit: Option, + ) -> Self { + Self { + payload_with_proof, + transaction_limit, + gas_limit, + } + } + + #[cfg(test)] + /// Returns an empty payload with proof and limit (for testing) + pub fn empty() -> Self { + Self { + payload_with_proof: PayloadWithProof::empty(), + transaction_limit: None, + gas_limit: None, + } + } +} + /// The transaction payload and proof of each block with a transaction limit #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct PayloadWithProofAndLimit { @@ -364,13 +396,19 @@ impl PayloadWithProofAndLimit { /// The transaction payload of each block #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub enum BlockTransactionPayload { - InQuorumStore(PayloadWithProof), - InQuorumStoreWithLimit(PayloadWithProofAndLimit), + // TODO: deprecate InQuorumStore* variants + DeprecatedInQuorumStore(PayloadWithProof), + DeprecatedInQuorumStoreWithLimit(PayloadWithProofAndLimit), QuorumStoreInlineHybrid(PayloadWithProofAndLimit, Vec), OptQuorumStore( PayloadWithProofAndLimit, /* OptQS and Inline Batches */ Vec, ), + QuorumStoreInlineHybridV2(PayloadWithProofAndLimits, Vec), + OptQuorumStoreV2( + PayloadWithProofAndLimits, + /* OptQS and Inline Batches */ Vec, + ), } impl BlockTransactionPayload { @@ -380,7 +418,7 @@ impl BlockTransactionPayload { proofs: Vec, ) -> Self { let payload_with_proof = PayloadWithProof::new(transactions, proofs); - Self::InQuorumStore(payload_with_proof) + Self::DeprecatedInQuorumStore(payload_with_proof) } /// Creates a returns a new InQuorumStoreWithLimit transaction payload @@ -391,7 +429,7 @@ impl BlockTransactionPayload { ) -> Self { let payload_with_proof = PayloadWithProof::new(transactions, proofs); let proof_with_limit = PayloadWithProofAndLimit::new(payload_with_proof, limit); - Self::InQuorumStoreWithLimit(proof_with_limit) + Self::DeprecatedInQuorumStoreWithLimit(proof_with_limit) } /// Creates a returns a new QuorumStoreInlineHybrid transaction payload @@ -426,37 +464,54 @@ impl BlockTransactionPayload { /// Returns the list of inline batches and optimistic batches in the transaction payload pub fn optqs_and_inline_batches(&self) -> &[BatchInfo] { match self { - BlockTransactionPayload::QuorumStoreInlineHybrid(_, inline_batches) => inline_batches, - BlockTransactionPayload::OptQuorumStore(_, optqs_and_inline_batches) => { - optqs_and_inline_batches - }, - _ => &[], + BlockTransactionPayload::DeprecatedInQuorumStore(_) + | BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(_) => &[], + BlockTransactionPayload::QuorumStoreInlineHybrid(_, inline_batches) + | BlockTransactionPayload::OptQuorumStore(_, inline_batches) => inline_batches, + BlockTransactionPayload::QuorumStoreInlineHybridV2(_, inline_batches) + | BlockTransactionPayload::OptQuorumStoreV2(_, inline_batches) => inline_batches, } } - /// Returns the limit of the transaction payload - pub fn limit(&self) -> Option { + /// Returns the transaction limit of the payload + pub fn transaction_limit(&self) -> Option { match self { - BlockTransactionPayload::InQuorumStore(_) => None, - BlockTransactionPayload::InQuorumStoreWithLimit(payload) => payload.transaction_limit, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { + BlockTransactionPayload::DeprecatedInQuorumStore(_) => None, + BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.transaction_limit }, - BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) + | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => payload.transaction_limit, + } + } + + /// Returns the block gas limit of the payload + pub fn gas_limit(&self) -> Option { + match self { + BlockTransactionPayload::DeprecatedInQuorumStore(_) + | BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(_) + | BlockTransactionPayload::QuorumStoreInlineHybrid(_, _) + | BlockTransactionPayload::OptQuorumStore(_, _) => None, + BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) + | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => payload.gas_limit, } } /// Returns the proofs of the transaction payload pub fn payload_proofs(&self) -> Vec { match self { - BlockTransactionPayload::InQuorumStore(payload) => payload.proofs.clone(), - BlockTransactionPayload::InQuorumStoreWithLimit(payload) => { + BlockTransactionPayload::DeprecatedInQuorumStore(payload) => payload.proofs.clone(), + BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.payload_with_proof.proofs.clone() }, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) + | BlockTransactionPayload::OptQuorumStore(payload, _) => { payload.payload_with_proof.proofs.clone() }, - BlockTransactionPayload::OptQuorumStore(payload, _) => { + BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) + | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => { payload.payload_with_proof.proofs.clone() }, } @@ -465,14 +520,18 @@ impl BlockTransactionPayload { /// Returns the transactions in the payload pub fn transactions(&self) -> Vec { match self { - BlockTransactionPayload::InQuorumStore(payload) => payload.transactions.clone(), - BlockTransactionPayload::InQuorumStoreWithLimit(payload) => { + BlockTransactionPayload::DeprecatedInQuorumStore(payload) => { + payload.transactions.clone() + }, + BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.payload_with_proof.transactions.clone() }, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) + | BlockTransactionPayload::OptQuorumStore(payload, _) => { payload.payload_with_proof.transactions.clone() }, - BlockTransactionPayload::OptQuorumStore(payload, _) => { + BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) + | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => { payload.payload_with_proof.transactions.clone() }, } @@ -624,16 +683,18 @@ impl BlockTransactionPayload { ) -> Result<(), Error> { // Get the payload limit let limit = match self { - BlockTransactionPayload::InQuorumStoreWithLimit(payload) => payload.transaction_limit, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { - payload.transaction_limit - }, - BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, - _ => { + BlockTransactionPayload::DeprecatedInQuorumStore(_) => { return Err(Error::InvalidMessageError( "Transaction payload does not contain a limit!".to_string(), )) }, + BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { + payload.transaction_limit + }, + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) + | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => payload.transaction_limit, }; // Compare the expected limit against the payload limit diff --git a/consensus/src/payload_manager.rs b/consensus/src/payload_manager.rs index 86a537edb940b..7b2df91e2dadf 100644 --- a/consensus/src/payload_manager.rs +++ b/consensus/src/payload_manager.rs @@ -504,7 +504,7 @@ impl TPayloadManager for QuorumStorePayloadManager { Ok(( transaction_payload.transactions(), - transaction_payload.limit(), + transaction_payload.transaction_limit(), )) } } @@ -554,7 +554,7 @@ async fn get_transactions_for_observer( // Return the transactions and the transaction limit Ok(( transaction_payload.transactions(), - transaction_payload.limit(), + transaction_payload.transaction_limit(), )) } From 350aecfa108a222fc61e13c4e13b3134bd1ea7f6 Mon Sep 17 00:00:00 2001 From: Brian Cho Date: Thu, 30 Jan 2025 14:48:39 -0800 Subject: [PATCH 2/3] remove unnecessary OptQuorumStoreV2 --- .../network/observer_message.rs | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/consensus/src/consensus_observer/network/observer_message.rs b/consensus/src/consensus_observer/network/observer_message.rs index 3fe7711cb09c4..6e5f6aec2c1f5 100644 --- a/consensus/src/consensus_observer/network/observer_message.rs +++ b/consensus/src/consensus_observer/network/observer_message.rs @@ -401,14 +401,10 @@ pub enum BlockTransactionPayload { DeprecatedInQuorumStoreWithLimit(PayloadWithProofAndLimit), QuorumStoreInlineHybrid(PayloadWithProofAndLimit, Vec), OptQuorumStore( - PayloadWithProofAndLimit, - /* OptQS and Inline Batches */ Vec, - ), - QuorumStoreInlineHybridV2(PayloadWithProofAndLimits, Vec), - OptQuorumStoreV2( PayloadWithProofAndLimits, /* OptQS and Inline Batches */ Vec, ), + QuorumStoreInlineHybridV2(PayloadWithProofAndLimits, Vec), } impl BlockTransactionPayload { @@ -451,8 +447,8 @@ impl BlockTransactionPayload { batch_infos: Vec, ) -> Self { let payload_with_proof = PayloadWithProof::new(transactions, proofs); - let proof_with_limit = PayloadWithProofAndLimit::new(payload_with_proof, limit); - Self::OptQuorumStore(proof_with_limit, batch_infos) + let proof_with_limits = PayloadWithProofAndLimits::new(payload_with_proof, limit, None); + Self::OptQuorumStore(proof_with_limits, batch_infos) } #[cfg(test)] @@ -467,9 +463,8 @@ impl BlockTransactionPayload { BlockTransactionPayload::DeprecatedInQuorumStore(_) | BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(_) => &[], BlockTransactionPayload::QuorumStoreInlineHybrid(_, inline_batches) + | BlockTransactionPayload::QuorumStoreInlineHybridV2(_, inline_batches) | BlockTransactionPayload::OptQuorumStore(_, inline_batches) => inline_batches, - BlockTransactionPayload::QuorumStoreInlineHybridV2(_, inline_batches) - | BlockTransactionPayload::OptQuorumStoreV2(_, inline_batches) => inline_batches, } } @@ -480,10 +475,11 @@ impl BlockTransactionPayload { BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.transaction_limit }, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { + payload.transaction_limit + }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => payload.transaction_limit, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, } } @@ -492,10 +488,9 @@ impl BlockTransactionPayload { match self { BlockTransactionPayload::DeprecatedInQuorumStore(_) | BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(_) - | BlockTransactionPayload::QuorumStoreInlineHybrid(_, _) - | BlockTransactionPayload::OptQuorumStore(_, _) => None, + | BlockTransactionPayload::QuorumStoreInlineHybrid(_, _) => None, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => payload.gas_limit, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.gas_limit, } } @@ -506,12 +501,11 @@ impl BlockTransactionPayload { BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.payload_with_proof.proofs.clone() }, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => { + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { payload.payload_with_proof.proofs.clone() }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => { + | BlockTransactionPayload::OptQuorumStore(payload, _) => { payload.payload_with_proof.proofs.clone() }, } @@ -526,12 +520,11 @@ impl BlockTransactionPayload { BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.payload_with_proof.transactions.clone() }, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => { + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { payload.payload_with_proof.transactions.clone() }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => { + | BlockTransactionPayload::OptQuorumStore(payload, _) => { payload.payload_with_proof.transactions.clone() }, } @@ -691,10 +684,11 @@ impl BlockTransactionPayload { BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(payload) => { payload.transaction_limit }, - BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + BlockTransactionPayload::QuorumStoreInlineHybrid(payload, _) => { + payload.transaction_limit + }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStoreV2(payload, _) => payload.transaction_limit, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, }; // Compare the expected limit against the payload limit From 7627a20eab3838fbf12edaf5f4362ff622564057 Mon Sep 17 00:00:00 2001 From: Brian Cho Date: Thu, 30 Jan 2025 15:11:34 -0800 Subject: [PATCH 3/3] Move the new BlockTransactionPayload to use enum TransactionsWithProof --- .../network/observer_message.rs | 93 +++++++++++++------ 1 file changed, 64 insertions(+), 29 deletions(-) diff --git a/consensus/src/consensus_observer/network/observer_message.rs b/consensus/src/consensus_observer/network/observer_message.rs index 6e5f6aec2c1f5..eea363a37e4ea 100644 --- a/consensus/src/consensus_observer/network/observer_message.rs +++ b/consensus/src/consensus_observer/network/observer_message.rs @@ -336,24 +336,18 @@ impl PayloadWithProof { } } -/// The transaction payload and proof of each block with a transaction and block gas limit +/// The transaction payload and proof of each block with a transaction limit #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] -pub struct PayloadWithProofAndLimits { +pub struct PayloadWithProofAndLimit { payload_with_proof: PayloadWithProof, transaction_limit: Option, - gas_limit: Option, } -impl PayloadWithProofAndLimits { - pub fn new( - payload_with_proof: PayloadWithProof, - transaction_limit: Option, - gas_limit: Option, - ) -> Self { +impl PayloadWithProofAndLimit { + pub fn new(payload_with_proof: PayloadWithProof, limit: Option) -> Self { Self { payload_with_proof, - transaction_limit, - gas_limit, + transaction_limit: limit, } } @@ -363,23 +357,65 @@ impl PayloadWithProofAndLimits { Self { payload_with_proof: PayloadWithProof::empty(), transaction_limit: None, - gas_limit: None, } } } -/// The transaction payload and proof of each block with a transaction limit #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] -pub struct PayloadWithProofAndLimit { +pub enum TransactionsWithProof { + TransactionsWithProofAndLimits(TransactionsWithProofAndLimits), +} + +impl TransactionsWithProof { + pub fn transactions(&self) -> Vec { + match self { + TransactionsWithProof::TransactionsWithProofAndLimits(payload) => { + payload.payload_with_proof.transactions.clone() + }, + } + } + + pub fn proofs(&self) -> Vec { + match self { + TransactionsWithProof::TransactionsWithProofAndLimits(payload) => { + payload.payload_with_proof.proofs.clone() + }, + } + } + + pub fn transaction_limit(&self) -> Option { + match self { + TransactionsWithProof::TransactionsWithProofAndLimits(payload) => { + payload.transaction_limit + }, + } + } + + pub fn gas_limit(&self) -> Option { + match self { + TransactionsWithProof::TransactionsWithProofAndLimits(payload) => payload.gas_limit, + } + } +} + +/// The transaction payload and proof of each block with a transaction and block gas limit +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct TransactionsWithProofAndLimits { payload_with_proof: PayloadWithProof, transaction_limit: Option, + gas_limit: Option, } -impl PayloadWithProofAndLimit { - pub fn new(payload_with_proof: PayloadWithProof, limit: Option) -> Self { +impl TransactionsWithProofAndLimits { + pub fn new( + payload_with_proof: PayloadWithProof, + transaction_limit: Option, + gas_limit: Option, + ) -> Self { Self { payload_with_proof, - transaction_limit: limit, + transaction_limit, + gas_limit, } } @@ -389,6 +425,7 @@ impl PayloadWithProofAndLimit { Self { payload_with_proof: PayloadWithProof::empty(), transaction_limit: None, + gas_limit: None, } } } @@ -401,10 +438,10 @@ pub enum BlockTransactionPayload { DeprecatedInQuorumStoreWithLimit(PayloadWithProofAndLimit), QuorumStoreInlineHybrid(PayloadWithProofAndLimit, Vec), OptQuorumStore( - PayloadWithProofAndLimits, + TransactionsWithProof, /* OptQS and Inline Batches */ Vec, ), - QuorumStoreInlineHybridV2(PayloadWithProofAndLimits, Vec), + QuorumStoreInlineHybridV2(TransactionsWithProof, Vec), } impl BlockTransactionPayload { @@ -447,7 +484,9 @@ impl BlockTransactionPayload { batch_infos: Vec, ) -> Self { let payload_with_proof = PayloadWithProof::new(transactions, proofs); - let proof_with_limits = PayloadWithProofAndLimits::new(payload_with_proof, limit, None); + let proof_with_limits = TransactionsWithProof::TransactionsWithProofAndLimits( + TransactionsWithProofAndLimits::new(payload_with_proof, limit, None), + ); Self::OptQuorumStore(proof_with_limits, batch_infos) } @@ -479,7 +518,7 @@ impl BlockTransactionPayload { payload.transaction_limit }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit(), } } @@ -490,7 +529,7 @@ impl BlockTransactionPayload { | BlockTransactionPayload::DeprecatedInQuorumStoreWithLimit(_) | BlockTransactionPayload::QuorumStoreInlineHybrid(_, _) => None, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.gas_limit, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.gas_limit(), } } @@ -505,9 +544,7 @@ impl BlockTransactionPayload { payload.payload_with_proof.proofs.clone() }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => { - payload.payload_with_proof.proofs.clone() - }, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.proofs(), } } @@ -524,9 +561,7 @@ impl BlockTransactionPayload { payload.payload_with_proof.transactions.clone() }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => { - payload.payload_with_proof.transactions.clone() - }, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transactions(), } } @@ -688,7 +723,7 @@ impl BlockTransactionPayload { payload.transaction_limit }, BlockTransactionPayload::QuorumStoreInlineHybridV2(payload, _) - | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit, + | BlockTransactionPayload::OptQuorumStore(payload, _) => payload.transaction_limit(), }; // Compare the expected limit against the payload limit