pox-4: remove pox-reject

[friedger]

Description

This PR removes pox-reject from pox-4 and relevant code in boot/mod.rs

Applicable issues

• fixes Nakamoto: PoX-4 should have vote-disabling removed #4108

Additional info (benefits, drawbacks, caveats)

Checklist

• Test coverage for new or modified code paths
• Changelog is updated
• Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
• New clarity functions have corresponding PR in clarity-benchmarking repo
• New integration test(s) added to bitcoin-tests.yml

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (70f7b7d) 82.91% compared to head (0d75ac7) 82.91%.

Files	Patch %	Lines
testnet/stacks-node/src/mockamoto/tests.rs	92.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #4163      +/-   ##
==========================================
- Coverage   82.91%   82.91%   -0.01%     
==========================================
  Files         429      429              
  Lines      302249   302304      +55     
==========================================
+ Hits       250613   250647      +34     
- Misses      51636    51657      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[obycode]

This seems uncontroversial, let's get it merged.

[jcnelson]

The removal of these fields will break /v2/pox and lead to a runtime panic. Please fix stackslib/src/net/api/getpoxinfo.rs.

[friedger]

done thanks to @kantai

[jcnelson]

Please also remove the declaration of pox_rejection_fraction.

[friedger]

done

[jcnelson]

Nice catch! 👍

[jcnelson]

Hi @friedger, thanks for sending this! Please address my comments and I'll rereview. As-is, this would introduce a runtime panic in the /v2/pox handler, because it expects PoX rejection state to be present in pox-4.

[kantai]

In the interest of speeding this along, here's a suggested patch for dealing with the issue @jcnelson mentioned:

It also adds a test which panics without the changes in getpoxinfo, but passes with those changes, so that this would have surfaced via a unit test.

diff --git a/stackslib/src/net/api/getpoxinfo.rs b/stackslib/src/net/api/getpoxinfo.rs
index 0987b9ede..32c9ed6db 100644
--- a/stackslib/src/net/api/getpoxinfo.rs
+++ b/stackslib/src/net/api/getpoxinfo.rs
@@ -70,7 +70,7 @@ pub struct RPCPoxNextCycleInfo {
     pub blocks_until_prepare_phase: i64,
     pub reward_phase_start_block_height: u64,
     pub blocks_until_reward_phase: u64,
-    pub ustx_until_pox_rejection: u64,
+    pub ustx_until_pox_rejection: Option<u64>,
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -90,7 +90,7 @@ pub struct RPCPoxInfoData {
     pub prepare_phase_block_length: u64,
     pub reward_phase_block_length: u64,
     pub reward_slots: u64,
-    pub rejection_fraction: u64,
+    pub rejection_fraction: Option<u64>,
     pub total_liquid_supply_ustx: u64,
     pub current_cycle: RPCPoxCurrentCycleInfo,
     pub next_cycle: RPCPoxNextCycleInfo,
@@ -100,7 +100,7 @@ pub struct RPCPoxInfoData {
     pub prepare_cycle_length: u64,
     pub reward_cycle_id: u64,
     pub reward_cycle_length: u64,
-    pub rejection_votes_left_required: u64,
+    pub rejection_votes_left_required: Option<u64>,
     pub next_reward_cycle_in: u64,
 
     // Information specific to each PoX contract version
@@ -202,12 +202,6 @@ impl RPCPoxInfoData {
             .to_owned()
             .expect_u128() as u64;
 
-        let rejection_fraction = res
-            .get("rejection-fraction")
-            .expect(&format!("FATAL: no 'rejection-fraction'"))
-            .to_owned()
-            .expect_u128() as u64;
-
         let reward_cycle_id = res
             .get("reward-cycle-id")
             .expect(&format!("FATAL: no 'reward-cycle-id'"))
@@ -220,24 +214,39 @@ impl RPCPoxInfoData {
             .to_owned()
             .expect_u128() as u64;
 
-        let current_rejection_votes = res
-            .get("current-rejection-votes")
-            .expect(&format!("FATAL: no 'current-rejection-votes'"))
-            .to_owned()
-            .expect_u128() as u64;
-
         let total_liquid_supply_ustx = res
             .get("total-liquid-supply-ustx")
             .expect(&format!("FATAL: no 'total-liquid-supply-ustx'"))
             .to_owned()
             .expect_u128() as u64;
 
-        let total_required = (total_liquid_supply_ustx as u128 / 100)
-            .checked_mul(rejection_fraction as u128)
-            .ok_or_else(|| NetError::DBError(DBError::Overflow))?
-            as u64;
-
-        let rejection_votes_left_required = total_required.saturating_sub(current_rejection_votes);
+        let has_rejection_data = pox_contract_name == POX_1_NAME
+            || pox_contract_name == POX_2_NAME
+            || pox_contract_name == POX_3_NAME;
+
+        let (rejection_fraction, rejection_votes_left_required) = if has_rejection_data {
+            let rejection_fraction = res
+                .get("rejection-fraction")
+                .expect(&format!("FATAL: no 'rejection-fraction'"))
+                .to_owned()
+                .expect_u128() as u64;
+
+            let current_rejection_votes = res
+                .get("current-rejection-votes")
+                .expect(&format!("FATAL: no 'current-rejection-votes'"))
+                .to_owned()
+                .expect_u128() as u64;
+
+            let total_required = (total_liquid_supply_ustx as u128 / 100)
+                .checked_mul(rejection_fraction as u128)
+                .ok_or_else(|| NetError::DBError(DBError::Overflow))?
+                as u64;
+
+            let votes_left = total_required.saturating_sub(current_rejection_votes);
+            (Some(rejection_fraction), Some(votes_left))
+        } else {
+            (None, None)
+        };
 
         let burnchain_tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())?;
 
diff --git a/testnet/stacks-node/src/mockamoto/tests.rs b/testnet/stacks-node/src/mockamoto/tests.rs
index 2c7df7000..a2d43f4f7 100644
--- a/testnet/stacks-node/src/mockamoto/tests.rs
+++ b/testnet/stacks-node/src/mockamoto/tests.rs
@@ -19,7 +19,7 @@ use wsts::curve::scalar::Scalar;
 use super::MockamotoNode;
 use crate::config::{EventKeyType, EventObserverConfig};
 use crate::neon_node::PeerThread;
-use crate::tests::neon_integrations::{submit_tx, test_observer};
+use crate::tests::neon_integrations::{get_pox_info, submit_tx, test_observer};
 use crate::tests::{make_contract_call, make_stacks_transfer, to_addr};
 use crate::{Config, ConfigFile};
 
@@ -354,3 +354,63 @@ fn observe_set_aggregate_key() {
     assert_eq!(orig_aggregate_key.unwrap(), orig_key);
     assert_eq!(new_aggregate_key.unwrap(), orig_key);
 }
+
+#[test]
+fn rpc_pox_info() {
+    let mut conf = Config::from_config_file(ConfigFile::mockamoto()).unwrap();
+    conf.node.mockamoto_time_ms = 10;
+    conf.node.rpc_bind = "127.0.0.1:19543".into();
+    conf.node.p2p_bind = "127.0.0.1:19544".into();
+
+    let observer_port = 19500;
+    test_observer::spawn_at(observer_port);
+    conf.events_observers.insert(EventObserverConfig {
+        endpoint: format!("localhost:{observer_port}"),
+        events_keys: vec![EventKeyType::AnyEvent],
+    });
+
+    let mut mockamoto = MockamotoNode::new(&conf).unwrap();
+    let globals = mockamoto.globals.clone();
+
+    let http_origin = format!("http://{}", &conf.node.rpc_bind);
+
+    let start = Instant::now();
+
+    let node_thread = thread::Builder::new()
+        .name("mockamoto-main".into())
+        .spawn(move || mockamoto.run())
+        .expect("FATAL: failed to start mockamoto main thread");
+
+    // mine 5 blocks
+    let completed = loop {
+        // complete within 2 minutes or abort
+        if Instant::now().duration_since(start) > Duration::from_secs(120) {
+            break false;
+        }
+        let latest_block = test_observer::get_blocks().pop();
+        thread::sleep(Duration::from_secs(1));
+        let Some(ref latest_block) = latest_block else {
+            info!("No block observed yet!");
+            continue;
+        };
+        let stacks_block_height = latest_block.get("block_height").unwrap().as_u64().unwrap();
+        info!("Block height observed: {stacks_block_height}");
+
+        if stacks_block_height >= 5 {
+            break true;
+        }
+    };
+
+    // fetch rpc poxinfo
+    let _pox_info = get_pox_info(&http_origin);
+
+    globals.signal_stop();
+
+    assert!(
+        completed,
+        "Mockamoto node failed to produce and announce 100 blocks before timeout"
+    );
+    node_thread
+        .join()
+        .expect("Failed to join node thread to exit");
+}
diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs
index 5676d1bb1..db507e48c 100644
--- a/testnet/stacks-node/src/tests/neon_integrations.rs
+++ b/testnet/stacks-node/src/tests/neon_integrations.rs
@@ -5992,7 +5992,7 @@ fn pox_integration_test() {
     );
     assert_eq!(
         pox_info.rejection_fraction,
-        pox_constants.pox_rejection_fraction
+        Some(pox_constants.pox_rejection_fraction)
     );
     assert_eq!(pox_info.reward_cycle_id, 0);
     assert_eq!(pox_info.current_cycle.id, 0);
@@ -6060,7 +6060,7 @@ fn pox_integration_test() {
     );
     assert_eq!(
         pox_info.rejection_fraction,
-        pox_constants.pox_rejection_fraction
+        Some(pox_constants.pox_rejection_fraction)
     );
     assert_eq!(pox_info.reward_cycle_id, 14);
     assert_eq!(pox_info.current_cycle.id, 14);
@@ -6191,7 +6191,7 @@ fn pox_integration_test() {
     );
     assert_eq!(
         pox_info.rejection_fraction,
-        pox_constants.pox_rejection_fraction
+        Some(pox_constants.pox_rejection_fraction)
     );
     assert_eq!(pox_info.reward_cycle_id, 14);
     assert_eq!(pox_info.current_cycle.id, 14);

[friedger]

Thank you @kantai
Comments addressed, @jcnelson

[kantai]

LGTM, thanks @friedger!

[kantai]

Tagging @jcnelson for a re-review here