Do not count received valid signatures towards threshold weight when ignore flag set

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs

@@ -886,11 +886,6 @@ impl SignCoordinator {
                             );
                             continue;
                         }
-                        if !gathered_signatures.contains_key(&slot_id) {
-                            total_weight_signed = total_weight_signed
-                                .checked_add(signer_entry.weight)
-                                .expect("FATAL: total weight signed exceeds u32::MAX");
-                        }
 
                         if Self::fault_injection_ignore_signatures() {
                             warn!("SignCoordinator: fault injection: ignoring well-formed signature for block";
@@ -906,6 +901,12 @@ impl SignCoordinator {
                             continue;
                         }
 
+                        if !gathered_signatures.contains_key(&slot_id) {
+                            total_weight_signed = total_weight_signed
+                                .checked_add(signer_entry.weight)
+                                .expect("FATAL: total weight signed exceeds u32::MAX");
+                        }
+
                         info!("SignCoordinator: Signature Added to block";
                             "block_signer_sighash" => %block_sighash,
                             "signer_pubkey" => signer_pubkey.to_hex(),
@@ -986,7 +987,6 @@ impl SignCoordinator {
                     }
                 };
             }
-
             // After gathering all signatures, return them if we've hit the threshold
             if total_weight_signed >= self.weight_threshold {
                 info!("SignCoordinator: Received enough signatures. Continuing.";

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

@@ -2227,53 +2227,56 @@ fn signers_broadcast_signed_blocks() {
     let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
 
     signer_test.boot_to_epoch_3();
-    sleep_ms(10_000);
-
+    let info_before = get_chain_info(&signer_test.running_nodes.conf);
+    let blocks_before = signer_test
+        .running_nodes
+        .nakamoto_blocks_mined
+        .load(Ordering::SeqCst);
     signer_test.mine_nakamoto_block(Duration::from_secs(30));
-    sleep_ms(10_000);
 
-    TEST_IGNORE_SIGNERS.lock().unwrap().replace(true);
+    wait_for(30, || {
+        let blocks_mined = signer_test
+            .running_nodes
+            .nakamoto_blocks_mined
+            .load(Ordering::SeqCst);
+        let info = get_chain_info(&signer_test.running_nodes.conf);
+        debug!(
+            "blocks_mined: {},{}, stacks_tip_height: {},{}",
+            blocks_mined, blocks_before, info.stacks_tip_height, info_before.stacks_tip_height
+        );
+        Ok(blocks_mined > blocks_before && info.stacks_tip_height > info_before.stacks_tip_height)
+    })
+    .expect("Timed out waiting for first nakamoto block to be mined");
 
+    TEST_IGNORE_SIGNERS.lock().unwrap().replace(true);
     let blocks_before = signer_test
         .running_nodes
         .nakamoto_blocks_mined
         .load(Ordering::SeqCst);
-
     let signer_pushed_before = signer_test
         .running_nodes
         .nakamoto_blocks_signer_pushed
         .load(Ordering::SeqCst);
-
     let info_before = get_chain_info(&signer_test.running_nodes.conf);
 
-    // submit a tx so that the miner will mine a block
+    // submit a tx so that the miner will mine a blockn
     let sender_nonce = 0;
     let transfer_tx =
         make_stacks_transfer(&sender_sk, sender_nonce, send_fee, &recipient, send_amt);
     submit_tx(&http_origin, &transfer_tx);
 
     debug!("Transaction sent; waiting for block-mining");
 
-    let start = Instant::now();
-    let duration = 60;
-    loop {
-        let blocks_mined = signer_test
-            .running_nodes
-            .nakamoto_blocks_mined
-            .load(Ordering::SeqCst);
+    wait_for(30, || {
         let signer_pushed = signer_test
             .running_nodes
             .nakamoto_blocks_signer_pushed
             .load(Ordering::SeqCst);
-
+        let blocks_mined = signer_test
+            .running_nodes
+            .nakamoto_blocks_mined
+            .load(Ordering::SeqCst);
         let info = get_chain_info(&signer_test.running_nodes.conf);
-        if blocks_mined > blocks_before
-            && signer_pushed > signer_pushed_before
-            && info.stacks_tip_height > info_before.stacks_tip_height
-        {
-            break;
-        }
-
         debug!(
             "blocks_mined: {},{}, signers_pushed: {},{}, stacks_tip_height: {},{}",
             blocks_mined,
@@ -2283,12 +2286,11 @@ fn signers_broadcast_signed_blocks() {
             info.stacks_tip_height,
             info_before.stacks_tip_height
         );
-
-        std::thread::sleep(Duration::from_millis(100));
-        if start.elapsed() >= Duration::from_secs(duration) {
-            panic!("Timed out");
-        }
-    }
+        Ok(blocks_mined > blocks_before
+            && info.stacks_tip_height > info_before.stacks_tip_height
+            && signer_pushed > signer_pushed_before)
+    })
+    .expect("Timed out waiting for second nakamoto block to be mined");
 
     signer_test.shutdown();
 }
@@ -4754,7 +4756,7 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
 
     info!("Allowing miner to accept block responses again. ");
     TEST_IGNORE_SIGNERS.lock().unwrap().replace(false);
-    info!("Allowing singers to broadcast block N+1 to the miner");
+    info!("Allowing signers to broadcast block N+1 to the miner");
     TEST_PAUSE_BLOCK_BROADCAST.lock().unwrap().replace(false);
 
     // Assert the N+1' block was rejected