Skip to content

Commit e702cf6

Browse files
committed
test: add test for commit message handling without proposal acceptance
Add failing test that demonstrates QBFT incorrectly drops commit messages when no proposal has been accepted for the current round. This prevents proper catch-up scenarios where nodes should be able to achieve consensus based on commit quorum even without seeing the original proposal. The test shows that commit messages are dropped in the commit message handling logic, which should buffer these messages instead for later processing when a proposal arrives.
1 parent 744f998 commit e702cf6

File tree

1 file changed

+125
-0
lines changed

1 file changed

+125
-0
lines changed

anchor/common/qbft/src/tests.rs

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,128 @@ fn test_node_recovery() {
223223
let num_consensus = test_instance.wait_until_end();
224224
assert_eq!(num_consensus, 5); // Should reach full consensus after recovery
225225
}
226+
227+
#[test]
228+
/// Test that demonstrates QBFT incorrectly drops commit messages when no proposal accepted
229+
///
230+
/// In a proper QBFT implementation, commit messages should be buffered when they arrive
231+
/// before a proposal, allowing catch-up scenarios where a node can achieve consensus
232+
/// based on a commit quorum even without seeing the original proposal.
233+
///
234+
/// Current bug: Individual commit messages are dropped when
235+
/// proposal_accepted_for_current_round is false, preventing nodes from ever
236+
/// reaching commit quorum in catch-up scenarios.
237+
fn test_commit_messages_dropped_without_proposal_acceptance() {
238+
if ENABLE_TEST_LOGGING {
239+
let env_filter = EnvFilter::new("debug");
240+
let _ = tracing_subscriber::fmt()
241+
.compact()
242+
.with_env_filter(env_filter)
243+
.try_init();
244+
}
245+
246+
use ssv_types::{
247+
consensus::QbftMessage,
248+
message::{MsgType, RSA_SIGNATURE_SIZE, SSVMessage, SignedSSVMessage},
249+
};
250+
251+
// Create QBFT instance with 3 nodes (f=0, quorum=3)
252+
let config = ConfigBuilder::<DefaultLeaderFunction>::new(
253+
1.into(),
254+
InstanceHeight::default(),
255+
(1..4).map(OperatorId::from).collect(), // 3 nodes, quorum = 3
256+
)
257+
.with_operator_id(OperatorId::from(1))
258+
.build()
259+
.expect("config should be valid");
260+
261+
let test_data = TestData(789);
262+
let mut qbft_instance = Qbft::new(
263+
config,
264+
test_data.clone(),
265+
Box::new(NoDataValidation),
266+
MessageId::from([0; 56]),
267+
|_| {},
268+
);
269+
270+
// Verify initial state: no proposal accepted
271+
assert!(!qbft_instance.proposal_accepted_for_current_round);
272+
assert!(matches!(
273+
qbft_instance.state,
274+
InstanceState::AwaitingProposal
275+
));
276+
277+
// STEP 1: Send commit messages BEFORE accepting any proposal (catch-up scenario)
278+
// This simulates a node that missed the proposal but receives commit messages from other nodes
279+
280+
let _commit_messages_before = qbft_instance
281+
.commit_container
282+
.get_messages_for_round(1.into())
283+
.len();
284+
285+
// Create 3 valid commit messages for the same data
286+
for operator_id in [1, 2, 3] {
287+
let commit_msg = QbftMessage {
288+
qbft_message_type: QbftMessageType::Commit,
289+
height: 0,
290+
round: 1,
291+
identifier: [0; 56].to_vec().try_into().unwrap(),
292+
root: test_data.hash(),
293+
data_round: 0,
294+
round_change_justification: vec![],
295+
prepare_justification: vec![],
296+
};
297+
298+
let commit_ssv_message = SSVMessage::new(
299+
MsgType::SSVConsensusMsgType,
300+
MessageId::from([0; 56]),
301+
commit_msg.as_ssz_bytes(),
302+
)
303+
.expect("should create commit SSVMessage");
304+
305+
let signed_commit = SignedSSVMessage::new(
306+
vec![vec![0; RSA_SIGNATURE_SIZE]],
307+
vec![OperatorId::from(operator_id)],
308+
commit_ssv_message,
309+
vec![], // no full_data for commit
310+
)
311+
.expect("should create signed commit");
312+
313+
let wrapped_commit = WrappedQbftMessage {
314+
signed_message: signed_commit,
315+
qbft_message: commit_msg,
316+
};
317+
318+
// Send the commit message - this should be buffered, not dropped
319+
qbft_instance.receive(wrapped_commit);
320+
}
321+
322+
let commit_messages_after = qbft_instance
323+
.commit_container
324+
.get_messages_for_round(1.into())
325+
.len();
326+
327+
// The commit messages should be buffered for catch-up scenario processing
328+
// This assertion FAILS due to the bug in commit message handling
329+
assert_eq!(
330+
commit_messages_after, 3,
331+
"BUG: Commit messages should be buffered when no proposal accepted for catch-up scenarios. \
332+
Expected: 3 commit messages buffered. Actual: {} messages. \
333+
Commit messages are dropped when proposal_accepted_for_current_round \
334+
is false, preventing nodes from achieving consensus in catch-up scenarios where they \
335+
receive commits before proposals.",
336+
commit_messages_after
337+
);
338+
339+
// Instance should still be in AwaitingProposal state since commits were dropped
340+
assert!(
341+
matches!(qbft_instance.state, InstanceState::AwaitingProposal),
342+
"Instance should remain in AwaitingProposal state since commits were incorrectly dropped"
343+
);
344+
345+
// The instance should NOT have reached consensus due to dropped commits
346+
assert!(
347+
qbft_instance.completed.is_none(),
348+
"Instance should not have completed consensus due to dropped commit messages"
349+
);
350+
}

0 commit comments

Comments
 (0)