From 405c7e27f5336bf5d4f53c7d92dc3c18e28354d3 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 23 Aug 2026 20:40:55 +0700 Subject: [PATCH 1/4] fix(drive-abci): execute a different block at the same height/round instead of serving the stale context process_proposal keeps its block execution context keyed by height and round only. When a second, different block arrived for the same height and round it either answered with the cached result of the block it held (proposer path: the app hash of our own proposal was returned for the network's block) or refused with an ABCI error. Tenderdash turns the first into a mustValidate panic and the second into a mustEnsureProcess panic, and its WAL replay recreates the same situation on every restart. This is exactly what happens to a validator that hands over from block sync to consensus while still behind: it proposes at a historical height, and the block the network really committed at that height and round then arrives through consensus catch-up (dashpay/tenderdash#1413). Key the cache by block hash: the same block is still served from the proposer cache / re-run as before; a different block at the same height and round drops the stale context and is executed. Two strategy tests pin both behaviours. Co-Authored-By: Claude Fable 5 --- .../src/abci/handler/process_proposal.rs | 67 ++--- .../tests/strategy_tests/test_cases/mod.rs | 1 + .../process_proposal_collision_tests.rs | 256 ++++++++++++++++++ 3 files changed, 292 insertions(+), 32 deletions(-) create mode 100644 packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs diff --git a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs index 69bae1c5fcd..4279ae1bf67 100644 --- a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs @@ -37,48 +37,51 @@ where // We were not the proposer, and we should process something new drop_block_execution_context = true; } else if let Some(current_block_hash) = block_state_info.block_hash() { - // There is also the possibility that this block already came in, but tenderdash crashed - // Now tenderdash is sending it again - if let Some(proposal_info) = block_execution_context.proposer_results() { - tracing::debug!( - method = "process_proposal", - "we knew block hash, block execution context already had a proposer result", - ); - // We were the proposer as well, so we have the result in cache - return Ok(proto::ResponseProcessProposal { - status: proto::response_process_proposal::ProposalStatus::Accept.into(), - app_hash: proposal_info.app_hash.clone(), - tx_results: proposal_info.tx_results.clone(), - consensus_param_updates: proposal_info.consensus_param_updates.clone(), - validator_set_update: proposal_info.validator_set_update.clone(), - events: Vec::new(), - }); - } - if current_block_hash.as_slice() == request.hash { - // We were not the proposer, just drop the execution context - tracing::warn!( + // There is also the possibility that this block already came in, but tenderdash crashed + // Now tenderdash is sending it again + if let Some(proposal_info) = block_execution_context.proposer_results() { + tracing::debug!( method = "process_proposal", - "block execution context already existed, but we are running it again for same height {}/round {}", - request.height, - request.round, + "we knew block hash, block execution context already had a proposer result", ); + // We were the proposer as well, so we have the result in cache + return Ok(proto::ResponseProcessProposal { + status: proto::response_process_proposal::ProposalStatus::Accept.into(), + app_hash: proposal_info.app_hash.clone(), + tx_results: proposal_info.tx_results.clone(), + consensus_param_updates: proposal_info.consensus_param_updates.clone(), + validator_set_update: proposal_info.validator_set_update.clone(), + events: Vec::new(), + }); + } + + // We were not the proposer, just drop the execution context + tracing::warn!( + method = "process_proposal", + "block execution context already existed, but we are running it again for same height {}/round {}", + request.height, + request.round, + ); drop_block_execution_context = true; } else { - // We are getting a different block hash for a block of the same round - // This is a terrible issue - tracing::error!( + // A different block for the same height and round. With a single honest + // proposer per round this only happens when the block we hold is stale: + // typically our own proposal, built while we were still catching up, and + // now the block the network actually committed at this height and round + // arrives through consensus catch-up. The cached result belongs to the + // other block, so it must neither be served nor block execution; the only + // correct answer is to execute the block we were asked about. + tracing::warn!( method = "process_proposal", block_state_info = ?block_state_info, - "received a process proposal request twice with different hash for height {}/round {}: existing hash {:?}, new hash {:?}", + "received a process proposal request for a different block at the same height {}/round {}: existing hash {}, new hash {}; dropping the existing block execution context and executing the new block", request.height, request.round, - current_block_hash, - request.hash, + hex::encode(current_block_hash), + hex::encode(&request.hash), ); - Err(Error::Abci(AbciError::BadRequest( - "received a process proposal request twice with different hash".to_string(), - )))?; + drop_block_execution_context = true; } } else { // we were the proposer diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs index dcd6ddd7ea4..1e963cb1cbd 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs @@ -7,6 +7,7 @@ mod core_update_tests; mod data_contract_history_tests; mod identity_and_document_tests; mod identity_transfer_tests; +mod process_proposal_collision_tests; mod required_since_update_tests; // TODO: re-enable once OperationType shielded variants are implemented // mod shielded_tests; diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs new file mode 100644 index 00000000000..bd124df57a3 --- /dev/null +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs @@ -0,0 +1,256 @@ +//! Two proposals for the same height and round. +//! +//! A node that hands over from block sync to consensus while still behind becomes a +//! proposer at a historical height (it is in that era's quorum) and builds a block of its own +//! there. The committed block for that height then reaches it through consensus catch-up: +//! same height, same round, a different block. Drive's `process_proposal` keeps an execution +//! context keyed only by height and round, so it answers the second block from the first +//! block's cache, or refuses it outright. Either answer wedges Tenderdash: a cached app hash +//! fails `ValidateBlockWithRoundState` (panic in `ApplyCommit`), and an ABCI error panics in +//! `mustEnsureProcess`. The WAL replays both proposals on restart, so the node never recovers. +//! +//! These tests assert the behaviour Drive needs: a proposal whose hash differs from the one +//! in the execution context must be executed afresh, never served from the cache and never +//! rejected with an error. +#[cfg(test)] +mod tests { + use crate::execution::run_chain_for_strategy; + use crate::strategy::{ChainExecutionOutcome, NetworkStrategy}; + use dpp::block::block_info::BlockInfo; + use dpp::block::epoch::Epoch; + use dpp::dashcore::hashes::Hash; + use drive_abci::config::{ExecutionConfig, PlatformConfig, PlatformTestConfig}; + use drive_abci::mimic::MimicExecuteBlockOptions; + use drive_abci::platform_types::platform_state::PlatformStateV0Methods; + use drive_abci::test::helpers::setup::TestPlatformBuilder; + use platform_version::version::PlatformVersion; + use strategy_tests::{IdentityInsertInfo, StartAddresses, StartIdentities, Strategy}; + use tenderdash_abci::proto::abci::response_process_proposal::ProposalStatus; + use tenderdash_abci::proto::abci::RequestProcessProposal; + use tenderdash_abci::proto::google::protobuf::Timestamp; + use tenderdash_abci::proto::version::Consensus; + use tenderdash_abci::Application; + + const BLOCK_SPACING_MS: u64 = 3000; + + fn strategy() -> NetworkStrategy { + NetworkStrategy { + strategy: Strategy { + start_contracts: vec![], + operations: vec![], + start_identities: StartIdentities::default(), + start_addresses: StartAddresses::default(), + identity_inserts: IdentityInsertInfo::default(), + identity_contract_nonce_gaps: None, + signer: None, + }, + total_hpmns: 50, + extra_normal_mns: 0, + validator_quorum_count: 10, + chain_lock_quorum_count: 10, + upgrading_info: None, + proposer_strategy: Default::default(), + rotate_quorums: false, + failure_testing: None, + query_testing: None, + verify_state_transition_results: false, + ..Default::default() + } + } + + fn config() -> PlatformConfig { + PlatformConfig { + execution: ExecutionConfig { + verify_sum_trees: true, + ..Default::default() + }, + block_spacing_ms: BLOCK_SPACING_MS, + testing_configs: PlatformTestConfig::default_minimal_verifications(), + ..Default::default() + } + } + + /// A `RequestProcessProposal` for the next block after `outcome`, attributed to the + /// proposer at `proposer_index`, with `hash` as its block hash. `time_offset_ms` shifts + /// the block time so that two requests built from different offsets execute to + /// different app hashes. + fn next_block_request( + outcome: &ChainExecutionOutcome, + proposer_index: usize, + hash: [u8; 32], + time_offset_ms: u64, + ) -> RequestProcessProposal { + let platform_state = outcome.abci_app.platform.state.load(); + let height = platform_state.last_committed_block_height() + 1; + let core_height = platform_state.last_committed_core_height(); + let time_ms = outcome.end_time_ms + BLOCK_SPACING_MS + time_offset_ms; + let proposer = outcome.proposers[proposer_index].pro_tx_hash(); + + RequestProcessProposal { + txs: vec![], + proposed_last_commit: None, + misbehavior: vec![], + hash: hash.to_vec(), + height: height as i64, + time: Some(Timestamp { + seconds: (time_ms / 1000) as i64, + nanos: ((time_ms % 1000) * 1000) as i32, + }), + next_validators_hash: [0u8; 32].to_vec(), + round: 0, + core_chain_locked_height: core_height, + core_chain_lock_update: None, + proposer_pro_tx_hash: proposer.to_byte_array().to_vec(), + proposed_app_version: PlatformVersion::latest().protocol_version as u64, + version: Some(Consensus { + block: 0, + app: PlatformVersion::latest().protocol_version as u64, + }), + quorum_hash: outcome + .current_quorum() + .quorum_hash + .to_byte_array() + .to_vec(), + } + } + + /// The node prepared its own block at (H, 0) and then receives the network's block for + /// (H, 0) — a different block. Drive must execute it and return *its* app hash, not the + /// app hash of the block the node itself proposed. + #[tokio::test] + async fn process_proposal_for_a_different_block_must_not_be_served_from_the_proposer_cache() { + let config = config(); + let mut platform = TestPlatformBuilder::new() + .with_config(config.clone()) + .build_with_mock_rpc(); + + let outcome = run_chain_for_strategy( + &mut platform, + 5, + strategy(), + config, + 7, + &mut None, + &mut None, + ) + .await; + + let platform_state = outcome.abci_app.platform.state.load(); + let height = platform_state.last_committed_block_height() + 1; + let block_info = BlockInfo { + time_ms: outcome.end_time_ms + BLOCK_SPACING_MS, + height, + core_height: platform_state.last_committed_core_height(), + epoch: Epoch::new(outcome.end_epoch_index).expect("epoch"), + }; + drop(platform_state); + + // PrepareProposal + ProcessProposal of our own block at (H, 0), not finalized: this is + // the state a proposer is in while it waits for votes that will never come. + let own_block = outcome + .abci_app + .mimic_execute_block( + outcome.proposers[0].pro_tx_hash().to_byte_array(), + outcome.current_quorum(), + PlatformVersion::latest().protocol_version, + block_info, + 0, + &[], + false, + vec![], + MimicExecuteBlockOptions { + dont_finalize_block: true, + rounds_before_finalization: None, + max_tx_bytes_per_block: 40000, + independent_process_proposal_verification: false, + }, + ) + .expect("our own block should prepare and process"); + + // The network's block for the same height and round: different proposer, different + // time, different hash. + let network_block = next_block_request(&outcome, 1, [0x42u8; 32], 1000); + + let response = outcome + .abci_app + .process_proposal(network_block.clone()) + .expect("a different block at the same height/round must be processed, not refused"); + + assert_eq!(response.status, ProposalStatus::Accept as i32); + + // What the network block really executes to: run it against a clean context. + outcome + .abci_app + .block_execution_context + .write() + .unwrap() + .take(); + let reference = outcome + .abci_app + .process_proposal(network_block) + .expect("clean execution of the network block"); + + assert_ne!( + reference.app_hash, + own_block.root_app_hash.to_vec(), + "test premise: the two blocks must execute to different app hashes" + ); + assert_eq!( + response.app_hash, reference.app_hash, + "Drive answered the network's block with the app hash of the node's own proposal" + ); + } + + /// Two different blocks processed for the same height and round, neither of them ours: + /// the second must be executed, not answered with an ABCI error (which Tenderdash turns + /// into a panic that the WAL replay reproduces on every restart). + #[tokio::test] + async fn process_proposal_for_a_second_different_block_in_the_same_round_must_execute() { + let config = config(); + let mut platform = TestPlatformBuilder::new() + .with_config(config.clone()) + .build_with_mock_rpc(); + + let outcome = run_chain_for_strategy( + &mut platform, + 5, + strategy(), + config, + 7, + &mut None, + &mut None, + ) + .await; + + let first = next_block_request(&outcome, 1, [0x42u8; 32], 0); + let second = next_block_request(&outcome, 2, [0x43u8; 32], 1000); + + let first_response = outcome + .abci_app + .process_proposal(first) + .expect("first block processes"); + assert_eq!(first_response.status, ProposalStatus::Accept as i32); + + let second_response = outcome.abci_app.process_proposal(second.clone()).expect( + "a second, different block at the same height/round must be executed, not refused", + ); + assert_eq!(second_response.status, ProposalStatus::Accept as i32); + + outcome + .abci_app + .block_execution_context + .write() + .unwrap() + .take(); + let reference = outcome + .abci_app + .process_proposal(second) + .expect("clean execution of the second block"); + + assert_ne!(first_response.app_hash, reference.app_hash); + assert_eq!( + second_response.app_hash, reference.app_hash, + "Drive must answer the second block with its own app hash" + ); + } +} From 822f94de512fa2d0ed34e6801404937571ec5b5c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 25 Aug 2026 09:50:10 +0200 Subject: [PATCH 2/4] fix(drive-abci): use nanosecond scale for proposal test timestamps Co-Authored-By: Claude Fable 5 --- .../test_cases/process_proposal_collision_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs index bd124df57a3..b552dea9c82 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs @@ -94,7 +94,7 @@ mod tests { height: height as i64, time: Some(Timestamp { seconds: (time_ms / 1000) as i64, - nanos: ((time_ms % 1000) * 1000) as i32, + nanos: ((time_ms % 1000) * 1_000_000) as i32, }), next_validators_hash: [0u8; 32].to_vec(), round: 0, From 4fde0962bdebafd37bbaf46226bbe64ed7f3cb86 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 25 Aug 2026 11:35:29 +0200 Subject: [PATCH 3/4] fix(drive-abci): match hashless prepared contexts on full proposal content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hash-aware branch discards a stale execution context and executes a different block at the same height and round, but the context left behind immediately after PrepareProposal has no block hash to compare against — Tenderdash computes one only once the response is in. That context was matched on transaction count and the core chain lock update alone, so the first competing ProcessProposal at the same height and round could still be served the prepared block's app hash. Height was never compared at all. Decide identity on the execution inputs instead: the ordered transaction bytes, block time, proposer, core chain locked height, core chain lock update, proposed app version, validator set quorum hash, consensus app version and height. Anything BlockProposal discards on conversion — proposed_last_commit, misbehavior, next_validators_hash and the consensus block version — cannot reach execution and is left out. A mismatch on any of them takes the same way out the hash-mismatch case already took: discard the context and execute the block we were asked about. The proposed app version and the quorum hash were not retained anywhere, so proposer_results now carries them alongside the response. It is written only by prepare_proposal and read only by process_proposal, so every other construction site keeps its `proposer_results: None` unchanged. Co-Authored-By: Claude Fable 5 --- .../src/abci/handler/prepare_proposal.rs | 14 +- .../src/abci/handler/process_proposal.rs | 171 ++++++--- .../types/block_execution_context/mod.rs | 13 +- .../types/block_execution_context/v0/mod.rs | 43 ++- .../process_proposal_collision_tests.rs | 341 +++++++++++++++++- 5 files changed, 495 insertions(+), 87 deletions(-) diff --git a/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs b/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs index e85bd7dc285..269abc53a7d 100644 --- a/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs @@ -3,7 +3,7 @@ use crate::abci::AbciError; use crate::error::Error; use crate::execution::engine::consensus_params_update::consensus_params_update; use crate::execution::types::block_execution_context::v0::{ - BlockExecutionContextV0Getters, BlockExecutionContextV0Setters, + BlockExecutionContextV0Getters, BlockExecutionContextV0Setters, ProposerResults, }; use crate::platform_types::block_execution_outcome; use crate::platform_types::block_proposal::v0::BlockProposal; @@ -154,6 +154,12 @@ where .as_ref() .expect("transaction must be started"); + // Taken from the proposal execution is about to consume: process proposal needs them to + // tell this block apart from a competing one at the same height and round, and nothing else + // in the block execution context retains them. + let proposed_app_version = block_proposal.proposed_app_version; + let validator_set_quorum_hash = block_proposal.validator_set_quorum_hash; + // Running the proposal executes all the state transitions for the block let mut run_result = app.platform().run_block_proposal( block_proposal, @@ -280,7 +286,11 @@ where app_version: platform_version.protocol_version as u64, }; - block_execution_context.set_proposer_results(Some(response.clone())); + block_execution_context.set_proposer_results(Some(ProposerResults { + response: response.clone(), + proposed_app_version, + validator_set_quorum_hash, + })); app.block_execution_context() .write() diff --git a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs index 4279ae1bf67..f1df6687163 100644 --- a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs @@ -3,11 +3,12 @@ use crate::abci::AbciError; use crate::error::Error; use crate::execution::engine::consensus_params_update::consensus_params_update; use crate::execution::types::block_execution_context::v0::{ - BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, + BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, ProposerResults, }; use crate::execution::types::block_state_info::v0::{ BlockStateInfoV0Getters, BlockStateInfoV0Setters, }; +use crate::execution::types::block_state_info::BlockStateInfo; use crate::platform_types::block_execution_outcome; use crate::platform_types::platform_state::PlatformStateV0Methods; use crate::platform_types::state_transitions_processing_result::StateTransitionExecutionResult; @@ -16,6 +17,76 @@ use dpp::dashcore::Network; use dpp::version::TryIntoPlatformVersioned; use tenderdash_abci::proto::abci as proto; use tenderdash_abci::proto::abci::tx_record::TxAction; +use tenderdash_abci::proto::ToMillis; + +/// Compares the proposal our own `PrepareProposal` executed against the one `request` describes, +/// returning the name of the first execution input that differs, or `None` when the two are the +/// same block. +/// +/// Tenderdash computes a block hash only once `PrepareProposal` has returned, so a context it +/// left behind has no hash to match on and identity has to come from the execution inputs +/// instead. Every field `run_block_proposal` reads is compared here, the round excepted — the +/// caller has already established that. The request fields left out are exactly the ones +/// `BlockProposal` discards on conversion — `proposed_last_commit`, `misbehavior`, +/// `next_validators_hash` and the consensus block version — which therefore cannot reach +/// execution or move the app hash. +fn prepared_proposal_difference( + block_state_info: &BlockStateInfo, + proposer_results: &ProposerResults, + request: &proto::RequestProcessProposal, +) -> Option<&'static str> { + if u64::try_from(request.height).ok() != Some(block_state_info.height()) { + return Some("height"); + } + + let request_block_time_ms = request.time.as_ref().and_then(|time| time.to_millis().ok()); + if request_block_time_ms != Some(block_state_info.block_time_ms()) { + return Some("block time"); + } + + if request.proposer_pro_tx_hash != block_state_info.proposer_pro_tx_hash() { + return Some("proposer pro tx hash"); + } + + if request.core_chain_locked_height != block_state_info.core_chain_locked_height() { + return Some("core chain locked height"); + } + + if request.core_chain_lock_update != proposer_results.response.core_chain_lock_update { + return Some("core chain lock update"); + } + + if request.proposed_app_version != proposer_results.proposed_app_version { + return Some("proposed app version"); + } + + if request.quorum_hash != proposer_results.validator_set_quorum_hash { + return Some("validator set quorum hash"); + } + + if request.version.as_ref().map(|version| version.app) + != Some(proposer_results.response.app_version) + { + return Some("consensus app version"); + } + + // The transactions Tenderdash builds the block from are the ones we did not ask it to drop, + // in the order we returned them, so compare the bytes rather than just how many there are. + let prepared_transactions = proposer_results + .response + .tx_records + .iter() + .filter(|record| { + record.action != TxAction::Removed as i32 && record.action != TxAction::Delayed as i32 + }) + .map(|record| &record.tx); + + if !prepared_transactions.eq(request.txs.iter()) { + return Some("transactions"); + } + + None +} pub fn process_proposal<'a, A, C>( app: &A, @@ -40,7 +111,7 @@ where if current_block_hash.as_slice() == request.hash { // There is also the possibility that this block already came in, but tenderdash crashed // Now tenderdash is sending it again - if let Some(proposal_info) = block_execution_context.proposer_results() { + if let Some(proposer_results) = block_execution_context.proposer_results() { tracing::debug!( method = "process_proposal", "we knew block hash, block execution context already had a proposer result", @@ -48,10 +119,16 @@ where // We were the proposer as well, so we have the result in cache return Ok(proto::ResponseProcessProposal { status: proto::response_process_proposal::ProposalStatus::Accept.into(), - app_hash: proposal_info.app_hash.clone(), - tx_results: proposal_info.tx_results.clone(), - consensus_param_updates: proposal_info.consensus_param_updates.clone(), - validator_set_update: proposal_info.validator_set_update.clone(), + app_hash: proposer_results.response.app_hash.clone(), + tx_results: proposer_results.response.tx_results.clone(), + consensus_param_updates: proposer_results + .response + .consensus_param_updates + .clone(), + validator_set_update: proposer_results + .response + .validator_set_update + .clone(), events: Vec::new(), }); } @@ -84,51 +161,46 @@ where drop_block_execution_context = true; } } else { - // we were the proposer - let Some(proposal_info) = block_execution_context.proposer_results() else { + // We were the proposer. Tenderdash only computes the block hash after our + // PrepareProposal has returned, so the context we hold carries none and the block + // we prepared has to be recognised by the inputs it executed. + let Some(proposer_results) = block_execution_context.proposer_results() else { Err(Error::Abci(AbciError::BadRequest( "received a process proposal request twice".to_string(), )))? }; - let expected_transactions = proposal_info - .tx_records - .iter() - .filter_map(|record| { - if record.action == TxAction::Removed as i32 - || record.action == TxAction::Delayed as i32 - { - None - } else { - Some(&record.tx) - } - }) - .collect::>(); - - // While it is true that the length could be same, seeing how this is such a rare situation - // It does not seem worth to deal with situations where the length is the same but the transactions have changed - if expected_transactions.len() == request.txs.len() - && proposal_info.core_chain_lock_update == request.core_chain_lock_update + if let Some(difference) = + prepared_proposal_difference(block_state_info, proposer_results, &request) { - let (app_hash, tx_results, consensus_param_updates, validator_set_update) = { - tracing::debug!( - method = "process_proposal", - "we didn't know block hash (we were most likely proposer), block execution context already had a proposer result {:?}", - proposal_info, - ); - - // Cloning all required properties from proposal_info and then dropping it - let app_hash = proposal_info.app_hash.clone(); - let tx_results = proposal_info.tx_results.clone(); - let consensus_param_updates = proposal_info.consensus_param_updates.clone(); - let validator_set_update = proposal_info.validator_set_update.clone(); - ( - app_hash, - tx_results, - consensus_param_updates, - validator_set_update, - ) - }; + // A different block at the same height and round, reaching us before Tenderdash + // ever told us a hash for the one we prepared. Answering it with the prepared + // app hash is the same failure the hash-aware branch above guards against, so + // take the same way out: discard the stale context and execute what we were + // asked about. + tracing::warn!( + method = "process_proposal", + block_state_info = ?block_state_info, + difference, + "received a process proposal request for a block that is not the one we prepared at the same height {}/round {}; dropping the existing block execution context and executing the new block", + request.height, + request.round, + ); + + drop_block_execution_context = true; + } else { + tracing::debug!( + method = "process_proposal", + "we didn't know block hash (we were most likely proposer), block execution context already had a proposer result {:?}", + proposer_results, + ); + + // Cloning all required properties from the prepared response and then dropping it + let app_hash = proposer_results.response.app_hash.clone(); + let tx_results = proposer_results.response.tx_results.clone(); + let consensus_param_updates = + proposer_results.response.consensus_param_updates.clone(); + let validator_set_update = proposer_results.response.validator_set_update.clone(); // We need to set the block hash block_execution_context @@ -146,14 +218,7 @@ where validator_set_update, events: Vec::new(), }); - } else { - tracing::warn!( - method = "process_proposal", - "we didn't know block hash (we were most likely proposer), block execution context already had a proposer result, but we are requesting a different amount of transactions, dropping the cache", - ); - - drop_block_execution_context = true; - }; + } } } diff --git a/packages/rs-drive-abci/src/execution/types/block_execution_context/mod.rs b/packages/rs-drive-abci/src/execution/types/block_execution_context/mod.rs index 3a866d17c75..42ebb8405e6 100644 --- a/packages/rs-drive-abci/src/execution/types/block_execution_context/mod.rs +++ b/packages/rs-drive-abci/src/execution/types/block_execution_context/mod.rs @@ -4,14 +4,13 @@ pub mod v0; use crate::error::Error; use crate::execution::types::block_execution_context::v0::{ BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, - BlockExecutionContextV0OwnedGetters, BlockExecutionContextV0Setters, + BlockExecutionContextV0OwnedGetters, BlockExecutionContextV0Setters, ProposerResults, }; use crate::execution::types::block_state_info::BlockStateInfo; use crate::platform_types::epoch_info::EpochInfo; use crate::platform_types::platform_state::PlatformState; use crate::platform_types::withdrawal::unsigned_withdrawal_txs::v0::UnsignedWithdrawalTxs; use derive_more::From; -use tenderdash_abci::proto::abci::ResponsePrepareProposal; /// The versioned block execution context #[derive(Debug, From, Clone)] @@ -54,7 +53,7 @@ impl BlockExecutionContextV0Getters for BlockExecutionContext { } } - fn proposer_results(&self) -> Option<&ResponsePrepareProposal> { + fn proposer_results(&self) -> Option<&ProposerResults> { match self { BlockExecutionContext::V0(v0) => v0.proposer_results.as_ref(), } @@ -86,7 +85,7 @@ impl BlockExecutionContextV0Setters for BlockExecutionContext { } } - fn set_proposer_results(&mut self, results: Option) { + fn set_proposer_results(&mut self, results: Option) { match self { BlockExecutionContext::V0(v0) => v0.proposer_results = results, } @@ -112,7 +111,7 @@ impl BlockExecutionContextV0MutableGetters for BlockExecutionContext { } } - fn proposer_results_mut(&mut self) -> Option<&mut ResponsePrepareProposal> { + fn proposer_results_mut(&mut self) -> Option<&mut ProposerResults> { match self { BlockExecutionContext::V0(v0) => v0.proposer_results_mut(), } @@ -147,8 +146,8 @@ impl BlockExecutionContextV0OwnedGetters for BlockExecutionContext { } } - /// Consumes the object and returns the owned `ResponsePrepareProposal`. - fn proposer_results_owned(self) -> Option { + /// Consumes the object and returns the owned `ProposerResults`. + fn proposer_results_owned(self) -> Option { match self { BlockExecutionContext::V0(v0) => v0.proposer_results, } diff --git a/packages/rs-drive-abci/src/execution/types/block_execution_context/v0/mod.rs b/packages/rs-drive-abci/src/execution/types/block_execution_context/v0/mod.rs index dee0160394e..d166a0a96df 100644 --- a/packages/rs-drive-abci/src/execution/types/block_execution_context/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/types/block_execution_context/v0/mod.rs @@ -9,6 +9,27 @@ use dpp::address_funds::PlatformAddress; use dpp::fee::Credits; use tenderdash_abci::proto::abci::ResponsePrepareProposal; +/// What our own `PrepareProposal` left behind for a later `ProcessProposal` to answer from. +/// +/// Tenderdash computes the block hash only once `PrepareProposal` has returned, so the context +/// left behind carries no block hash and `ProcessProposal` has to recognise the prepared block +/// by the inputs it executed instead. Most of those are kept elsewhere already — height, round, +/// block time, proposer and core chain locked height in the block state info, the transactions +/// and the core chain lock update in the response — and the two below are the ones nothing else +/// retains. +#[derive(Debug, Clone)] +pub struct ProposerResults { + /// The response `PrepareProposal` returned, replayed verbatim when the block coming back + /// through `ProcessProposal` is the one that was prepared. + pub response: ResponsePrepareProposal, + /// The app version this node voted for in the block it prepared. + /// `update_validator_proposed_app_version` writes it to state, so it moves the app hash. + pub proposed_app_version: u64, + /// The quorum the prepared block's unsigned withdrawal transactions were built against, + /// and which is asked to sign them on extend vote. + pub validator_set_quorum_hash: [u8; 32], +} + /// V0 of the Block execution context #[derive(Debug, Clone)] pub struct BlockExecutionContextV0 { @@ -22,8 +43,8 @@ pub struct BlockExecutionContextV0 { pub block_address_balance_changes: BTreeMap, /// Block state pub block_platform_state: PlatformState, - /// The response prepare proposal if proposed by us - pub proposer_results: Option, + /// The prepare proposal results if proposed by us + pub proposer_results: Option, } /// A trait defining getter methods for interacting with a BlockExecutionContextV0. pub trait BlockExecutionContextV0Getters { @@ -40,7 +61,7 @@ pub trait BlockExecutionContextV0Getters { fn block_platform_state(&self) -> &PlatformState; /// Returns a reference of the proposer_results field. - fn proposer_results(&self) -> Option<&ResponsePrepareProposal>; + fn proposer_results(&self) -> Option<&ProposerResults>; } /// A trait defining setter methods for interacting with a BlockExecutionContextV0. @@ -58,7 +79,7 @@ pub trait BlockExecutionContextV0Setters { fn set_block_platform_state(&mut self, state: PlatformState); /// Sets the proposer_results field. - fn set_proposer_results(&mut self, results: Option); + fn set_proposer_results(&mut self, results: Option); } /// A trait defining methods for interacting with a BlockExecutionContextV0. @@ -73,7 +94,7 @@ pub trait BlockExecutionContextV0MutableGetters { fn block_platform_state_mut(&mut self) -> &mut PlatformState; /// Returns a mutable reference to the proposer_results field. - fn proposer_results_mut(&mut self) -> Option<&mut ResponsePrepareProposal>; + fn proposer_results_mut(&mut self) -> Option<&mut ProposerResults>; /// Returns a mut reference of the withdrawal_transactions field. fn unsigned_withdrawal_transactions_mut(&mut self) -> &mut UnsignedWithdrawalTxs; @@ -91,7 +112,7 @@ pub trait BlockExecutionContextV0OwnedGetters { fn block_platform_state_owned(self) -> PlatformState; /// Consumes the BlockExecutionContextV0 and returns the proposer_results field. - fn proposer_results_owned(self) -> Option; + fn proposer_results_owned(self) -> Option; } impl BlockExecutionContextV0Getters for BlockExecutionContextV0 { @@ -116,7 +137,7 @@ impl BlockExecutionContextV0Getters for BlockExecutionContextV0 { } /// Returns a reference to the proposer_results field. - fn proposer_results(&self) -> Option<&ResponsePrepareProposal> { + fn proposer_results(&self) -> Option<&ProposerResults> { self.proposer_results.as_ref() } } @@ -139,7 +160,7 @@ impl BlockExecutionContextV0Setters for BlockExecutionContextV0 { self.block_platform_state = state; } /// Sets the proposer_results field. - fn set_proposer_results(&mut self, results: Option) { + fn set_proposer_results(&mut self, results: Option) { self.proposer_results = results; } } @@ -161,7 +182,7 @@ impl BlockExecutionContextV0MutableGetters for BlockExecutionContextV0 { } /// Returns a mutable reference to the proposer_results field. - fn proposer_results_mut(&mut self) -> Option<&mut ResponsePrepareProposal> { + fn proposer_results_mut(&mut self) -> Option<&mut ProposerResults> { self.proposer_results.as_mut() } @@ -186,8 +207,8 @@ impl BlockExecutionContextV0OwnedGetters for BlockExecutionContextV0 { self.block_platform_state } - /// Consumes the object and returns the owned `ResponsePrepareProposal`. - fn proposer_results_owned(self) -> Option { + /// Consumes the object and returns the owned `ProposerResults`. + fn proposer_results_owned(self) -> Option { self.proposer_results } } diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs index b552dea9c82..b2fe58a7222 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/process_proposal_collision_tests.rs @@ -9,9 +9,11 @@ //! fails `ValidateBlockWithRoundState` (panic in `ApplyCommit`), and an ABCI error panics in //! `mustEnsureProcess`. The WAL replays both proposals on restart, so the node never recovers. //! -//! These tests assert the behaviour Drive needs: a proposal whose hash differs from the one -//! in the execution context must be executed afresh, never served from the cache and never -//! rejected with an error. +//! These tests assert the behaviour Drive needs: a proposal that is not the one the execution +//! context holds must be executed afresh, never served from the cache and never rejected with +//! an error. That has to hold in both states the context can be in — after Tenderdash has told +//! us a block hash, and in the window right after `PrepareProposal` where it has not computed +//! one yet and the prepared block is identified by its execution inputs alone. #[cfg(test)] mod tests { use crate::execution::run_chain_for_strategy; @@ -20,18 +22,24 @@ mod tests { use dpp::block::epoch::Epoch; use dpp::dashcore::hashes::Hash; use drive_abci::config::{ExecutionConfig, PlatformConfig, PlatformTestConfig}; + use drive_abci::execution::types::block_execution_context::v0::BlockExecutionContextV0Getters; + use drive_abci::execution::types::block_state_info::v0::BlockStateInfoV0Getters; use drive_abci::mimic::MimicExecuteBlockOptions; use drive_abci::platform_types::platform_state::PlatformStateV0Methods; use drive_abci::test::helpers::setup::TestPlatformBuilder; use platform_version::version::PlatformVersion; use strategy_tests::{IdentityInsertInfo, StartAddresses, StartIdentities, Strategy}; use tenderdash_abci::proto::abci::response_process_proposal::ProposalStatus; - use tenderdash_abci::proto::abci::RequestProcessProposal; + use tenderdash_abci::proto::abci::tx_record::TxAction; + use tenderdash_abci::proto::abci::{ + RequestPrepareProposal, RequestProcessProposal, ResponsePrepareProposal, + }; use tenderdash_abci::proto::google::protobuf::Timestamp; use tenderdash_abci::proto::version::Consensus; use tenderdash_abci::Application; const BLOCK_SPACING_MS: u64 = 3000; + const MAX_TX_BYTES_PER_BLOCK: i64 = 40000; fn strategy() -> NetworkStrategy { NetworkStrategy { @@ -70,10 +78,21 @@ mod tests { } } + /// The time of the next block after `outcome`. `time_offset_ms` shifts it so that two + /// proposals built from different offsets are different blocks. + fn next_block_time_ms(outcome: &ChainExecutionOutcome, time_offset_ms: u64) -> u64 { + outcome.end_time_ms + BLOCK_SPACING_MS + time_offset_ms + } + + fn timestamp(time_ms: u64) -> Timestamp { + Timestamp { + seconds: (time_ms / 1000) as i64, + nanos: ((time_ms % 1000) * 1_000_000) as i32, + } + } + /// A `RequestProcessProposal` for the next block after `outcome`, attributed to the - /// proposer at `proposer_index`, with `hash` as its block hash. `time_offset_ms` shifts - /// the block time so that two requests built from different offsets execute to - /// different app hashes. + /// proposer at `proposer_index`, with `hash` as its block hash. fn next_block_request( outcome: &ChainExecutionOutcome, proposer_index: usize, @@ -83,7 +102,6 @@ mod tests { let platform_state = outcome.abci_app.platform.state.load(); let height = platform_state.last_committed_block_height() + 1; let core_height = platform_state.last_committed_core_height(); - let time_ms = outcome.end_time_ms + BLOCK_SPACING_MS + time_offset_ms; let proposer = outcome.proposers[proposer_index].pro_tx_hash(); RequestProcessProposal { @@ -92,10 +110,7 @@ mod tests { misbehavior: vec![], hash: hash.to_vec(), height: height as i64, - time: Some(Timestamp { - seconds: (time_ms / 1000) as i64, - nanos: ((time_ms % 1000) * 1_000_000) as i32, - }), + time: Some(timestamp(next_block_time_ms(outcome, time_offset_ms))), next_validators_hash: [0u8; 32].to_vec(), round: 0, core_chain_locked_height: core_height, @@ -114,6 +129,114 @@ mod tests { } } + /// The `RequestPrepareProposal` the node's own Tenderdash sends when it is the proposer at + /// `proposer_index` for the next block after `outcome`. Deliberately paired with + /// `next_block_request` at the same offset and proposer index: the two then describe the + /// same block, so a test that varies exactly one field isolates that field. + fn next_block_prepare_request( + outcome: &ChainExecutionOutcome, + proposer_index: usize, + time_offset_ms: u64, + ) -> RequestPrepareProposal { + let platform_state = outcome.abci_app.platform.state.load(); + let height = platform_state.last_committed_block_height() + 1; + let core_height = platform_state.last_committed_core_height(); + let proposer = outcome.proposers[proposer_index].pro_tx_hash(); + + RequestPrepareProposal { + max_tx_bytes: MAX_TX_BYTES_PER_BLOCK, + txs: vec![], + local_last_commit: None, + misbehavior: vec![], + height: height as i64, + time: Some(timestamp(next_block_time_ms(outcome, time_offset_ms))), + next_validators_hash: [0u8; 32].to_vec(), + round: 0, + core_chain_locked_height: core_height, + proposer_pro_tx_hash: proposer.to_byte_array().to_vec(), + proposed_app_version: PlatformVersion::latest().protocol_version as u64, + // Tenderdash sends 0 on prepare proposal; the app version it puts in the header + // is the one that comes back in the response. + version: Some(Consensus { block: 0, app: 0 }), + quorum_hash: outcome + .current_quorum() + .quorum_hash + .to_byte_array() + .to_vec(), + } + } + + /// The `RequestProcessProposal` Tenderdash sends back for the block it just had prepared: + /// the transactions the proposer kept, in order, the app version the response asked for in + /// the header, and the block hash Tenderdash has computed by now. + fn process_request_for_prepared_block( + prepare_request: &RequestPrepareProposal, + prepare_response: &ResponsePrepareProposal, + time_ms: u64, + hash: [u8; 32], + ) -> RequestProcessProposal { + let txs = prepare_response + .tx_records + .iter() + .filter(|record| { + record.action != TxAction::Removed as i32 + && record.action != TxAction::Delayed as i32 + }) + .map(|record| record.tx.clone()) + .collect(); + + RequestProcessProposal { + txs, + proposed_last_commit: None, + misbehavior: vec![], + hash: hash.to_vec(), + height: prepare_request.height, + time: Some(timestamp(time_ms)), + next_validators_hash: prepare_request.next_validators_hash.clone(), + round: prepare_request.round, + core_chain_locked_height: prepare_response + .core_chain_lock_update + .as_ref() + .map(|chain_lock| chain_lock.core_block_height) + .unwrap_or(prepare_request.core_chain_locked_height), + core_chain_lock_update: prepare_response.core_chain_lock_update.clone(), + proposer_pro_tx_hash: prepare_request.proposer_pro_tx_hash.clone(), + proposed_app_version: prepare_request.proposed_app_version, + version: Some(Consensus { + block: 0, + app: prepare_response.app_version, + }), + quorum_hash: prepare_request.quorum_hash.clone(), + } + } + + /// The block time the execution context currently holds, which is the block Drive last + /// executed. Reading it tells a re-execution apart from a cache hit without depending on + /// the two blocks reaching different app hashes. + fn context_block_time_ms(outcome: &ChainExecutionOutcome) -> u64 { + outcome + .abci_app + .block_execution_context + .read() + .unwrap() + .as_ref() + .expect("a block execution context must be held after process proposal") + .block_state_info() + .block_time_ms() + } + + fn context_block_hash(outcome: &ChainExecutionOutcome) -> Option<[u8; 32]> { + outcome + .abci_app + .block_execution_context + .read() + .unwrap() + .as_ref() + .expect("a block execution context must be held after process proposal") + .block_state_info() + .block_hash() + } + /// The node prepared its own block at (H, 0) and then receives the network's block for /// (H, 0) — a different block. Drive must execute it and return *its* app hash, not the /// app hash of the block the node itself proposed. @@ -138,7 +261,7 @@ mod tests { let platform_state = outcome.abci_app.platform.state.load(); let height = platform_state.last_committed_block_height() + 1; let block_info = BlockInfo { - time_ms: outcome.end_time_ms + BLOCK_SPACING_MS, + time_ms: next_block_time_ms(&outcome, 0), height, core_height: platform_state.last_committed_core_height(), epoch: Epoch::new(outcome.end_epoch_index).expect("epoch"), @@ -161,7 +284,7 @@ mod tests { MimicExecuteBlockOptions { dont_finalize_block: true, rounds_before_finalization: None, - max_tx_bytes_per_block: 40000, + max_tx_bytes_per_block: MAX_TX_BYTES_PER_BLOCK as u64, independent_process_proposal_verification: false, }, ) @@ -253,4 +376,194 @@ mod tests { "Drive must answer the second block with its own app hash" ); } + + /// The window right after `PrepareProposal`, before Tenderdash has computed a hash for the + /// block it just had prepared: the execution context holds a proposer result and no block + /// hash, so there is no hash to compare a competing block against. + /// + /// The competing block here matches the prepared one in everything the height-and-round + /// cache used to look at — same transaction count (none either side) and the same absent + /// core chain lock update — and even in block time, differing only in its proposer. That is + /// enough to make it a different block: the proposer is written to state as the block's + /// author and as the caster of its app version vote, so it moves the app hash. Drive must + /// execute it rather than hand back the prepared block's app hash. + #[tokio::test] + async fn process_proposal_for_a_different_block_before_any_block_hash_is_known_must_not_be_served_from_the_proposer_cache( + ) { + let config = config(); + let mut platform = TestPlatformBuilder::new() + .with_config(config.clone()) + .build_with_mock_rpc(); + + let outcome = run_chain_for_strategy( + &mut platform, + 5, + strategy(), + config, + 7, + &mut None, + &mut None, + ) + .await; + + // Our own PrepareProposal, and nothing after it: no ProcessProposal has run, so + // nothing has told the context a block hash. + let prepare_response = outcome + .abci_app + .prepare_proposal(next_block_prepare_request(&outcome, 0, 0)) + .expect("our own block should prepare"); + assert_eq!( + context_block_hash(&outcome), + None, + "test premise: the context left by prepare proposal must carry no block hash" + ); + + // The competing block: same height, same round, same block time, no transactions and + // no core chain lock update on either side. Only the proposer differs. + let competing_block = next_block_request(&outcome, 1, [0x42u8; 32], 0); + + let response = outcome + .abci_app + .process_proposal(competing_block.clone()) + .expect("a different block at the same height/round must be processed, not refused"); + + assert_eq!(response.status, ProposalStatus::Accept as i32); + + // What the competing block really executes to: run it against a clean context. + outcome + .abci_app + .block_execution_context + .write() + .unwrap() + .take(); + let reference = outcome + .abci_app + .process_proposal(competing_block) + .expect("clean execution of the competing block"); + + assert_ne!( + reference.app_hash, prepare_response.app_hash, + "test premise: the two blocks must execute to different app hashes" + ); + assert_eq!( + response.app_hash, reference.app_hash, + "Drive answered a competing block with the app hash of the block it had prepared" + ); + } + + /// The same window, with a competing block that differs from the prepared one only in its + /// block time. Nothing in the pair reaches state through a path that a shifted time alone + /// would redirect, so the two execute to the same app hash and the served hash proves + /// nothing; what the context holds afterwards does. A cache hit would leave the prepared + /// block's time in place and merely stamp the request's hash onto it, so finding the + /// competing block's time there is what says Drive executed the block it was asked about. + #[tokio::test] + async fn process_proposal_for_a_block_differing_only_in_time_before_any_block_hash_is_known_must_execute( + ) { + let config = config(); + let mut platform = TestPlatformBuilder::new() + .with_config(config.clone()) + .build_with_mock_rpc(); + + let outcome = run_chain_for_strategy( + &mut platform, + 5, + strategy(), + config, + 7, + &mut None, + &mut None, + ) + .await; + + outcome + .abci_app + .prepare_proposal(next_block_prepare_request(&outcome, 0, 0)) + .expect("our own block should prepare"); + assert_eq!( + context_block_time_ms(&outcome), + next_block_time_ms(&outcome, 0), + "test premise: the context holds the prepared block's time" + ); + + // Same proposer, same height and round, no transactions and no core chain lock update + // on either side: a shifted block time is the only difference. + let competing_block = next_block_request(&outcome, 0, [0x44u8; 32], 1000); + + let response = outcome + .abci_app + .process_proposal(competing_block) + .expect("a different block at the same height/round must be processed, not refused"); + + assert_eq!(response.status, ProposalStatus::Accept as i32); + assert_eq!( + context_block_time_ms(&outcome), + next_block_time_ms(&outcome, 1000), + "Drive answered a block one second later than the one it prepared out of the prepared context" + ); + assert_eq!( + context_block_hash(&outcome), + Some([0x44u8; 32]), + "the context must be the competing block's own" + ); + } + + /// The legitimate case the proposer cache exists for: Tenderdash comes back with the very + /// block it just had prepared, now carrying a hash. Identity is established from the + /// execution inputs, the prepared result is replayed instead of executing the block twice, + /// and the hash Tenderdash computed is recorded on the context that extend vote and + /// finalize block go on to use. + #[tokio::test] + async fn process_proposal_for_the_prepared_block_must_still_be_served_from_the_proposer_cache() + { + let config = config(); + let mut platform = TestPlatformBuilder::new() + .with_config(config.clone()) + .build_with_mock_rpc(); + + let outcome = run_chain_for_strategy( + &mut platform, + 5, + strategy(), + config, + 7, + &mut None, + &mut None, + ) + .await; + + let prepare_request = next_block_prepare_request(&outcome, 0, 0); + let prepare_response = outcome + .abci_app + .prepare_proposal(prepare_request.clone()) + .expect("our own block should prepare"); + + let prepared_block = process_request_for_prepared_block( + &prepare_request, + &prepare_response, + next_block_time_ms(&outcome, 0), + [0x45u8; 32], + ); + + let response = outcome + .abci_app + .process_proposal(prepared_block) + .expect("the prepared block must be processed"); + + assert_eq!(response.status, ProposalStatus::Accept as i32); + assert_eq!( + response.app_hash, prepare_response.app_hash, + "the block we prepared must still be answered from the proposer cache" + ); + assert_eq!( + context_block_hash(&outcome), + Some([0x45u8; 32]), + "the cached context must record the hash Tenderdash computed for it" + ); + assert_eq!( + context_block_time_ms(&outcome), + next_block_time_ms(&outcome, 0), + "the cached context must be the one prepare proposal built" + ); + } } From 8d196921ef267aacac0172e8795c6f5ce5abf68e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 25 Aug 2026 11:43:11 +0200 Subject: [PATCH 4/4] ci: run the process_proposal collision regression tests on pull requests They pin a consensus-halt regression, run in under a second combined, and their binary is already built by the PR phase. Excluded from the push-only chain-simulation phase to keep the codecov flags disjoint. Co-Authored-By: Claude Fable 5 --- .github/workflows/tests-rs-workspace.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests-rs-workspace.yml b/.github/workflows/tests-rs-workspace.yml index 1c159c2c706..6e64434f886 100644 --- a/.github/workflows/tests-rs-workspace.yml +++ b/.github/workflows/tests-rs-workspace.yml @@ -319,7 +319,9 @@ jobs: # wall time) is reduced here to the single comprehensive simulation — # see the doc comment on # run_chain_comprehensive_mixed_operations_with_epoch_change_and_quorum_rotation - # for what it covers. The remaining simulations run in their own phase + # for what it covers — plus the process_proposal collision module, + # which pins a consensus-halt regression and runs in under a second + # combined. The remaining simulations run in their own phase # below on push and nightly only, so a regression in one of them is # caught minutes after merge — the same safety-net pattern as the # shielded phase. Keeping this phase's filter identical across events @@ -352,7 +354,7 @@ jobs: --package keyword-search-contract \ --all-features \ --locked \ - -E 'not test(~shield) and (not binary_id(=drive-abci::strategy_tests) or test(~comprehensive_mixed_operations))' + -E 'not test(~shield) and (not binary_id(=drive-abci::strategy_tests) or test(~comprehensive_mixed_operations) or test(~process_proposal_collision))' env: RUST_MIN_STACK: 4194304 CARGO_PROFILE_DEV_DEBUG: "0" @@ -380,7 +382,7 @@ jobs: --package drive-abci \ --all-features \ --locked \ - -E 'binary_id(=drive-abci::strategy_tests) and not test(~comprehensive_mixed_operations) and not test(~shield)' + -E 'binary_id(=drive-abci::strategy_tests) and not test(~comprehensive_mixed_operations) and not test(~process_proposal_collision) and not test(~shield)' env: RUST_MIN_STACK: 4194304 CARGO_PROFILE_DEV_DEBUG: "0"