Merge branch 'develop' into fix/5285

jcnelson · web-flow · commit 086578792af0 · 2024-10-24T20:37:21.000Z
diff --git a/clarity/src/vm/docs/mod.rs b/clarity/src/vm/docs/mod.rs
@@ -1775,17 +1775,17 @@ this value is less than or equal to the value for `miner-spend-total` at the sam
 const GET_BURN_BLOCK_INFO_API: SpecialAPI = SpecialAPI {
     input_type: "BurnBlockInfoPropertyName, uint",
     output_type: "(optional buff) | (optional (tuple (addrs (list 2 (tuple (hashbytes (buff 32)) (version (buff 1))))) (payout uint)))",
-    snippet: "get-burn-block-info? ${1:prop} ${2:block-height}",
-    signature: "(get-burn-block-info? prop-name block-height)",
+    snippet: "get-burn-block-info? ${1:prop} ${2:burn-block-height}",
+    signature: "(get-burn-block-info? prop-name burn-block-height)",
     description: "The `get-burn-block-info?` function fetches data for a block of the given *burnchain* block height. The
-value and type returned are determined by the specified `BlockInfoPropertyName`.  Valid values for `block-height` only
+value and type returned are determined by the specified `BlockInfoPropertyName`.  Valid values for `burn-block-height` only
 include heights between the burnchain height at the time the Stacks chain was launched, and the last-processed burnchain
-block.  If the `block-height` argument falls outside of this range, then `none` shall be returned.
+block.  If the `burn-block-height` argument falls outside of this range, then `none` shall be returned.
 
 The following `BlockInfoPropertyName` values are defined:
 
 * The `header-hash` property returns a 32-byte buffer representing the header hash of the burnchain block at
-burnchain height `block-height`.
+burnchain height `burn-block-height`.
 
 * The `pox-addrs` property returns a tuple with two items: a list of up to two PoX addresses that received a PoX payout at that block height, and the amount of burnchain
 tokens paid to each address (note that per the blockchain consensus rules, each PoX payout will be the same for each address in the block-commit transaction).
@@ -1811,11 +1811,11 @@ The `addrs` list contains the same PoX address values passed into the PoX smart
 
 const GET_STACKS_BLOCK_INFO_API: SpecialAPI = SpecialAPI {
     input_type: "StacksBlockInfoPropertyName, uint",
-    snippet: "get-stacks-block-info? ${1:prop} ${2:block-height}",
+    snippet: "get-stacks-block-info? ${1:prop} ${2:stacks-block-height}",
     output_type: "(optional buff) | (optional uint)",
-    signature: "(get-stacks-block-info? prop-name block-height)",
+    signature: "(get-stacks-block-info? prop-name stacks-block-height)",
     description: "The `get-stacks-block-info?` function fetches data for a block of the given *Stacks* block height. The
-value and type returned are determined by the specified `StacksBlockInfoPropertyName`. If the provided `block-height` does
+value and type returned are determined by the specified `StacksBlockInfoPropertyName`. If the provided `stacks-block-height` does
 not correspond to an existing block prior to the current block, the function returns `none`. The currently available property names
 are as follows:
 
@@ -1840,11 +1840,11 @@ the mining of this block started, but is not guaranteed to be accurate. This tim
 
 const GET_TENURE_INFO_API: SpecialAPI = SpecialAPI {
     input_type: "TenureInfoPropertyName, uint",
-    snippet: "get-tenure-info? ${1:prop} ${2:block-height}",
+    snippet: "get-tenure-info? ${1:prop} ${2:stacks-block-height}",
     output_type: "(optional buff) | (optional uint)",
-    signature: "(get-tenure-info? prop-name block-height)",
+    signature: "(get-tenure-info? prop-name stacks-block-height)",
     description: "The `get-tenure-info?` function fetches data for the tenure at the given block height. The
-value and type returned are determined by the specified `TenureInfoPropertyName`. If the provided `block-height` does
+value and type returned are determined by the specified `TenureInfoPropertyName`. If the provided `stacks-block-height` does
 not correspond to an existing block prior to the current block, the function returns `none`. The currently available property names
 are as follows:
 
diff --git a/stacks-signer/src/lib.rs b/stacks-signer/src/lib.rs
@@ -121,7 +121,7 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SpawnedSigner
             as this could potentially expose sensitive data or functionalities to security risks \
             if additional proper security checks are not integrated in place. \
             For more information, check the documentation at \
-            https://docs.stacks.co/nakamoto-upgrade/signing-and-stacking/faq#what-should-the-networking-setup-for-my-signer-look-like."
+            https://docs.stacks.co/guides-and-tutorials/running-a-signer#preflight-setup"
         );
         let (res_send, res_recv) = channel();
         let ev = SignerEventReceiver::new(config.network.is_mainnet());
diff --git a/stackslib/src/net/relay.rs b/stackslib/src/net/relay.rs
@@ -2504,14 +2504,27 @@ impl Relayer {
                     for chunk in sync_result.chunks_to_store.into_iter() {
                         let md = chunk.get_slot_metadata();
                         if let Err(e) = tx.try_replace_chunk(&sc, &md, &chunk.data) {
-                            warn!(
-                                "Failed to store chunk for StackerDB";
-                                "stackerdb_contract_id" => &format!("{}", &sync_result.contract_id),
-                                "slot_id" => md.slot_id,
-                                "slot_version" => md.slot_version,
-                                "num_bytes" => chunk.data.len(),
-                                "error" => %e
-                            );
+                            if matches!(e, Error::StaleChunk { .. }) {
+                                // This is a common and expected message, so log it as a debug and with a sep message
+                                // to distinguish it from other message types.
+                                debug!(
+                                    "Dropping stale StackerDB chunk";
+                                    "stackerdb_contract_id" => &format!("{}", &sync_result.contract_id),
+                                    "slot_id" => md.slot_id,
+                                    "slot_version" => md.slot_version,
+                                    "num_bytes" => chunk.data.len(),
+                                    "error" => %e
+                                );
+                            } else {
+                                warn!(
+                                    "Failed to store chunk for StackerDB";
+                                    "stackerdb_contract_id" => &format!("{}", &sync_result.contract_id),
+                                    "slot_id" => md.slot_id,
+                                    "slot_version" => md.slot_version,
+                                    "num_bytes" => chunk.data.len(),
+                                    "error" => %e
+                                );
+                            }
                             continue;
                         } else {
                             debug!("Stored chunk"; "stackerdb_contract_id" => &format!("{}", &sync_result.contract_id), "slot_id" => md.slot_id, "slot_version" => md.slot_version);
diff --git a/testnet/stacks-node/src/run_loop/nakamoto.rs b/testnet/stacks-node/src/run_loop/nakamoto.rs
@@ -723,10 +723,17 @@ impl RunLoop {
 
                     // at tip, and not downloading. proceed to mine.
                     if last_tenure_sortition_height != sortition_db_height {
-                        info!(
-                            "Runloop: Synchronized full burnchain up to height {}. Proceeding to mine blocks",
-                            sortition_db_height
-                        );
+                        if is_miner {
+                            info!(
+                                "Runloop: Synchronized full burnchain up to height {}. Proceeding to mine blocks",
+                                sortition_db_height
+                            );
+                        } else {
+                            info!(
+                                "Runloop: Synchronized full burnchain up to height {}.",
+                                sortition_db_height
+                            );
+                        }
                         last_tenure_sortition_height = sortition_db_height;
                         globals.raise_initiative("runloop-synced".to_string());
                     }
diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs
@@ -1460,6 +1460,7 @@ fn multiple_miners() {
     let node_2_rpc_bind = format!("{localhost}:{node_2_rpc}");
     let mut node_2_listeners = Vec::new();
 
+    let max_nakamoto_tenures = 30;
     // partition the signer set so that ~half are listening and using node 1 for RPC and events,
     //  and the rest are using node 2
 
@@ -1481,6 +1482,7 @@ fn multiple_miners() {
             config.node.p2p_address = format!("{localhost}:{node_1_p2p}");
             config.miner.wait_on_interim_blocks = Duration::from_secs(5);
             config.node.pox_sync_sample_secs = 30;
+            config.burnchain.pox_reward_length = Some(max_nakamoto_tenures);
 
             config.node.seed = btc_miner_1_seed.clone();
             config.node.local_peer_seed = btc_miner_1_seed.clone();
@@ -1560,8 +1562,6 @@ fn multiple_miners() {
 
     info!("------------------------- Reached Epoch 3.0 -------------------------");
 
-    let max_nakamoto_tenures = 20;
-
     // due to the random nature of mining sortitions, the way this test is structured
     //  is that we keep track of how many tenures each miner produced, and once enough sortitions
     //  have been produced such that each miner has produced 3 tenures, we stop and check the
@@ -1645,11 +1645,11 @@ fn multiple_miners() {
     assert_eq!(peer_1_height, peer_2_height);
     assert_eq!(
         peer_1_height,
-        pre_nakamoto_peer_1_height + btc_blocks_mined - 1
+        pre_nakamoto_peer_1_height + btc_blocks_mined as u64 - 1
     );
     assert_eq!(
         btc_blocks_mined,
-        u64::try_from(miner_1_tenures + miner_2_tenures).unwrap()
+        u32::try_from(miner_1_tenures + miner_2_tenures).unwrap()
     );
 
     rl2_coord_channels
@@ -1664,7 +1664,7 @@ fn multiple_miners() {
 /// Read processed nakamoto block IDs from the test observer, and use `config` to open
 ///  a chainstate DB and returns their corresponding StacksHeaderInfos
 fn get_nakamoto_headers(config: &Config) -> Vec<StacksHeaderInfo> {
-    let nakamoto_block_ids: Vec<_> = test_observer::get_blocks()
+    let nakamoto_block_ids: HashSet<_> = test_observer::get_blocks()
         .into_iter()
         .filter_map(|block_json| {
             if block_json
@@ -1746,6 +1746,8 @@ fn miner_forking() {
     let node_2_rpc_bind = format!("{localhost}:{node_2_rpc}");
     let mut node_2_listeners = Vec::new();
 
+    let max_sortitions = 30;
+
     // partition the signer set so that ~half are listening and using node 1 for RPC and events,
     //  and the rest are using node 2
 
@@ -1776,6 +1778,7 @@ fn miner_forking() {
             config.burnchain.local_mining_public_key = Some(btc_miner_1_pk.to_hex());
             config.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[1]));
             config.node.pox_sync_sample_secs = 30;
+            config.burnchain.pox_reward_length = Some(max_sortitions as u32);
 
             config.events_observers.retain(|listener| {
                 let Ok(addr) = std::net::SocketAddr::from_str(&listener.endpoint) else {
@@ -1797,11 +1800,10 @@ fn miner_forking() {
     );
     let conf = signer_test.running_nodes.conf.clone();
     let mut conf_node_2 = conf.clone();
-    let localhost = "127.0.0.1";
-    conf_node_2.node.rpc_bind = format!("{}:{}", localhost, node_2_rpc);
-    conf_node_2.node.p2p_bind = format!("{}:{}", localhost, node_2_p2p);
-    conf_node_2.node.data_url = format!("http://{}:{}", localhost, node_2_rpc);
-    conf_node_2.node.p2p_address = format!("{}:{}", localhost, node_2_p2p);
+    conf_node_2.node.rpc_bind = node_2_rpc_bind;
+    conf_node_2.node.p2p_bind = format!("{localhost}:{node_2_p2p}");
+    conf_node_2.node.data_url = format!("http://{localhost}:{node_2_rpc}");
+    conf_node_2.node.p2p_address = format!("{localhost}:{node_2_p2p}");
     conf_node_2.node.seed = btc_miner_2_seed.clone();
     conf_node_2.burnchain.local_mining_public_key = Some(btc_miner_2_pk.to_hex());
     conf_node_2.node.local_peer_seed = btc_miner_2_seed.clone();
@@ -1931,7 +1933,6 @@ fn miner_forking() {
     // (a) its the first nakamoto tenure
     // (b) the prior sortition didn't have a tenure (because by this time RL2 will have up-to-date block processing)
     let mut expects_miner_2_to_be_valid = true;
-    let max_sortitions = 20;
     // due to the random nature of mining sortitions, the way this test is structured
     //  is that keeps track of two scenarios that we want to cover, and once enough sortitions
     //  have been produced to cover those scenarios, it stops and checks the results at the end.
@@ -3242,7 +3243,8 @@ fn signer_set_rollover() {
 
 #[test]
 #[ignore]
-/// This test checks that the signers will broadcast a block once they receive enough signatures.
+/// This test checks that the miners and signers will not produce Nakamoto blocks
+/// until the minimum time has passed between blocks.
 fn min_gap_between_blocks() {
     if env::var("BITCOIND_TEST") != Ok("1".into()) {
         return;
@@ -3259,11 +3261,14 @@ fn min_gap_between_blocks() {
     let sender_addr = tests::to_addr(&sender_sk);
     let send_amt = 100;
     let send_fee = 180;
+
+    let mut sender_nonce = 0;
+    let interim_blocks = 5;
     let recipient = PrincipalData::from(StacksAddress::burn_address(false));
     let time_between_blocks_ms = 10_000;
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
         num_signers,
-        vec![(sender_addr.clone(), send_amt + send_fee)],
+        vec![(sender_addr.clone(), (send_amt + send_fee) * interim_blocks)],
         |_config| {},
         |config| {
             config.miner.min_time_between_blocks_ms = time_between_blocks_ms;
@@ -3276,73 +3281,81 @@ fn min_gap_between_blocks() {
 
     signer_test.boot_to_epoch_3();
 
-    info!("Ensure that the first Nakamoto block is mined after the gap is exceeded");
+    info!("Ensure that the first Nakamoto block was mined");
     let blocks = get_nakamoto_headers(&signer_test.running_nodes.conf);
     assert_eq!(blocks.len(), 1);
-    let first_block = blocks.last().unwrap();
-    let blocks = test_observer::get_blocks();
-    let parent = blocks
-        .iter()
-        .find(|b| b.get("block_height").unwrap() == first_block.stacks_block_height - 1)
-        .unwrap();
-    let first_block_time = first_block
-        .anchored_header
-        .as_stacks_nakamoto()
-        .unwrap()
-        .timestamp;
-    let parent_block_time = parent.get("burn_block_time").unwrap().as_u64().unwrap();
-    assert!(
-        Duration::from_secs(first_block_time - parent_block_time)
-            >= Duration::from_millis(time_between_blocks_ms),
-        "First block proposed before gap was exceeded: {}s - {}s > {}ms",
-        first_block_time,
-        parent_block_time,
-        time_between_blocks_ms
-    );
+    // mine the interim blocks
+    info!("Mining interim blocks");
+    for interim_block_ix in 0..interim_blocks {
+        let blocks_processed_before = signer_test
+            .running_nodes
+            .nakamoto_blocks_mined
+            .load(Ordering::SeqCst);
+        // submit a tx so that the miner will mine an extra block
+        let transfer_tx = make_stacks_transfer(
+            &sender_sk,
+            sender_nonce,
+            send_fee,
+            signer_test.running_nodes.conf.burnchain.chain_id,
+            &recipient,
+            send_amt,
+        );
+        sender_nonce += 1;
+        submit_tx(&http_origin, &transfer_tx);
 
-    // Submit a tx so that the miner will mine a block
-    let sender_nonce = 0;
-    let transfer_tx = make_stacks_transfer(
-        &sender_sk,
-        sender_nonce,
-        send_fee,
-        signer_test.running_nodes.conf.burnchain.chain_id,
-        &recipient,
-        send_amt,
-    );
-    submit_tx(&http_origin, &transfer_tx);
+        info!("Submitted transfer tx and waiting for block to be processed");
+        wait_for(60, || {
+            let blocks_processed = signer_test
+                .running_nodes
+                .nakamoto_blocks_mined
+                .load(Ordering::SeqCst);
+            Ok(blocks_processed > blocks_processed_before)
+        })
+        .unwrap();
+        info!("Mined interim block:{}", interim_block_ix);
+    }
 
-    info!("Submitted transfer tx and waiting for block to be processed. Ensure it does not arrive before the gap is exceeded");
     wait_for(60, || {
-        let blocks = get_nakamoto_headers(&signer_test.running_nodes.conf);
-        Ok(blocks.len() >= 2)
+        let new_blocks = get_nakamoto_headers(&signer_test.running_nodes.conf);
+        Ok(new_blocks.len() == blocks.len() + interim_blocks as usize)
     })
     .unwrap();
 
-    // Verify that the second Nakamoto block is mined after the gap is exceeded
-    let blocks = get_nakamoto_headers(&signer_test.running_nodes.conf);
-    let last_block = blocks.last().unwrap();
-    let last_block_time = last_block
-        .anchored_header
-        .as_stacks_nakamoto()
-        .unwrap()
-        .timestamp;
-    assert!(blocks.len() >= 2, "Expected at least 2 mined blocks");
-    let penultimate_block = blocks.get(blocks.len() - 2).unwrap();
-    let penultimate_block_time = penultimate_block
-        .anchored_header
-        .as_stacks_nakamoto()
-        .unwrap()
-        .timestamp;
-    assert!(
-        Duration::from_secs(last_block_time - penultimate_block_time)
-            >= Duration::from_millis(time_between_blocks_ms),
-        "Block proposed before gap was exceeded: {}s - {}s > {}ms",
-        last_block_time,
-        penultimate_block_time,
-        time_between_blocks_ms
-    );
-
+    // Verify that every Nakamoto block is mined after the gap is exceeded between each
+    let mut blocks = get_nakamoto_headers(&signer_test.running_nodes.conf);
+    blocks.sort_by(|a, b| a.stacks_block_height.cmp(&b.stacks_block_height));
+    for i in 1..blocks.len() {
+        let block = &blocks[i];
+        let parent_block = &blocks[i - 1];
+        assert_eq!(
+            block.stacks_block_height,
+            parent_block.stacks_block_height + 1
+        );
+        info!(
+            "Checking that the time between blocks {} and {} is respected",
+            parent_block.stacks_block_height, block.stacks_block_height
+        );
+        let block_time = block
+            .anchored_header
+            .as_stacks_nakamoto()
+            .unwrap()
+            .timestamp;
+        let parent_block_time = parent_block
+            .anchored_header
+            .as_stacks_nakamoto()
+            .unwrap()
+            .timestamp;
+        assert!(
+            block_time > parent_block_time,
+            "Block time is BEFORE parent block time"
+        );
+        assert!(
+            Duration::from_secs(block_time - parent_block_time)
+                >= Duration::from_millis(time_between_blocks_ms),
+            "Block mined before gap was exceeded: {block_time}s - {parent_block_time}s > {time_between_blocks_ms}ms",
+        );
+    }
+    debug!("Shutting down min_gap_between_blocks test");
     signer_test.shutdown();
 }
 
@@ -5316,7 +5329,19 @@ fn signing_in_0th_tenure_of_reward_cycle() {
     })
     .unwrap();
 
-    for signer in &signer_public_keys {
+    let block_mined = test_observer::get_mined_nakamoto_blocks()
+        .last()
+        .unwrap()
+        .clone();
+    // Must ensure that the signers that signed the block have their blocks_signed updated appropriately
+    for signature in &block_mined.signer_signature {
+        let signer = signer_public_keys
+            .iter()
+            .find(|pk| {
+                pk.verify(block_mined.signer_signature_hash.as_bytes(), signature)
+                    .unwrap()
+            })
+            .expect("Unknown signer signature");
         let blocks_signed = get_v3_signer(&signer, next_reward_cycle);
         assert_eq!(blocks_signed, 1);
     }
