Merge remote-tracking branch 'origin/develop' into feat/naka-miner-heuristics

Author: kantai

stacks-signer/src/chainstate.rs

@@ -187,6 +187,7 @@ impl SortitionsView {
         block: &NakamotoBlock,
         block_pk: &StacksPublicKey,
         reward_cycle: u64,
+        reset_view_if_wrong_consensus_hash: bool,
     ) -> Result<bool, SignerChainstateError> {
         if self
             .cur_sortition
@@ -236,6 +237,23 @@ impl SortitionsView {
                 })
             })
         else {
+            if reset_view_if_wrong_consensus_hash {
+                info!(
+                    "Miner block proposal has consensus hash that is neither the current or last sortition. Resetting view.";
+                    "proposed_block_consensus_hash" => %block.header.consensus_hash,
+                    "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
+                    "last_sortition_consensus_hash" => ?self.last_sortition.as_ref().map(|x| x.consensus_hash),
+                );
+                self.reset_view(client)?;
+                return self.check_proposal(
+                    client,
+                    signer_db,
+                    block,
+                    block_pk,
+                    reward_cycle,
+                    false,
+                );
+            }
             warn!(
                 "Miner block proposal has consensus hash that is neither the current or last sortition. Considering invalid.";
                 "proposed_block_consensus_hash" => %block.header.consensus_hash,
@@ -624,4 +642,23 @@ impl SortitionsView {
             config,
         })
     }
+
+    /// Reset the view to the current sortition and last sortition
+    pub fn reset_view(&mut self, client: &StacksClient) -> Result<(), ClientError> {
+        let CurrentAndLastSortition {
+            current_sortition,
+            last_sortition,
+        } = client.get_current_and_last_sortition()?;
+
+        let cur_sortition = SortitionState::try_from(current_sortition)?;
+        let last_sortition = last_sortition
+            .map(SortitionState::try_from)
+            .transpose()
+            .ok()
+            .flatten();
+
+        self.cur_sortition = cur_sortition;
+        self.last_sortition = last_sortition;
+        Ok(())
+    }
 }

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)?;
@@ -578,7 +574,9 @@ impl StacksClient {
                     return block_push_result;
                 }
                 Err(e) => {
-                    if cfg!(test) && start_time.elapsed() > Duration::from_secs(30) {
+                    if cfg!(any(test, feature = "testing"))
+                        && start_time.elapsed() > Duration::from_secs(30)
+                    {
                         panic!(
                             "{log_fmt}: Timed out in test while pushing block to stacks node: {e}"
                         );

stacks-signer/src/tests/chainstate.rs

@@ -26,6 +26,7 @@ use blockstack_lib::chainstate::stacks::{
     TransactionSpendingCondition, TransactionVersion,
 };
 use blockstack_lib::net::api::get_tenures_fork_info::TenureForkingInfo;
+use blockstack_lib::net::api::getsortition::SortitionInfo;
 use clarity::types::chainstate::{BurnchainHeaderHash, SortitionId};
 use clarity::util::vrf::VRFProof;
 use libsigner::BlockProposal;
@@ -128,13 +129,13 @@ fn check_proposal_units() {
         setup_test_environment("check_proposal_units");
 
     assert!(!view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 
     view.last_sortition = None;
 
     assert!(!view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 }
 
@@ -150,7 +151,8 @@ fn check_proposal_miner_pkh_mismatch() {
             &mut signer_db,
             &block,
             &different_block_pk,
-            1
+            1,
+            false,
         )
         .unwrap());
 
@@ -161,7 +163,8 @@ fn check_proposal_miner_pkh_mismatch() {
             &mut signer_db,
             &block,
             &different_block_pk,
-            1
+            1,
+            false,
         )
         .unwrap());
 }
@@ -257,7 +260,7 @@ fn reorg_timing_testing(
         config,
     } = MockServerClient::new();
     let h = std::thread::spawn(move || {
-        view.check_proposal(&client, &mut signer_db, &block, &block_pk, 1)
+        view.check_proposal(&client, &mut signer_db, &block, &block_pk, 1, false)
     });
     header_clone.chain_length -= 1;
     let response = crate::client::tests::build_get_tenure_tip_response(
@@ -294,16 +297,16 @@ fn check_proposal_invalid_status() {
         setup_test_environment("invalid_status");
     block.header.consensus_hash = view.cur_sortition.consensus_hash;
     assert!(view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
     view.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedAfterFirstBlock;
     assert!(!view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 
     block.header.consensus_hash = view.last_sortition.as_ref().unwrap().consensus_hash;
     assert!(!view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 
     view.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
@@ -314,7 +317,7 @@ fn check_proposal_invalid_status() {
     // parent blocks have been seen before, while the signer state checks are only reasoning about
     // stacks blocks seen by the signer, which may be a subset)
     assert!(view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 }
 
@@ -363,7 +366,7 @@ fn check_proposal_tenure_extend_invalid_conditions() {
     let tx = make_tenure_change_tx(extend_payload);
     block.txs = vec![tx];
     assert!(!view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 
     let mut extend_payload = make_tenure_change_payload();
@@ -373,7 +376,7 @@ fn check_proposal_tenure_extend_invalid_conditions() {
     let tx = make_tenure_change_tx(extend_payload);
     block.txs = vec![tx];
     assert!(view
-        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1)
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
         .unwrap());
 }
 
@@ -400,7 +403,8 @@ fn check_block_proposal_timeout() {
             &mut signer_db,
             &curr_sortition_block,
             &block_pk,
-            1
+            1,
+            false,
         )
         .unwrap());
 
@@ -410,7 +414,8 @@ fn check_block_proposal_timeout() {
             &mut signer_db,
             &last_sortition_block,
             &block_pk,
-            1
+            1,
+            false,
         )
         .unwrap());
 
@@ -422,7 +427,8 @@ fn check_block_proposal_timeout() {
             &mut signer_db,
             &curr_sortition_block,
             &block_pk,
-            1
+            1,
+            false,
         )
         .unwrap());
 
@@ -432,7 +438,8 @@ fn check_block_proposal_timeout() {
             &mut signer_db,
             &last_sortition_block,
             &block_pk,
-            1
+            1,
+            false,
         )
         .unwrap());
 }
@@ -513,3 +520,64 @@ fn check_sortition_timeout() {
         .is_timed_out(Duration::from_secs(1), &signer_db)
         .unwrap());
 }
+
+/// Test that the sortition info is refreshed once
+/// when `check_proposal` is called with a sortition view
+/// that doesn't match the block proposal
+#[test]
+fn check_proposal_refresh() {
+    let (stacks_client, mut signer_db, block_pk, mut view, mut block) =
+        setup_test_environment("check_proposal_refresh");
+    block.header.consensus_hash = view.cur_sortition.consensus_hash;
+    assert!(view
+        .check_proposal(&stacks_client, &mut signer_db, &block, &block_pk, 1, false)
+        .unwrap());
+
+    let MockServerClient {
+        server,
+        client,
+        config: _,
+    } = MockServerClient::new();
+
+    let last_sortition = view.last_sortition.as_ref().unwrap();
+
+    let expected_result = vec![
+        SortitionInfo {
+            burn_block_hash: last_sortition.burn_block_hash,
+            burn_block_height: 2,
+            sortition_id: SortitionId([2; 32]),
+            parent_sortition_id: SortitionId([1; 32]),
+            consensus_hash: block.header.consensus_hash,
+            was_sortition: true,
+            burn_header_timestamp: 2,
+            miner_pk_hash160: Some(view.cur_sortition.miner_pkh),
+            stacks_parent_ch: Some(view.cur_sortition.parent_tenure_id),
+            last_sortition_ch: Some(view.cur_sortition.parent_tenure_id),
+            committed_block_hash: None,
+        },
+        SortitionInfo {
+            burn_block_hash: BurnchainHeaderHash([128; 32]),
+            burn_block_height: 1,
+            sortition_id: SortitionId([1; 32]),
+            parent_sortition_id: SortitionId([0; 32]),
+            consensus_hash: view.cur_sortition.parent_tenure_id,
+            was_sortition: true,
+            burn_header_timestamp: 1,
+            miner_pk_hash160: Some(view.cur_sortition.miner_pkh),
+            stacks_parent_ch: Some(view.cur_sortition.parent_tenure_id),
+            last_sortition_ch: Some(view.cur_sortition.parent_tenure_id),
+            committed_block_hash: None,
+        },
+    ];
+
+    view.cur_sortition.consensus_hash = ConsensusHash([128; 20]);
+    let h = std::thread::spawn(move || {
+        view.check_proposal(&client, &mut signer_db, &block, &block_pk, 1, true)
+    });
+    crate::client::tests::write_response(
+        server,
+        format!("HTTP/1.1 200 Ok\n\n{}", serde_json::json!(expected_result)).as_bytes(),
+    );
+    let result = h.join().unwrap();
+    assert!(result.unwrap());
+}

stacks-signer/src/v0/signer.rs

@@ -379,6 +379,7 @@ impl Signer {
                 &block_proposal.block,
                 miner_pubkey,
                 self.reward_cycle,
+                true,
             ) {
                 // Error validating block
                 Err(e) => {
@@ -512,7 +513,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()