Merge pull request #5237 from stacks-network/chore/fix-multiple-miners

Fix multiple_miners* tests

Author: jferrant

stacks-signer/src/client/mod.rs

@@ -89,9 +89,6 @@ pub enum ClientError {
     /// Invalid response from the stacks node
     #[error("Invalid response from the stacks node: {0}")]
     InvalidResponse(String),
-    /// A successful sortition has not occurred yet
-    #[error("The Stacks chain has not processed any successful sortitions yet")]
-    NoSortitionOnChain,
     /// A successful sortition's info response should be parseable into a SortitionState
     #[error("A successful sortition's info response should be parseable into a SortitionState")]
     UnexpectedSortitionInfo,

stacks-signer/src/client/stacks_client.rs

@@ -32,7 +32,7 @@ use blockstack_lib::net::api::get_tenures_fork_info::{
 use blockstack_lib::net::api::getaccount::AccountEntryResponse;
 use blockstack_lib::net::api::getpoxinfo::RPCPoxInfoData;
 use blockstack_lib::net::api::getsortition::{SortitionInfo, RPC_SORTITION_INFO_PATH};
-use blockstack_lib::net::api::getstackers::{GetStackersErrors, GetStackersResponse};
+use blockstack_lib::net::api::getstackers::GetStackersResponse;
 use blockstack_lib::net::api::postblock::StacksBlockAcceptedData;
 use blockstack_lib::net::api::postblock_proposal::NakamotoBlockProposal;
 use blockstack_lib::net::api::postblock_v3;
@@ -80,7 +80,6 @@ pub struct StacksClient {
 
 #[derive(Deserialize)]
 struct GetStackersErrorResp {
-    err_type: String,
     err_msg: String,
 }
 
@@ -485,14 +484,11 @@ impl StacksClient {
                 warn!("Failed to parse the GetStackers error response: {e}");
                 backoff::Error::permanent(e.into())
             })?;
-            if error_data.err_type == GetStackersErrors::NOT_AVAILABLE_ERR_TYPE {
-                Err(backoff::Error::permanent(ClientError::NoSortitionOnChain))
-            } else {
-                warn!("Got error response ({status}): {}", error_data.err_msg);
-                Err(backoff::Error::permanent(ClientError::RequestFailure(
-                    status,
-                )))
-            }
+
+            warn!("Got error response ({status}): {}", error_data.err_msg);
+            Err(backoff::Error::permanent(ClientError::RequestFailure(
+                status,
+            )))
         };
         let stackers_response =
             retry_with_exponential_backoff::<_, ClientError, GetStackersResponse>(send_request)?;

stacks-signer/src/v0/signer.rs

@@ -512,7 +512,16 @@ impl Signer {
             .signer_db
             .block_lookup(self.reward_cycle, &signer_signature_hash)
         {
-            Ok(Some(block_info)) => block_info,
+            Ok(Some(block_info)) => {
+                if block_info.state == BlockState::GloballyRejected
+                    || block_info.state == BlockState::GloballyAccepted
+                {
+                    debug!("{self}: Received block validation for a block that is already marked as {}. Ignoring...", block_info.state);
+                    return None;
+                } else {
+                    block_info
+                }
+            }
             Ok(None) => {
                 // We have not seen this block before. Why are we getting a response for it?
                 debug!("{self}: Received a block validate response for a block we have not seen before. Ignoring...");

testnet/stacks-node/src/tests/epoch_25.rs

@@ -211,11 +211,12 @@ fn microblocks_disabled() {
     );
     assert_eq!(account.nonce, 1);
 
-    info!(
-        "Microblocks assembled: {}",
-        test_observer::get_microblocks().len()
+    let microblocks_assembled = test_observer::get_microblocks().len();
+    info!("Microblocks assembled: {microblocks_assembled}",);
+    assert!(
+        microblocks_assembled > 0,
+        "There should be at least 1 microblock assembled"
     );
-    assert_eq!(test_observer::get_microblocks().len(), 1);
 
     let miner_nonce_before_microblock_assembly = get_account(&http_origin, &miner_account).nonce;
 
@@ -244,8 +245,8 @@ fn microblocks_disabled() {
     );
     assert_eq!(account.nonce, 1);
 
-    // but we should have assembled and announced at least 1 to the observer
-    assert!(test_observer::get_microblocks().len() >= 2);
+    // but we should have assembled and announced at least 1 more block to the observer
+    assert!(test_observer::get_microblocks().len() > microblocks_assembled);
     info!(
         "Microblocks assembled: {}",
         test_observer::get_microblocks().len()

testnet/stacks-node/src/tests/nakamoto_integrations.rs

@@ -1155,69 +1155,6 @@ pub fn is_key_set_for_cycle(
     Ok(key.is_some())
 }
 
-fn signer_vote_if_needed(
-    btc_regtest_controller: &BitcoinRegtestController,
-    naka_conf: &Config,
-    signer_sks: &[StacksPrivateKey], // TODO: Is there some way to get this from the TestSigners?
-    signers: &TestSigners,
-) {
-    // When we reach the next prepare phase, submit new voting transactions
-    let block_height = btc_regtest_controller.get_headers_height();
-    let reward_cycle = btc_regtest_controller
-        .get_burnchain()
-        .block_height_to_reward_cycle(block_height)
-        .unwrap();
-    let prepare_phase_start = btc_regtest_controller
-        .get_burnchain()
-        .pox_constants
-        .prepare_phase_start(
-            btc_regtest_controller.get_burnchain().first_block_height,
-            reward_cycle,
-        );
-
-    if block_height >= prepare_phase_start {
-        // If the key is already set, do nothing.
-        if is_key_set_for_cycle(
-            reward_cycle + 1,
-            naka_conf.is_mainnet(),
-            &naka_conf.node.rpc_bind,
-        )
-        .unwrap_or(false)
-        {
-            return;
-        }
-
-        // If we are self-signing, then we need to vote on the aggregate public key
-        let http_origin = format!("http://{}", &naka_conf.node.rpc_bind);
-
-        // Get the aggregate key
-        let aggregate_key = signers.clone().generate_aggregate_key(reward_cycle + 1);
-        let aggregate_public_key = clarity::vm::Value::buff_from(aggregate_key)
-            .expect("Failed to serialize aggregate public key");
-
-        for (i, signer_sk) in signer_sks.iter().enumerate() {
-            let signer_nonce = get_account(&http_origin, &to_addr(signer_sk)).nonce;
-
-            // Vote on the aggregate public key
-            let voting_tx = tests::make_contract_call(
-                &signer_sk,
-                signer_nonce,
-                300,
-                &StacksAddress::burn_address(false),
-                SIGNERS_VOTING_NAME,
-                "vote-for-aggregate-public-key",
-                &[
-                    clarity::vm::Value::UInt(i as u128),
-                    aggregate_public_key.clone(),
-                    clarity::vm::Value::UInt(0),
-                    clarity::vm::Value::UInt(reward_cycle as u128 + 1),
-                ],
-            );
-            submit_tx(&http_origin, &voting_tx);
-        }
-    }
-}
-
 pub fn setup_epoch_3_reward_set(
     naka_conf: &Config,
     blocks_processed: &Arc<AtomicU64>,
@@ -1553,13 +1490,6 @@ fn simple_neon_integration() {
             &commits_submitted,
         )
         .unwrap();
-
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
     }
 
     // Submit a TX
@@ -1595,13 +1525,6 @@ fn simple_neon_integration() {
             &commits_submitted,
         )
         .unwrap();
-
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
     }
 
     // load the chain tip, and assert that it is a nakamoto block and at least 30 blocks have advanced in epoch 3
@@ -1805,13 +1728,6 @@ fn simple_neon_integration_with_flash_blocks_on_epoch_3() {
             &commits_submitted,
         )
         .unwrap();
-
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
     }
 
     // Submit a TX
@@ -1847,13 +1763,6 @@ fn simple_neon_integration_with_flash_blocks_on_epoch_3() {
             &commits_submitted,
         )
         .unwrap();
-
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
     }
 
     // load the chain tip, and assert that it is a nakamoto block and at least 30 blocks have advanced in epoch 3
@@ -2144,6 +2053,7 @@ fn multiple_miners() {
     let node_2_p2p = 51025;
     let http_origin = format!("http://{}", &naka_conf.node.rpc_bind);
     naka_conf.miner.wait_on_interim_blocks = Duration::from_secs(1);
+    naka_conf.node.pox_sync_sample_secs = 5;
     let sender_sk = Secp256k1PrivateKey::new();
     let sender_signer_sk = Secp256k1PrivateKey::new();
     let sender_signer_addr = tests::to_addr(&sender_signer_sk);
@@ -2579,13 +2489,6 @@ fn correct_burn_outs() {
         &naka_conf,
     );
 
-    signer_vote_if_needed(
-        &btc_regtest_controller,
-        &naka_conf,
-        &[sender_signer_sk],
-        &signers,
-    );
-
     run_until_burnchain_height(
         &mut btc_regtest_controller,
         &blocks_processed,
@@ -2645,13 +2548,6 @@ fn correct_burn_outs() {
             tip_sn.block_height > prior_tip,
             "The new burnchain tip must have been processed"
         );
-
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
     }
 
     coord_channel
@@ -4751,13 +4647,6 @@ fn forked_tenure_is_ignored() {
     })
     .unwrap();
 
-    signer_vote_if_needed(
-        &btc_regtest_controller,
-        &naka_conf,
-        &[sender_signer_sk],
-        &signers,
-    );
-
     info!("Commit op is submitted; unpause Tenure B's block");
 
     // Unpause the broadcast of Tenure B's block, do not submit commits, and do not allow blocks to
@@ -6198,13 +6087,6 @@ fn signer_chainstate() {
             make_stacks_transfer(&sender_sk, sender_nonce, send_fee, &recipient, send_amt);
         submit_tx(&http_origin, &transfer_tx);
 
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
-
         let timer = Instant::now();
         while proposals_submitted.load(Ordering::SeqCst) <= before {
             thread::sleep(Duration::from_millis(5));
@@ -6681,13 +6563,6 @@ fn continue_tenure_extend() {
     )
     .unwrap();
 
-    signer_vote_if_needed(
-        &btc_regtest_controller,
-        &naka_conf,
-        &[sender_signer_sk],
-        &signers,
-    );
-
     wait_for(5, || {
         let blocks_processed = coord_channel
             .lock()
@@ -6707,13 +6582,6 @@ fn continue_tenure_extend() {
 
     next_block_and(&mut btc_regtest_controller, 60, || Ok(true)).unwrap();
 
-    signer_vote_if_needed(
-        &btc_regtest_controller,
-        &naka_conf,
-        &[sender_signer_sk],
-        &signers,
-    );
-
     wait_for(5, || {
         let blocks_processed = coord_channel
             .lock()
@@ -6755,13 +6623,6 @@ fn continue_tenure_extend() {
     next_block_and_process_new_stacks_block(&mut btc_regtest_controller, 60, &coord_channel)
         .unwrap();
 
-    signer_vote_if_needed(
-        &btc_regtest_controller,
-        &naka_conf,
-        &[sender_signer_sk],
-        &signers,
-    );
-
     wait_for(5, || {
         let blocks_processed = coord_channel
             .lock()
@@ -6778,13 +6639,6 @@ fn continue_tenure_extend() {
 
     next_block_and(&mut btc_regtest_controller, 60, || Ok(true)).unwrap();
 
-    signer_vote_if_needed(
-        &btc_regtest_controller,
-        &naka_conf,
-        &[sender_signer_sk],
-        &signers,
-    );
-
     wait_for(5, || {
         let blocks_processed = coord_channel
             .lock()
@@ -6810,13 +6664,6 @@ fn continue_tenure_extend() {
         })
         .unwrap();
 
-        signer_vote_if_needed(
-            &btc_regtest_controller,
-            &naka_conf,
-            &[sender_signer_sk],
-            &signers,
-        );
-
         wait_for(5, || {
             let blocks_processed = coord_channel
                 .lock()