Skip to content

Commit c9d5141

Browse files
committed
test(dash): replace feed_qr_info_does_not_downgrade_already_verified_cycle with direct gate test
The old integration-style test claimed to exercise the storage gate's anti-clobber behavior, but actually never reached the gate. The synthetic fake quorums in `last_commitment_per_index` had a single-element `signers` bitvector, so `validate_structure` rejected them with `InsufficientSigners` and `feed_qr_info` returned `Err` before `store_cycle_if_fully_verified` was ever called. The post-call assertion that `rotated_quorums_per_cycle[K]` was unchanged passed for a trivial reason: nothing wrote to it. The named invariant ("does not downgrade already-verified cycle") was not under test. Replace it with a focused unit test that calls `store_cycle_if_fully_verified` directly and covers both short-circuit branches: (a) the cycle is already fully `Verified`, (b) at least one input entry is not `Verified`. The test asserts the gate returns `Ok(None)` and does not mutate `rotated_quorums_per_cycle` in either case. Net change: -115/+44 lines, 1 of 2 unused test imports also removed. Addresses CodeRabbit review comment on PR #736 #736 (comment)
1 parent a0a0c19 commit c9d5141

1 file changed

Lines changed: 40 additions & 115 deletions

File tree

  • dash/src/sml/masternode_list_engine

dash/src/sml/masternode_list_engine/mod.rs

Lines changed: 40 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,10 +1478,8 @@ mod tests {
14781478
super::build_cycle_quorum_map,
14791479
crate::BlockHash,
14801480
crate::QuorumHash,
1481-
crate::Transaction,
14821481
crate::bls_sig_utils::{BLSPublicKey, BLSSignature},
14831482
crate::hash_types::QuorumVVecHash,
1484-
crate::network::message_qrinfo::{MNSkipListMode, QuorumSnapshot},
14851483
crate::transaction::special_transaction::quorum_commitment::QuorumEntry,
14861484
};
14871485

@@ -2032,134 +2030,61 @@ mod tests {
20322030
);
20332031
}
20342032

2035-
/// Anti-clobber: a thin/incomplete second QRInfo where rotated entries
2036-
/// land as `Skipped` (no rotation CL sigs reachable) must not downgrade
2037-
/// an already-fully-Verified rotation cycle. Feeds the mainnet fixture
2038-
/// once to populate `rotated_quorums_per_cycle[K]` with all entries
2039-
/// `Verified`, then feeds a synthetic self-loop QRInfo whose diffs do
2040-
/// not carry rotation sigs, with `last_commitment_per_index` matching
2041-
/// the existing cycle's first quorum hash plus a fresh fake quorum
2042-
/// that needs new validation. Asserts the existing cycle's entries
2043-
/// remain `Verified`.
2033+
/// Direct coverage for the `store_cycle_if_fully_verified` storage gate.
2034+
/// The gate must short-circuit with `Ok(None)` (and not write) in two
2035+
/// cases: any input entry is not `Verified`, or the target cycle is
2036+
/// already fully `Verified`.
20442037
#[cfg(feature = "quorum_validation")]
20452038
#[test]
2046-
fn feed_qr_info_does_not_downgrade_already_verified_cycle() {
2039+
fn store_cycle_if_fully_verified_short_circuits() {
20472040
let (mut engine, qr_info) = load_qrinfo_2240504_fixture();
2048-
20492041
engine.feed_qr_info(qr_info, false, true).expect("first feed should succeed");
20502042

20512043
let cycle_key = *engine
20522044
.rotated_quorums_per_cycle
20532045
.keys()
20542046
.next()
2055-
.expect("first feed must have stored at least one rotation cycle");
2056-
2047+
.expect("first feed must store at least one rotation cycle");
20572048
let original_cycle =
20582049
engine.rotated_quorums_per_cycle.get(&cycle_key).expect("cycle present").clone();
2050+
let rotation_quorum_type =
2051+
original_cycle.values().next().expect("cycle non-empty").quorum_entry.llmq_type;
2052+
2053+
let already_verified: Vec<QualifiedQuorumEntry> =
2054+
original_cycle.values().cloned().collect();
2055+
let result = engine
2056+
.store_cycle_if_fully_verified(cycle_key, already_verified, rotation_quorum_type)
2057+
.expect("gate must not error on already-verified cycle");
20592058
assert!(
2060-
!original_cycle.is_empty()
2061-
&& original_cycle
2062-
.values()
2063-
.all(|q| matches!(q.verified, LLMQEntryVerificationStatus::Verified)),
2064-
"first feed must leave the cycle fully Verified"
2059+
result.is_none(),
2060+
"gate must short-circuit when target cycle is already fully Verified, got {:?}",
2061+
result
2062+
);
2063+
assert_eq!(
2064+
engine.rotated_quorums_per_cycle.get(&cycle_key).unwrap(),
2065+
&original_cycle,
2066+
"gate must not mutate the stored cycle on the already-verified short-circuit"
20652067
);
20662068

2067-
let known_quorum = original_cycle
2068-
.values()
2069-
.find(|q| q.quorum_entry.quorum_hash == cycle_key)
2070-
.expect("cycle map must contain its key entry")
2071-
.clone();
2072-
let rotation_quorum_type = known_quorum.quorum_entry.llmq_type;
2069+
// Use a fresh cycle_key so the already-verified short-circuit cannot
2070+
// fire. `make_qualified_quorum_entry` defaults `verified` to `Skipped`,
2071+
// so the gate must refuse to write a degraded cycle.
2072+
let fresh_key = BlockHash::from_byte_array([0xAB; 32]);
20732073
let active_count = rotation_quorum_type.active_quorum_count() as i16;
2074-
2075-
// Build `last_commitment_per_index`: first entry is the existing
2076-
// cycle key (so cycle_key matches, exercising the gate), rest are
2077-
// fresh fakes that need new validation.
2078-
let mut last_commitment_per_index = vec![known_quorum.quorum_entry.clone()];
2079-
for i in 1..active_count {
2080-
last_commitment_per_index.push(QuorumEntry {
2081-
version: 2,
2082-
llmq_type: rotation_quorum_type,
2083-
quorum_hash: QuorumHash::from_byte_array([0xCC; 32].map(|b| b ^ i as u8)),
2084-
quorum_index: Some(i),
2085-
signers: vec![true],
2086-
valid_members: vec![true],
2087-
quorum_public_key: BLSPublicKey::from([0; 48]),
2088-
quorum_vvec_hash: QuorumVVecHash::all_zeros(),
2089-
threshold_sig: BLSSignature::from([0; 96]),
2090-
all_commitment_aggregated_signature: BLSSignature::from([0; 96]),
2091-
});
2092-
}
2093-
2094-
// Self-loop diffs at the engine's current tip make the second feed a
2095-
// no-op for masternode-list state while still exercising the QRInfo
2096-
// verification pipeline.
2097-
let tip_hash = engine
2098-
.masternode_lists
2099-
.iter()
2100-
.next_back()
2101-
.map(|(_, list)| list.block_hash)
2102-
.expect("engine has a tip list after first feed");
2103-
let make_self_loop = |hash: BlockHash| MnListDiff {
2104-
version: 1,
2105-
base_block_hash: hash,
2106-
block_hash: hash,
2107-
total_transactions: 0,
2108-
merkle_hashes: vec![],
2109-
merkle_flags: vec![],
2110-
coinbase_tx: Transaction {
2111-
version: 1,
2112-
lock_time: 0,
2113-
input: vec![],
2114-
output: vec![],
2115-
special_transaction_payload: None,
2116-
},
2117-
deleted_masternodes: vec![],
2118-
new_masternodes: vec![],
2119-
deleted_quorums: vec![],
2120-
new_quorums: vec![],
2121-
quorums_chainlock_signatures: vec![],
2122-
};
2123-
let empty_snapshot = || QuorumSnapshot {
2124-
skip_list_mode: MNSkipListMode::NoSkipping,
2125-
active_quorum_members: vec![],
2126-
skip_list: vec![],
2127-
};
2128-
2129-
let thin_qr_info = QRInfo {
2130-
quorum_snapshot_at_h_minus_c: empty_snapshot(),
2131-
quorum_snapshot_at_h_minus_2c: empty_snapshot(),
2132-
quorum_snapshot_at_h_minus_3c: empty_snapshot(),
2133-
mn_list_diff_tip: make_self_loop(tip_hash),
2134-
mn_list_diff_h: make_self_loop(tip_hash),
2135-
mn_list_diff_at_h_minus_c: make_self_loop(tip_hash),
2136-
mn_list_diff_at_h_minus_2c: make_self_loop(tip_hash),
2137-
mn_list_diff_at_h_minus_3c: make_self_loop(tip_hash),
2138-
quorum_snapshot_and_mn_list_diff_at_h_minus_4c: None,
2139-
last_commitment_per_index,
2140-
quorum_snapshot_list: vec![],
2141-
mn_list_diff_list: vec![],
2142-
};
2143-
2144-
let _ = engine.feed_qr_info(thin_qr_info, false, true);
2145-
2146-
let after_cycle = engine
2147-
.rotated_quorums_per_cycle
2148-
.get(&cycle_key)
2149-
.expect("cycle must still exist after thin second feed");
2150-
assert_eq!(
2151-
after_cycle.len(),
2152-
original_cycle.len(),
2153-
"thin second feed must not change cycle entry count"
2074+
let degraded: Vec<QualifiedQuorumEntry> = (0..active_count)
2075+
.map(|i| make_qualified_quorum_entry(rotation_quorum_type, Some(i)))
2076+
.collect();
2077+
let result = engine
2078+
.store_cycle_if_fully_verified(fresh_key, degraded, rotation_quorum_type)
2079+
.expect("gate must not error when no entries are verified");
2080+
assert!(
2081+
result.is_none(),
2082+
"gate must short-circuit when not all entries are Verified, got {:?}",
2083+
result
2084+
);
2085+
assert!(
2086+
!engine.rotated_quorums_per_cycle.contains_key(&fresh_key),
2087+
"gate must not write a degraded cycle"
21542088
);
2155-
for (idx, entry) in after_cycle {
2156-
assert!(
2157-
matches!(entry.verified, LLMQEntryVerificationStatus::Verified),
2158-
"cycle quorum at index {} (hash {}) must remain Verified after thin second feed, got {}",
2159-
idx,
2160-
entry.quorum_entry.quorum_hash,
2161-
entry.verified
2162-
);
2163-
}
21642089
}
21652090
}

0 commit comments

Comments
 (0)