Add block_proposal_after_pushed_is_rejected to prove signer behaviour

stacks-node/src/tests/signer/commands/block_wait.rs

@@ -25,7 +25,7 @@ use stacks::chainstate::stacks::{TenureChangeCause, TenureChangePayload, Transac
 use super::context::{SignerTestContext, SignerTestState};
 use crate::tests::neon_integrations::get_chain_info;
 use crate::tests::signer::v0::{
-    wait_for_block_global_rejection_with_reject_reason, wait_for_block_proposal,
+    wait_for_block_global_rejection_with_reject_reason, wait_for_block_proposal_block,
     wait_for_block_pushed_by_miner_key,
 };
 
@@ -260,7 +260,7 @@ impl Command<SignerTestState, SignerTestContext> for ChainExpectNakaBlockProposa
 
         info!("Waiting for block proposal at height {expected_height}");
 
-        let proposed_block = wait_for_block_proposal(30, expected_height, &miner_pk)
+        let proposed_block = wait_for_block_proposal_block(30, expected_height, &miner_pk)
             .expect("Timed out waiting for block proposal");
 
         let block_hash = proposed_block.header.signer_signature_hash();

stacks-node/src/tests/signer/v0/mod.rs

@@ -467,27 +467,34 @@ impl SignerTest<SpawnedSigner> {
 
     /// Propose a block to the signers
     fn propose_block(&self, block: NakamotoBlock, timeout: Duration) {
-        let miners_contract_id = boot_code_id(MINERS_NAME, false);
-        let mut session = StackerDBSession::new(
-            &self.running_nodes.conf.node.rpc_bind,
-            miners_contract_id,
-            self.running_nodes.conf.miner.stackerdb_timeout,
-        );
         let burn_height = self
             .running_nodes
             .btc_regtest_controller
             .get_headers_height();
         let reward_cycle = self.get_current_reward_cycle();
-        let signer_signature_hash = block.header.signer_signature_hash();
-        let signed_by = block.header.recover_miner_pk().expect(
-            "FATAL: signer tests should only propose blocks that have been signed by the signer test miner. Otherwise, signers won't even consider them via this channel."
-        );
-        let message = SignerMessage::BlockProposal(BlockProposal {
+        let block_proposal = BlockProposal {
             block,
             burn_height,
             reward_cycle,
             block_proposal_data: BlockProposalData::empty(),
-        });
+        };
+        self.send_block_proposal(block_proposal, timeout);
+    }
+
+    /// Send a block proposal to the signers
+    fn send_block_proposal(&self, block_proposal: BlockProposal, timeout: Duration) {
+        let miners_contract_id = boot_code_id(MINERS_NAME, false);
+        let mut session = StackerDBSession::new(
+            &self.running_nodes.conf.node.rpc_bind,
+            miners_contract_id,
+            self.running_nodes.conf.miner.stackerdb_timeout,
+        );
+        let signer_signature_hash: Sha512Trunc256Sum =
+            block_proposal.block.header.signer_signature_hash();
+        let signed_by = block_proposal.block.header.recover_miner_pk().expect(
+            "FATAL: signer tests should only propose blocks that have been signed by the signer test miner. Otherwise, signers won't even consider them via this channel."
+        );
+        let message = SignerMessage::BlockProposal(block_proposal);
         let miner_sk = self
             .running_nodes
             .conf
@@ -1210,12 +1217,23 @@ pub fn verify_sortition_winner(sortdb: &SortitionDB, miner_pkh: &Hash160) {
 }
 
 /// Waits for a block proposal to be observed in the test_observer stackerdb chunks at the expected height
-/// and signed by the expected miner
-pub fn wait_for_block_proposal(
+/// and signed by the expected miner. Returns the proposed NakamotoBlock.
+pub fn wait_for_block_proposal_block(
     timeout_secs: u64,
     expected_height: u64,
     expected_miner: &StacksPublicKey,
 ) -> Result<NakamotoBlock, String> {
+    wait_for_block_proposal(timeout_secs, expected_height, expected_miner)
+        .and_then(|proposal| Ok(proposal.block))
+}
+
+/// Waits for a block proposal to be observed in the test_observer stackerdb chunks at the expected height
+/// and signed by the expected miner. Returns the BlockProposal.
+pub fn wait_for_block_proposal(
+    timeout_secs: u64,
+    expected_height: u64,
+    expected_miner: &StacksPublicKey,
+) -> Result<BlockProposal, String> {
     let mut proposed_block = None;
     wait_for(timeout_secs, || {
         let chunks = test_observer::get_stackerdb_chunks();
@@ -1233,7 +1251,7 @@ pub fn wait_for_block_proposal(
                 continue;
             }
             if &miner_pk == expected_miner {
-                proposed_block = Some(proposal.block);
+                proposed_block = Some(proposal);
                 return Ok(true);
             }
         }
@@ -1686,6 +1704,162 @@ pub fn wait_for_state_machine_update_by_miner_tenure_id(
     })
 }
 
+#[tag(bitcoind)]
+#[test]
+#[ignore]
+/// Test that a signer that receives a block proposal for a block that they have a block pushed event
+/// for is rejected.
+///
+/// Test Setup:
+/// The test spins up five stacks signers, one miner Nakamoto node, and a corresponding bitcoind.
+///
+/// Test Execution:
+/// - Miner proposes a block N to all the signers
+/// - Signer 1 is set to ignore any incoming proposals (simulate it not receiving the proposal)
+/// - Signers 2-5 approve the block proposal
+/// - The chain advances to block N
+/// - Signer 1 is allowed to consider proposals again
+/// - Block N is reproposed to all Signers
+/// - Signer 1 rejects the proposal
+///
+/// Test Assertion:
+/// All signers but Signer 1 accept the proposal
+/// Signer 1 rejects the late proposal for block N with InvalidParentBlock reason
+fn block_proposal_after_pushed_is_rejected() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    tracing_subscriber::registry()
+        .with(fmt::layer())
+        .with(EnvFilter::from_default_env())
+        .init();
+
+    info!("------------------------- Test Setup -------------------------");
+    let num_signers = 5;
+    let sender_sk = Secp256k1PrivateKey::random();
+    let sender_addr = tests::to_addr(&sender_sk);
+    let send_amt = 100;
+    let send_fee = 180;
+    let recipient = PrincipalData::from(StacksAddress::burn_address(false));
+    let signer_test: SignerTest<SpawnedSigner> =
+        SignerTest::new(num_signers, vec![(sender_addr, send_amt + send_fee)]);
+    let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
+    signer_test.boot_to_epoch_3();
+
+    let miner_sk = signer_test
+        .running_nodes
+        .conf
+        .miner
+        .mining_key
+        .clone()
+        .unwrap();
+    let miner_pk = StacksPublicKey::from_private(&miner_sk);
+    let all_signers = signer_test.signer_test_pks();
+    let signer_1 = all_signers[0].clone();
+    info!("------------------------- Ignore all Proposals for Signer 1 -------------------------"; "signer_public_key" => ?signer_1);
+    test_observer::clear();
+    TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(vec![signer_1.clone()]);
+    info!("------------------------- Force Miner to Send a Block Proposal To Signers -------------------------");
+    let info_before = get_chain_info(&signer_test.running_nodes.conf);
+    // submit a tx to force a block proposal
+    let sender_nonce = 0;
+    let transfer_tx = make_stacks_transfer_serialized(
+        &sender_sk,
+        sender_nonce,
+        send_fee,
+        signer_test.running_nodes.conf.burnchain.chain_id,
+        &recipient,
+        send_amt,
+    );
+    submit_tx(&http_origin, &transfer_tx);
+    // Grab the proposal itself so it can be reproposed later
+    let block_n_proposal =
+        wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk)
+            .expect("Timed out waiting for block N+1 to be proposed");
+    let signer_signature_hash = block_n_proposal.block.header.signer_signature_hash();
+    let _ = wait_for_block_pushed(30, &signer_signature_hash)
+        .expect("Failed to get BlockPushed for block N");
+    info!("------------------------- Advance Chain to Include Block N -------------------------");
+    // Shouldn't have to wait long for the chain to advance
+    wait_for(10, || {
+        let info_after = get_chain_info(&signer_test.running_nodes.conf);
+        Ok(info_after.stacks_tip_height >= info_before.stacks_tip_height + 1)
+    })
+    .expect("Chain did not advance to block N+1");
+
+    info!("------------------------- Verify Signer 1 did NOT respond to the Block Proposal -------------------------");
+    let chunks = test_observer::get_stackerdb_chunks();
+    for chunk in chunks.into_iter().flat_map(|chunk| chunk.modified_slots) {
+        let Ok(message) = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) else {
+            continue;
+        };
+        match message {
+            SignerMessage::BlockResponse(BlockResponse::Rejected(rejected)) => {
+                if rejected.signer_signature_hash == signer_signature_hash {
+                    if rejected.signer_signature_hash == signer_signature_hash {
+                        if rejected
+                            .verify(&signer_1)
+                            .expect("Failed to verify signature")
+                        {
+                            panic!("Signer 1 rejected the re-proposed block when it should have ignored it");
+                        }
+                    }
+                }
+            }
+            SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) => {
+                if accepted.signer_signature_hash == signer_signature_hash {
+                    if signer_1
+                        .verify(
+                            accepted.signer_signature_hash.as_bytes(),
+                            &accepted.signature,
+                        )
+                        .expect("Failed to verify signature")
+                    {
+                        panic!(
+                            "Signer 1 accepted the block proposal when it should have ignored it"
+                        );
+                    }
+                }
+            }
+            _ => continue,
+        }
+    }
+    info!(
+        "------------------------- Allow Signer 1 to Consider Proposals -------------------------"
+    );
+    TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(vec![]);
+    info!("------------------------- Re-Propose Block N to the Signers -------------------------");
+    test_observer::clear();
+    signer_test.send_block_proposal(block_n_proposal, Duration::from_secs(30));
+    info!(
+        "------------------------- Verify Signer 1 Rejected the Proposal -------------------------"
+    );
+    wait_for(30, || {
+        let chunks: Vec<_> = test_observer::get_stackerdb_chunks()
+            .into_iter()
+            .flat_map(|chunk| chunk.modified_slots)
+            .collect();
+        for chunk in chunks {
+            let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice())
+                .expect("Failed to deserialize SignerMessage");
+
+            let SignerMessage::BlockResponse(BlockResponse::Rejected(rejected)) = message else {
+                continue;
+            };
+            if rejected.signer_signature_hash == signer_signature_hash {
+                return Ok(rejected
+                    .verify(&signer_1)
+                    .expect("Failed to verify signature")
+                    && rejected.reason == "The block does not confirm the expected parent block.");
+            }
+        }
+        Ok(false)
+    })
+    .expect("Timed out waiting for Signer 1 to reject the re-proposed block");
+    signer_test.shutdown();
+}
+
 #[tag(bitcoind)]
 #[test]
 #[ignore]
@@ -4094,7 +4268,7 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
     let tx = submit_tx(&http_origin, &transfer_tx);
 
     info!("Submitted tx {tx} in to attempt to mine block N+1");
-    let block_n_1 = wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk)
+    let block_n_1 = wait_for_block_proposal_block(30, info_before.stacks_tip_height + 1, &miner_pk)
         .expect("Timed out waiting for block N+1 to be proposed");
     let all_signers = signer_test.signer_test_pks();
     wait_for_block_global_acceptance_from_signers(
@@ -4133,8 +4307,9 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
         "------------------------- Attempt to Mine Nakamoto Block N+1' -------------------------"
     );
     // Wait for the miner to propose a new invalid block N+1'
-    let block_n_1_prime = wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk)
-        .expect("Timed out waiting for block N+1' to be proposed");
+    let block_n_1_prime =
+        wait_for_block_proposal_block(30, info_before.stacks_tip_height + 1, &miner_pk)
+            .expect("Timed out waiting for block N+1' to be proposed");
     assert_ne!(
         block_n_1_prime.header.signer_signature_hash(),
         block_n_1.header.signer_signature_hash()
@@ -4776,7 +4951,7 @@ fn block_validation_check_rejection_timeout_heuristic() {
         )
         .unwrap();
 
-        let proposal = wait_for_block_proposal(30, height_before + 1, &miner_pk)
+        let proposal = wait_for_block_proposal_block(30, height_before + 1, &miner_pk)
             .expect("Timed out waiting for block proposal");
 
         wait_for_block_rejections_from_signers(
@@ -5940,7 +6115,7 @@ fn block_proposal_timeout() {
     TEST_BROADCAST_PROPOSAL_STALL.set(vec![]);
 
     let block_proposal_n =
-        wait_for_block_proposal(30, chain_before.stacks_tip_height + 1, &miner_pk)
+        wait_for_block_proposal_block(30, chain_before.stacks_tip_height + 1, &miner_pk)
             .expect("Failed to get block proposal N");
     wait_for_block_global_rejection(
         30,
@@ -6281,7 +6456,7 @@ fn signer_can_accept_rejected_block() {
     submit_tx(&http_origin, &transfer_tx);
 
     info!("Submitted transfer tx and waiting for block proposal");
-    let block = wait_for_block_proposal(30, block_height_before + 1, &miner_pk)
+    let block = wait_for_block_proposal_block(30, block_height_before + 1, &miner_pk)
         .expect("Timed out waiting for block proposal");
     let expected_block_height = block.header.chain_length;
 
@@ -7115,7 +7290,7 @@ fn verify_mempool_caches() {
     submit_tx(&http_origin, &transfer_tx);
 
     info!("Submitted transfer tx and waiting for block proposal");
-    let block = wait_for_block_proposal(30, block_height_before + 1, &miner_pk)
+    let block = wait_for_block_proposal_block(30, block_height_before + 1, &miner_pk)
         .expect("Timed out waiting for block proposal");
 
     // Stall the miners so that this block is not re-proposed after being rejected
@@ -7930,7 +8105,7 @@ fn signers_do_not_commit_unless_threshold_precommitted() {
     )
     .unwrap();
 
-    let proposal = wait_for_block_proposal(30, height_before + 1, &miner_pk)
+    let proposal = wait_for_block_proposal_block(30, height_before + 1, &miner_pk)
         .expect("Timed out waiting for block proposal");
     let hash = proposal.header.signer_signature_hash();
     wait_for_block_pre_commits_from_signers(30, &hash, &pre_commit_signers)
@@ -8004,8 +8179,9 @@ fn signers_treat_signatures_as_precommits() {
     );
     signer_test.mine_bitcoin_block();
 
-    let block_proposal = wait_for_block_proposal(30, peer_info.stacks_tip_height + 1, &miner_pk)
-        .expect("Failed to propose a new tenure block");
+    let block_proposal =
+        wait_for_block_proposal_block(30, peer_info.stacks_tip_height + 1, &miner_pk)
+            .expect("Failed to propose a new tenure block");
 
     info!(
         "------------------------- Verify Only Operating Signer Issues Pre-Commit -------------------------"