Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 29 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
572fb81
chore: stdext and rlimit dev-dependencies
jcnelson Sep 16, 2024
95b72cd
bugfix: always save new neighbor data when we get it
jcnelson Sep 16, 2024
69ecf2e
chore: reduce some debug noise, and log connection name for bandwidth…
jcnelson Sep 16, 2024
62bdacd
chore: document neighbor walk connection opts, and add `log_neighbors…
jcnelson Sep 16, 2024
71bdd84
fix: if we're not in IBD and we dno't have a seed node connection (wh…
jcnelson Sep 16, 2024
6c56f71
fix: collate peers by public key and only report the one with the lat…
jcnelson Sep 16, 2024
43d1ba9
feat: log all p2p conversations every `log_neighbor_freq` milliseconds
jcnelson Sep 16, 2024
32b5726
fix: when discovering a new neighbor, don't replace its inbound peer …
jcnelson Sep 16, 2024
88c9a50
fix: don't forward stackerdb chunks that are known to be locally stale
jcnelson Sep 16, 2024
f9b94dc
chore: log the neighbor address which sent the chunk
jcnelson Sep 16, 2024
39265dd
fix: check local rc_consensus_hash against rc_consensus_hash of a sch…
jcnelson Sep 16, 2024
b389d5e
fix/refactor: make it so the small-scale neighbor tests will bind to …
jcnelson Sep 16, 2024
6f37fa6
fix: sort_unstable_by() for sorting peers by health, since our compar…
jcnelson Sep 16, 2024
50a967b
chore: add pathological reward cycles to downloader tests where the o…
jcnelson Sep 16, 2024
d442f2c
build: build tests with `feature = "testing"`, and disable unused war…
jcnelson Sep 16, 2024
f20061f
chore: add dev-dependencies that will allow test modules to compile f…
jcnelson Sep 16, 2024
2ee25e4
build: use `feature = "testing"` to build stackslib test code, and su…
jcnelson Sep 16, 2024
6ed83d0
build: plumb through features
jcnelson Sep 16, 2024
b721cf6
build: buidl stackslib and deps with "testing" feature so we can use …
jcnelson Sep 16, 2024
6324422
chore: expose walk_seed_probability and log_neighbors_freq connection…
jcnelson Sep 16, 2024
f7c2c2a
chore: move topology neighbor convergence tests to integration test CI
jcnelson Sep 16, 2024
ca9c516
chore: activate p2p convergence tests
jcnelson Sep 16, 2024
9ad8852
Merge branch 'develop' into fix/5159
jcnelson Sep 16, 2024
629d032
Merge branch 'develop' into fix/5159
jcnelson Sep 16, 2024
6edfac8
Merge branch 'develop' into fix/5159
jcnelson Sep 16, 2024
50b0ea3
Merge branch 'develop' into fix/5159
jcnelson Sep 16, 2024
8ad5e95
chore: move p2p::conv tests back to stackslib
kantai Sep 16, 2024
e5b9a73
chore: last cleanup from p2p::conv test movement
kantai Sep 16, 2024
23482eb
Add new workflow for p2p convergence tests
wileyj Sep 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/bitcoin-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
23 changes: 23 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
87 changes: 87 additions & 0 deletions .github/workflows/p2p-tests.yml
Original file line number Diff line number Diff line change
@@ -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"
18 changes: 18 additions & 0 deletions .github/workflows/standalone-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ on:
- Atlas Tests
- Bitcoin Tests
- Epoch Tests
- P2P Tests
- Slow Tests
- Stacks-Core Tests
- SBTC Tests
Expand Down Expand Up @@ -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:
Expand Down
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions stackslib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
2 changes: 1 addition & 1 deletion stackslib/src/chainstate/nakamoto/coordinator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
4 changes: 3 additions & 1 deletion stackslib/src/net/api/getneighbors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)?;

Expand Down
25 changes: 16 additions & 9 deletions stackslib/src/net/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1441,6 +1443,7 @@ impl ConversationP2P {
MAX_NEIGHBORS_DATA_LEN,
chain_view.burn_block_height,
false,
thread_rng().gen(),
)
.map_err(net_error::DBError)?;

Expand Down Expand Up @@ -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,
Expand All @@ -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,
)?;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -2436,7 +2443,7 @@ impl ConversationP2P {
}
}
}
debug!("{:?}: received {} bytes", self, total_recved);
test_debug!("{:?}: received {} bytes", self, total_recved);
Ok(total_recved)
}

Expand Down Expand Up @@ -2464,7 +2471,7 @@ impl ConversationP2P {
}
}
}
debug!("{:?}: sent {} bytes", self, total_sent);
test_debug!("{:?}: sent {} bytes", self, total_sent);
Ok(total_sent)
}

Expand Down
Loading