Skip to content

[consensus] Use OptQuorumStore payload exclusively in proof_manager #18653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ibalajiarun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[consensus] Use OptQuorumStore payload exclusively in proof_manager #18653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions consensus/consensus-types/src/block_data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ impl BlockData {
pub fn dummy_with_validator_txns(txns: Vec<ValidatorTransaction>) -> Self {
Self::new_proposal_ext(
txns,
Payload::empty(false, true),
Payload::empty(false),
Author::ONE,
vec![],
1,
Expand Down Expand Up @@ -450,7 +450,7 @@ fn test_reconfiguration_suffix() {
),
);
let reconfig_suffix_block = BlockData::new_proposal(
Payload::empty(false, true),
Payload::empty(false),
AccountAddress::random(),
Vec::new(),
2,
Expand Down
10 changes: 5 additions & 5 deletions consensus/consensus-types/src/block_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn test_nil_block() {
nil_block_qc.certified_block().id()
);
let nil_block_child = Block::new_proposal(
Payload::empty(false, true),
Payload::empty(false),
2,
aptos_infallible::duration_since_epoch().as_micros() as u64,
nil_block_qc,
Expand All @@ -90,7 +90,7 @@ fn test_block_relation() {
// Test genesis and the next block
let genesis_block = Block::make_genesis_block();
let quorum_cert = certificate_for_genesis();
let payload = Payload::empty(false, true);
let payload = Payload::empty(false);
let next_block = Block::new_proposal(
payload.clone(),
1,
Expand All @@ -117,7 +117,7 @@ fn test_same_qc_different_authors() {
let signer = signers.first().unwrap();
let genesis_qc = certificate_for_genesis();
let round = 1;
let payload = Payload::empty(false, true);
let payload = Payload::empty(false);
let current_timestamp = aptos_infallible::duration_since_epoch().as_micros() as u64;
let block_round_1 = Block::new_proposal(
payload.clone(),
Expand Down Expand Up @@ -179,7 +179,7 @@ fn test_block_metadata_bitvec() {
&ledger_info,
Block::make_genesis_block_from_ledger_info(&ledger_info).id(),
);
let payload = Payload::empty(false, true);
let payload = Payload::empty(false);
let start_round = 1;
let start_timestamp = aptos_infallible::duration_since_epoch().as_micros() as u64;

Expand Down Expand Up @@ -253,7 +253,7 @@ fn test_failed_authors_well_formed() {
let other = Author::random();
// Test genesis and the next block
let quorum_cert = certificate_for_genesis();
let payload = Payload::empty(false, true);
let payload = Payload::empty(false);

let create_block = |round: Round, failed_authors: Vec<(Round, Author)>| {
Block::new_proposal(
Expand Down
2 changes: 1 addition & 1 deletion consensus/consensus-types/src/block_test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ prop_compose! {
parent_qc in Just(parent_qc)
) -> Block {
Block::new_proposal(
Payload::empty(false, true),
Payload::empty(false),
round,
aptos_infallible::duration_since_epoch().as_micros() as u64,
parent_qc,
Expand Down
13 changes: 7 additions & 6 deletions consensus/consensus-types/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,14 @@ impl Payload {
}
}

pub fn empty(quorum_store_enabled: bool, allow_batches_without_pos_in_proposal: bool) -> Self {
pub fn empty(quorum_store_enabled: bool) -> Self {
if quorum_store_enabled {
if allow_batches_without_pos_in_proposal {
Payload::QuorumStoreInlineHybrid(Vec::new(), ProofWithData::new(Vec::new()), None)
} else {
Payload::InQuorumStore(ProofWithData::new(Vec::new()))
}
Payload::OptQuorumStore(OptQuorumStorePayload::new(
Vec::<(BatchInfo, Vec<SignedTransaction>)>::new().into(),
Vec::<BatchInfo>::new().into(),
Vec::<ProofOfStore<BatchInfo>>::new().into(),
PayloadExecutionLimit::None,
))
} else {
Payload::DirectMempool(Vec::new())
}
Expand Down
4 changes: 2 additions & 2 deletions consensus/consensus-types/src/opt_proposal_msg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ mod tests {

let opt_block_data = OptBlockData::new(
vec![],
Payload::empty(false, true),
Payload::empty(false),
signer.author(),
epoch,
round,
Expand Down Expand Up @@ -243,7 +243,7 @@ mod tests {
let block_data = msg.take_block_data();
let epoch_2_block_data = OptBlockData::new(
vec![],
Payload::empty(false, true),
Payload::empty(false),
signer.author(),
2, // Different epoch
block_data.round(),
Expand Down
2 changes: 1 addition & 1 deletion consensus/safety-rules/src/test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub fn make_proposal_with_qc(
validator_signer: &ValidatorSigner,
) -> VoteProposal {
make_proposal_with_qc_and_proof(
Payload::empty(false, true),
Payload::empty(false),
round,
empty_proof(),
qc,
Expand Down
22 changes: 5 additions & 17 deletions consensus/safety-rules/src/tests/suite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,7 @@ fn make_proposal_with_qc_and_proof(
qc: QuorumCert,
signer: &ValidatorSigner,
) -> VoteProposal {
test_utils::make_proposal_with_qc_and_proof(
Payload::empty(false, true),
round,
proof,
qc,
signer,
)
test_utils::make_proposal_with_qc_and_proof(Payload::empty(false), round, proof, qc, signer)
}

fn make_proposal_with_parent(
Expand All @@ -48,13 +42,7 @@ fn make_proposal_with_parent(
committed: Option<&VoteProposal>,
signer: &ValidatorSigner,
) -> VoteProposal {
test_utils::make_proposal_with_parent(
Payload::empty(false, true),
round,
parent,
committed,
signer,
)
test_utils::make_proposal_with_parent(Payload::empty(false), round, parent, committed, signer)
}

pub type Callback = Box<dyn Fn() -> (Box<dyn TSafetyRules + Send + Sync>, ValidatorSigner)>;
Expand Down Expand Up @@ -450,7 +438,7 @@ fn test_voting_bad_epoch(safety_rules: &Callback) {

let a1 = test_utils::make_proposal_with_qc(round + 1, genesis_qc, &signer);
let a2 = test_utils::make_proposal_with_parent_and_overrides(
Payload::empty(false, true),
Payload::empty(false),
round + 3,
&a1,
None,
Expand Down Expand Up @@ -618,7 +606,7 @@ fn test_validator_not_in_set(safety_rules: &Callback) {
next_epoch_state.verifier =
ValidatorVerifier::new_single(rand_signer.author(), rand_signer.public_key()).into();
let a2 = test_utils::make_proposal_with_parent_and_overrides(
Payload::empty(false, true),
Payload::empty(false),
round + 2,
&a1,
Some(&a1),
Expand Down Expand Up @@ -656,7 +644,7 @@ fn test_key_not_in_store(safety_rules: &Callback) {
next_epoch_state.verifier =
ValidatorVerifier::new_single(signer.author(), rand_signer.public_key()).into();
let a2 = test_utils::make_proposal_with_parent_and_overrides(
Payload::empty(false, true),
Payload::empty(false),
round + 2,
&a1,
Some(&a1),
Expand Down
4 changes: 2 additions & 2 deletions consensus/src/block_storage/block_store_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ async fn test_illegal_timestamp() {
let block_store = build_default_empty_tree();
let genesis = block_store.ordered_root();
let block_with_illegal_timestamp = Block::new_proposal(
Payload::empty(false, true),
Payload::empty(false),
0,
// This timestamp is illegal, it is the same as genesis
genesis.timestamp_usecs(),
Expand Down Expand Up @@ -457,7 +457,7 @@ async fn test_need_sync_for_ledger_info() {
certificate_for_genesis(),
1,
round,
Payload::empty(false, true),
Payload::empty(false),
vec![],
);
gen_test_certificate(
Expand Down
2 changes: 1 addition & 1 deletion consensus/src/consensusdb/consensusdb_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn test_dag() {
Author::random(),
123,
vec![],
Payload::empty(false, true),
Payload::empty(false),
vec![],
Extensions::empty(),
);
Expand Down
8 changes: 1 addition & 7 deletions consensus/src/dag/adapter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ pub(super) struct OrderedNotifierAdapter {
epoch_state: Arc<EpochState>,
ledger_info_provider: Arc<RwLock<LedgerInfoProvider>>,
block_ordered_ts: Arc<RwLock<BTreeMap<Round, Instant>>>,
allow_batches_without_pos_in_proposal: bool,
}

impl OrderedNotifierAdapter {
Expand All @@ -109,7 +108,6 @@ impl OrderedNotifierAdapter {
epoch_state: Arc<EpochState>,
parent_block_info: BlockInfo,
ledger_info_provider: Arc<RwLock<LedgerInfoProvider>>,
allow_batches_without_pos_in_proposal: bool,
) -> Self {
Self {
executor_channel,
Expand All @@ -118,7 +116,6 @@ impl OrderedNotifierAdapter {
epoch_state,
ledger_info_provider,
block_ordered_ts: Arc::new(RwLock::new(BTreeMap::new())),
allow_batches_without_pos_in_proposal,
}
}

Expand Down Expand Up @@ -148,10 +145,7 @@ impl OrderedNotifier for OrderedNotifierAdapter {
let timestamp = anchor.metadata().timestamp();
let author = *anchor.author();
let mut validator_txns = vec![];
let mut payload = Payload::empty(
!anchor.payload().is_direct(),
Copy link

@cursor cursor bot Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Payload extend panics with mixed OptQuorumStore and legacy types

High Severity

The Payload::empty() function now returns OptQuorumStore when quorum store is enabled, but Payload::extend() explicitly panics with unimplemented!() when combining OptQuorumStore with QuorumStoreInlineHybrid or QuorumStoreInlineHybridV2. In OrderedNotifierAdapter::send_ordered_nodes(), an empty OptQuorumStore payload is created and then extended with node payloads. During upgrades or mixed-version scenarios, nodes with legacy payload types would cause a runtime panic.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor Author

@ibalajiarun ibalajiarun Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend was implemented for DAG and is not used.

self.allow_batches_without_pos_in_proposal,
);
let mut payload = Payload::empty(!anchor.payload().is_direct());
let mut node_digests = vec![];
for node in &ordered_nodes {
validator_txns.extend(node.validator_txns().clone());
Expand Down
6 changes: 0 additions & 6 deletions consensus/src/dag/bootstrap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ pub struct DagBootstrapper {
randomness_config: OnChainRandomnessConfig,
jwk_consensus_config: OnChainJWKConsensusConfig,
executor: BoundedExecutor,
allow_batches_without_pos_in_proposal: bool,
}

impl DagBootstrapper {
Expand All @@ -364,7 +363,6 @@ impl DagBootstrapper {
randomness_config: OnChainRandomnessConfig,
jwk_consensus_config: OnChainJWKConsensusConfig,
executor: BoundedExecutor,
allow_batches_without_pos_in_proposal: bool,
) -> Self {
Self {
self_peer,
Expand All @@ -386,7 +384,6 @@ impl DagBootstrapper {
randomness_config,
jwk_consensus_config,
executor,
allow_batches_without_pos_in_proposal,
}
}

Expand Down Expand Up @@ -536,7 +533,6 @@ impl DagBootstrapper {
self.epoch_state.clone(),
parent_block_info,
ledger_info_provider.clone(),
self.allow_batches_without_pos_in_proposal,
));

let order_rule = Arc::new(Mutex::new(OrderRule::new(
Expand Down Expand Up @@ -649,7 +645,6 @@ impl DagBootstrapper {
self.config.node_payload_config.clone(),
health_backoff.clone(),
self.quorum_store_enabled,
self.allow_batches_without_pos_in_proposal,
);
let rb_handler = NodeBroadcastHandler::new(
dag_store.clone(),
Expand Down Expand Up @@ -768,7 +763,6 @@ pub(super) fn bootstrap_dag_for_test(
OnChainRandomnessConfig::default_enabled(),
OnChainJWKConsensusConfig::default_enabled(),
BoundedExecutor::new(2, Handle::current()),
true,
);

let (_base_state, handler, fetch_service) = bootstraper.full_bootstrap();
Expand Down
11 changes: 1 addition & 10 deletions consensus/src/dag/dag_driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ pub(crate) struct DagDriver {
payload_config: DagPayloadConfig,
health_backoff: HealthBackoff,
quorum_store_enabled: bool,
allow_batches_without_pos_in_proposal: bool,
}

impl DagDriver {
Expand All @@ -85,7 +84,6 @@ impl DagDriver {
payload_config: DagPayloadConfig,
health_backoff: HealthBackoff,
quorum_store_enabled: bool,
allow_batches_without_pos_in_proposal: bool,
) -> Self {
let pending_node = storage
.get_pending_node()
Expand All @@ -110,7 +108,6 @@ impl DagDriver {
payload_config,
health_backoff,
quorum_store_enabled,
allow_batches_without_pos_in_proposal,
};

// If we were broadcasting the node for the round already, resume it
Expand Down Expand Up @@ -281,13 +278,7 @@ impl DagDriver {
Ok(payload) => payload,
Err(e) => {
error!("error pulling payload: {}", e);
(
vec![],
Payload::empty(
self.quorum_store_enabled,
self.allow_batches_without_pos_in_proposal,
),
)
(vec![], Payload::empty(self.quorum_store_enabled))
},
};

Expand Down
1 change: 0 additions & 1 deletion consensus/src/dag/tests/dag_driver_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ fn setup(
NoPipelineBackpressure::new(),
),
false,
true,
)
}

Expand Down
4 changes: 2 additions & 2 deletions consensus/src/dag/tests/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(crate) fn new_certified_node(
author,
0,
vec![],
Payload::empty(false, true),
Payload::empty(false),
parents,
Extensions::empty(),
);
Expand All @@ -87,7 +87,7 @@ pub(crate) fn new_node(
author,
timestamp,
vec![],
Payload::empty(false, true),
Payload::empty(false),
parents,
Extensions::empty(),
)
Expand Down
4 changes: 2 additions & 2 deletions consensus/src/dag/tests/types_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn test_node_verify() {

let invalid_node = Node::new_for_test(
NodeMetadata::new_for_test(0, 0, signers[0].author(), 0, HashValue::random()),
Payload::empty(false, true),
Payload::empty(false),
vec![],
Extensions::empty(),
);
Expand Down Expand Up @@ -73,7 +73,7 @@ fn test_certified_node_verify() {

let invalid_node = Node::new_for_test(
NodeMetadata::new_for_test(0, 0, signers[0].author(), 0, HashValue::random()),
Payload::empty(false, true),
Payload::empty(false),
vec![],
Extensions::empty(),
);
Expand Down
6 changes: 0 additions & 6 deletions consensus/src/epoch_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,9 +943,6 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
chain_health_backoff_config,
self.quorum_store_enabled,
onchain_consensus_config.effective_validator_txn_config(),
self.config
.quorum_store
.allow_batches_without_pos_in_proposal,
opt_qs_payload_param_provider,
);
let (round_manager_tx, round_manager_rx) = aptos_channel::new(
Expand Down Expand Up @@ -1508,9 +1505,6 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
onchain_randomness_config,
onchain_jwk_consensus_config,
self.bounded_executor.clone(),
self.config
.quorum_store
.allow_batches_without_pos_in_proposal,
);

let (dag_rpc_tx, dag_rpc_rx) = aptos_channel::new(QueueStyle::FIFO, 10, None);
Expand Down
Loading
Loading