Merge pull request #5191 from stacks-network/fix/5159

Fix: 5159, 5169, 5171, 5172, and others

.github/workflows/bitcoin-tests.yml

@@ -105,6 +105,10 @@ jobs:
           - tests::signer::v0::locally_rejected_blocks_overriden_by_global_acceptance
           - tests::signer::v0::reorg_locally_accepted_blocks_across_tenures_succeeds
           - tests::signer::v0::miner_recovers_when_broadcast_block_delay_across_tenures_occurs
+          - tests::signer::v0::multiple_miners_with_nakamoto_blocks
+          - tests::signer::v0::partial_tenure_fork
+          - tests::signer::v0::mine_2_nakamoto_reward_cycles
+          - tests::signer::v0::signer_set_rollover
           - tests::nakamoto_integrations::stack_stx_burn_op_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state
@@ -117,10 +121,6 @@ jobs:
           - tests::nakamoto_integrations::follower_bootup_across_multiple_cycles
           - tests::nakamoto_integrations::utxo_check_on_startup_panic
           - tests::nakamoto_integrations::utxo_check_on_startup_recover
-          - tests::signer::v0::multiple_miners_with_nakamoto_blocks
-          - tests::signer::v0::partial_tenure_fork
-          - tests::signer::v0::mine_2_nakamoto_reward_cycles
-          - tests::signer::v0::signer_set_rollover
           # Do not run this one until we figure out why it fails in CI
           # - tests::neon_integrations::bitcoin_reorg_flap
           # - tests::neon_integrations::bitcoin_reorg_flap_with_follower

.github/workflows/ci.yml

@@ -186,6 +186,29 @@ jobs:
       - check-release
     uses: ./.github/workflows/bitcoin-tests.yml
 
+
+  p2p-tests:
+    if: |
+      needs.check-release.outputs.is_release == 'true' || (
+        github.event_name == 'workflow_dispatch' ||
+        github.event_name == 'pull_request' ||
+        github.event_name == 'merge_group' ||
+        (
+          contains('
+            refs/heads/master
+            refs/heads/develop
+            refs/heads/next
+          ', github.event.pull_request.head.ref) &&
+          github.event_name == 'push'
+        )
+      )
+    name: P2P Tests
+    needs:
+      - rustfmt
+      - create-cache
+      - check-release
+    uses: ./.github/workflows/p2p-tests.yml
+
   ## Test to run on a tagged release
   ##
   ## Runs when:

.github/workflows/p2p-tests.yml

@@ -0,0 +1,87 @@
+## Github workflow to run p2p tests
+
+name: Tests::P2P
+
+on:
+  workflow_call:
+
+## env vars are transferred to composite action steps
+env:
+  BITCOIND_TEST: 0
+  RUST_BACKTRACE: full
+  SEGMENT_DOWNLOAD_TIMEOUT_MINS: 15
+  TEST_TIMEOUT: 30
+
+concurrency:
+  group: stackslib-tests-${{ github.head_ref || github.ref || github.run_id}}
+  ## Only cancel in progress if this is for a PR
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+jobs:
+  # p2p integration tests with code coverage
+  integration-tests:
+    name: Integration Tests
+    runs-on: ubuntu-latest
+    strategy:
+      ## Continue with the test matrix even if we've had a failure
+      fail-fast: false
+      ## Run a maximum of 32 concurrent tests from the test matrix
+      max-parallel: 32
+      matrix:
+        test-name:
+          - net::tests::convergence::test_walk_ring_allow_15
+          - net::tests::convergence::test_walk_ring_15_plain
+          - net::tests::convergence::test_walk_ring_15_pingback
+          - net::tests::convergence::test_walk_ring_15_org_biased
+          - net::tests::convergence::test_walk_line_allowed_15
+          - net::tests::convergence::test_walk_line_15_plain
+          - net::tests::convergence::test_walk_line_15_org_biased
+          - net::tests::convergence::test_walk_line_15_pingback
+          - net::tests::convergence::test_walk_star_allowed_15
+          - net::tests::convergence::test_walk_star_15_plain
+          - net::tests::convergence::test_walk_star_15_pingback
+          - net::tests::convergence::test_walk_star_15_org_biased
+          - net::tests::convergence::test_walk_inbound_line_15
+    steps:
+      ## Setup test environment
+      - name: Setup Test Environment
+        id: setup_tests
+        uses: stacks-network/actions/stacks-core/testenv@main
+        with:
+          btc-version: "25.0"
+
+      ## Increase open file descriptors limit
+      - name: Increase Open File Descriptors
+        run: |
+          sudo prlimit --nofile=4096:4096
+
+      ## Run test matrix using restored cache of archive file
+      ##   - Test will timeout after env.TEST_TIMEOUT minutes
+      - name: Run Tests
+        id: run_tests
+        timeout-minutes: ${{ fromJSON(env.TEST_TIMEOUT) }}
+        uses: stacks-network/actions/stacks-core/run-tests@main
+        with:
+          test-name: ${{ matrix.test-name }}
+          threads: 1
+
+      ## Create and upload code coverage file
+      - name: Code Coverage
+        id: codecov
+        uses: stacks-network/actions/codecov@main
+        with:
+          test-name: ${{ matrix.test-name }}
+
+  check-tests:
+    name: Check Tests
+    runs-on: ubuntu-latest
+    if: always()
+    needs:
+      - integration-tests
+    steps:
+      - name: Check Tests Status
+        id: check_tests_status
+        uses: stacks-network/actions/check-jobs-status@main
+        with:
+          jobs: ${{ toJson(needs) }}
+          summary_print: "true"

.github/workflows/standalone-tests.yml

@@ -21,6 +21,7 @@ on:
           - Atlas Tests
           - Bitcoin Tests
           - Epoch Tests
+          - P2P Tests
           - Slow Tests
           - Stacks-Core Tests
           - SBTC Tests
@@ -69,6 +70,23 @@ jobs:
       - create-cache
     uses: ./.github/workflows/bitcoin-tests.yml
 
+  ## Runs when:
+  ##   either or of the following:
+  ##     - workflow is 'Release Tests'
+  ##     - workflow is 'CI Tests'
+  ##     - workflow is 'P2P Tests'
+  p2p-tests:
+    if: |
+      (
+        inputs.workflow == 'Release Tests' ||
+        inputs.workflow == 'CI Tests' ||
+        inputs.workflow == 'P2P Tests'
+      )
+    name: P2P Tests
+    needs:
+      - create-cache
+    uses: ./.github/workflows/p2p-tests.yml
+
   #####################################################
   ## Runs when:
   ##   either or of the following:

stackslib/Cargo.toml

@@ -101,6 +101,7 @@ stacks-common = { features = ["default", "testing"], path = "../stacks-common" }
 rstest = "0.17.0"
 rstest_reuse = "0.5.0"
 mutants = "0.0.3"
+rlimit = "0.10.2"
 
 [features]
 default = []

stackslib/src/chainstate/nakamoto/coordinator/mod.rs

@@ -506,7 +506,7 @@ pub fn load_nakamoto_reward_set<U: RewardSetProvider>(
                 Err(e) => return Some(Err(e)),
                 Ok(None) => {
                     // no header for this snapshot (possibly invalid)
-                    info!("Failed to find block by consensus hash"; "consensus_hash" => %sn.consensus_hash);
+                    debug!("Failed to find Stacks block by consensus hash"; "consensus_hash" => %sn.consensus_hash);
                     return None
                 }
             }

stackslib/src/net/api/getneighbors.rs

@@ -19,6 +19,7 @@ use std::io::{Read, Write};
 use clarity::vm::types::QualifiedContractIdentifier;
 use regex::{Captures, Regex};
 use stacks_common::types::net::{PeerAddress, PeerHost};
+use stacks_common::util::get_epoch_time_secs;
 use stacks_common::util::hash::Hash160;
 
 use crate::net::db::PeerDB;
@@ -145,10 +146,11 @@ impl RPCNeighborsInfo {
             peerdb_conn,
             network_id,
             network_epoch,
-            max_neighbor_age,
+            get_epoch_time_secs().saturating_sub(max_neighbor_age),
             MAX_NEIGHBORS_DATA_LEN,
             burnchain_view.burn_block_height,
             false,
+            true,
         )
         .map_err(NetError::DBError)?;
 

stackslib/src/net/chat.rs

@@ -1433,6 +1433,8 @@ impl ConversationP2P {
 
         // get neighbors at random as long as they're fresh, and as long as they're compatible with
         // the current system epoch.
+        // Alternate at random between serving public-only and public/private-mixed IPs, since for
+        // the time being, the remote peer has no way of asking for a particular subset.
         let mut neighbors = PeerDB::get_fresh_random_neighbors(
             peer_dbconn,
             self.network_id,
@@ -1441,6 +1443,7 @@ impl ConversationP2P {
             MAX_NEIGHBORS_DATA_LEN,
             chain_view.burn_block_height,
             false,
+            thread_rng().gen(),
         )
         .map_err(net_error::DBError)?;
 
@@ -1917,10 +1920,12 @@ impl ConversationP2P {
     /// Generates a Nack if we don't have this DB, or if the request's consensus hash is invalid.
     fn make_stacker_db_getchunkinv_response(
         network: &PeerNetwork,
+        naddr: NeighborAddress,
         chainstate: &mut StacksChainState,
         getchunkinv: &StackerDBGetChunkInvData,
     ) -> Result<StacksMessageType, net_error> {
         Ok(network.make_StackerDBChunksInv_or_Nack(
+            naddr,
             chainstate,
             &getchunkinv.contract_id,
             &getchunkinv.rc_consensus_hash,
@@ -1938,6 +1943,7 @@ impl ConversationP2P {
     ) -> Result<ReplyHandleP2P, net_error> {
         let response = ConversationP2P::make_stacker_db_getchunkinv_response(
             network,
+            self.to_neighbor_address(),
             chainstate,
             getchunkinv,
         )?;
@@ -2120,7 +2126,8 @@ impl ConversationP2P {
                 > (self.connection.options.max_block_push_bandwidth as f64)
         {
             debug!(
-                "Neighbor {:?} exceeded max block-push bandwidth of {} bytes/sec (currently at {})",
+                "{:?}: Neighbor {:?} exceeded max block-push bandwidth of {} bytes/sec (currently at {})",
+                &self,
                 &self.to_neighbor_key(),
                 self.connection.options.max_block_push_bandwidth,
                 self.stats.get_block_push_bandwidth()
@@ -2162,7 +2169,7 @@ impl ConversationP2P {
             && self.stats.get_microblocks_push_bandwidth()
                 > (self.connection.options.max_microblocks_push_bandwidth as f64)
         {
-            debug!("Neighbor {:?} exceeded max microblocks-push bandwidth of {} bytes/sec (currently at {})", &self.to_neighbor_key(), self.connection.options.max_microblocks_push_bandwidth, self.stats.get_microblocks_push_bandwidth());
+            debug!("{:?}: Neighbor {:?} exceeded max microblocks-push bandwidth of {} bytes/sec (currently at {})", self, &self.to_neighbor_key(), self.connection.options.max_microblocks_push_bandwidth, self.stats.get_microblocks_push_bandwidth());
             return self
                 .reply_nack(local_peer, chain_view, preamble, NackErrorCodes::Throttled)
                 .and_then(|handle| Ok(Some(handle)));
@@ -2199,7 +2206,7 @@ impl ConversationP2P {
             && self.stats.get_transaction_push_bandwidth()
                 > (self.connection.options.max_transaction_push_bandwidth as f64)
         {
-            debug!("Neighbor {:?} exceeded max transaction-push bandwidth of {} bytes/sec (currently at {})", &self.to_neighbor_key(), self.connection.options.max_transaction_push_bandwidth, self.stats.get_transaction_push_bandwidth());
+            debug!("{:?}: Neighbor {:?} exceeded max transaction-push bandwidth of {} bytes/sec (currently at {})", self, &self.to_neighbor_key(), self.connection.options.max_transaction_push_bandwidth, self.stats.get_transaction_push_bandwidth());
             return self
                 .reply_nack(local_peer, chain_view, preamble, NackErrorCodes::Throttled)
                 .and_then(|handle| Ok(Some(handle)));
@@ -2237,7 +2244,7 @@ impl ConversationP2P {
             && self.stats.get_stackerdb_push_bandwidth()
                 > (self.connection.options.max_stackerdb_push_bandwidth as f64)
         {
-            debug!("Neighbor {:?} exceeded max stackerdb-push bandwidth of {} bytes/sec (currently at {})", &self.to_neighbor_key(), self.connection.options.max_stackerdb_push_bandwidth, self.stats.get_stackerdb_push_bandwidth());
+            debug!("{:?}: Neighbor {:?} exceeded max stackerdb-push bandwidth of {} bytes/sec (currently at {})", self, &self.to_neighbor_key(), self.connection.options.max_stackerdb_push_bandwidth, self.stats.get_stackerdb_push_bandwidth());
             return self
                 .reply_nack(local_peer, chain_view, preamble, NackErrorCodes::Throttled)
                 .and_then(|handle| Ok(Some(handle)));
@@ -2276,7 +2283,7 @@ impl ConversationP2P {
             && self.stats.get_nakamoto_block_push_bandwidth()
                 > (self.connection.options.max_nakamoto_block_push_bandwidth as f64)
         {
-            debug!("Neighbor {:?} exceeded max Nakamoto block push bandwidth of {} bytes/sec (currently at {})", &self.to_neighbor_key(), self.connection.options.max_nakamoto_block_push_bandwidth, self.stats.get_nakamoto_block_push_bandwidth());
+            debug!("{:?}: Neighbor {:?} exceeded max Nakamoto block push bandwidth of {} bytes/sec (currently at {})", self, &self.to_neighbor_key(), self.connection.options.max_nakamoto_block_push_bandwidth, self.stats.get_nakamoto_block_push_bandwidth());
             return self
                 .reply_nack(local_peer, chain_view, preamble, NackErrorCodes::Throttled)
                 .and_then(|handle| Ok(Some(handle)));
@@ -2415,11 +2422,11 @@ impl ConversationP2P {
                 Ok(num_recved) => {
                     total_recved += num_recved;
                     if num_recved > 0 {
-                        debug!("{:?}: received {} bytes", self, num_recved);
+                        test_debug!("{:?}: received {} bytes", self, num_recved);
                         self.stats.last_recv_time = get_epoch_time_secs();
                         self.stats.bytes_rx += num_recved as u64;
                     } else {
-                        debug!("{:?}: received {} bytes, stopping", self, num_recved);
+                        test_debug!("{:?}: received {} bytes, stopping", self, num_recved);
                         break;
                     }
                 }
@@ -2436,7 +2443,7 @@ impl ConversationP2P {
                 }
             }
         }
-        debug!("{:?}: received {} bytes", self, total_recved);
+        test_debug!("{:?}: received {} bytes", self, total_recved);
         Ok(total_recved)
     }
 
@@ -2464,7 +2471,7 @@ impl ConversationP2P {
                 }
             }
         }
-        debug!("{:?}: sent {} bytes", self, total_sent);
+        test_debug!("{:?}: sent {} bytes", self, total_sent);
         Ok(total_sent)
     }