Skip to content

Commit 01d0f15

Browse files
jferrantjferrant
committed
Add DropSource throughout network stack
Signed-off-by: Jacinta Ferrant <[email protected]>
1 parent 092cc93 commit 01d0f15

File tree

15 files changed

+517
-202
lines changed

15 files changed

+517
-202
lines changed

‎stackslib/src/net/download/epoch2x.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use std::sync::mpsc::{
2222
sync_channel, Receiver, RecvError, RecvTimeoutError, SyncSender, TryRecvError, TrySendError,
2323
};
2424

25+
use p2p::DropSource;
2526
use rand::seq::SliceRandom;
2627
use rand::{thread_rng, RngCore};
2728
use stacks_common::types::chainstate::{BlockHeaderHash, PoxId, SortitionId, StacksBlockId};
@@ -502,9 +503,10 @@ impl BlockDownloader {
502503
self.broken_peers.push(event_id);
503504
self.broken_neighbors.push(DropNeighbor {
504505
key: block_key.neighbor.clone(),
505-
reason: DropReason::BrokenConnection(Some(
506+
reason: DropReason::BrokenConnection(
506507
"Remote neighbor sent an invalid block".into(),
507-
)),
508+
),
509+
source: DropSource::BlockDownloaderGetBlocks,
508510
});
509511
} else {
510512
// got the block
@@ -526,10 +528,11 @@ impl BlockDownloader {
526528
self.broken_peers.push(event_id);
527529
self.broken_neighbors.push(DropNeighbor {
528530
key: block_key.neighbor.clone(),
529-
reason: DropReason::BrokenConnection(Some(
531+
reason: DropReason::BrokenConnection(
530532
"Remote neighbor was missing an expected block"
531533
.into(),
532-
)),
534+
),
535+
source: DropSource::BlockDownloaderGetBlocks,
533536
});
534537
}
535538
Err(e) => {
@@ -539,7 +542,8 @@ impl BlockDownloader {
539542
self.broken_peers.push(event_id);
540543
self.broken_neighbors.push(DropNeighbor {
541544
key: block_key.neighbor.clone(),
542-
reason: DropReason::BrokenConnection(Some(format!("Error occurred decoding block response from neighbor: {e}")))
545+
reason: DropReason::BrokenConnection(format!("Error occurred decoding block response from neighbor: {e}")),
546+
source: DropSource::BlockDownloaderGetBlocks
543547
});
544548
}
545549
}
@@ -648,7 +652,8 @@ impl BlockDownloader {
648652
self.broken_peers.push(event_id);
649653
self.broken_neighbors.push(DropNeighbor {
650654
key: block_key.neighbor.clone(),
651-
reason: DropReason::BrokenConnection(Some("Remote neighbor sent an unexpected zero-length microblock stream".into()))
655+
reason: DropReason::BrokenConnection("Remote neighbor sent an unexpected zero-length microblock stream".into()),
656+
source: DropSource::BlockDownloaderGetMicroblocks
652657
});
653658
} else {
654659
// have microblocks (but we don't know yet if they're well-formed)
@@ -683,7 +688,8 @@ impl BlockDownloader {
683688
self.broken_peers.push(event_id);
684689
self.broken_neighbors.push(DropNeighbor {
685690
key: block_key.neighbor.clone(),
686-
reason: DropReason::BrokenConnection(Some(format!("Error occurred decoding microblock response from neighbor: {e}")))
691+
reason: DropReason::BrokenConnection(format!("Error occurred decoding microblock response from neighbor: {e}")),
692+
source: DropSource::BlockDownloaderGetMicroblocks
687693
});
688694
}
689695
}

‎stackslib/src/net/download/nakamoto/download_state_machine.rs

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ use crate::net::inv::epoch2x::InvState;
6363
use crate::net::inv::nakamoto::{NakamotoInvStateMachine, NakamotoTenureInv};
6464
use crate::net::neighbors::rpc::NeighborRPC;
6565
use crate::net::neighbors::NeighborComms;
66-
use crate::net::p2p::{CurrentRewardSet, PeerNetwork};
66+
use crate::net::p2p::{CurrentRewardSet, DropReason, DropSource, PeerNetwork};
6767
use crate::net::server::HttpPeer;
6868
use crate::net::{Error as NetError, Neighbor, NeighborAddress, NeighborKey};
6969
use crate::util_lib::db::{DBConn, Error as DBError};
@@ -1205,7 +1205,12 @@ impl NakamotoDownloadStateMachine {
12051205
"Downloader for {} failed; this peer is dead: {:?}",
12061206
&naddr, &e
12071207
);
1208-
neighbor_rpc.add_dead(network, naddr);
1208+
neighbor_rpc.add_dead(
1209+
network,
1210+
naddr,
1211+
DropReason::DeadConnection(format!("Failed to send download request: {e}")),
1212+
DropSource::NakamotoDownloadStateMachine,
1213+
);
12091214
continue;
12101215
};
12111216
}
@@ -1237,12 +1242,24 @@ impl NakamotoDownloadStateMachine {
12371242
) {
12381243
Ok(blocks_opt) => blocks_opt,
12391244
Err(NetError::StaleView) => {
1240-
neighbor_rpc.add_dead(network, &naddr);
1245+
neighbor_rpc.add_dead(
1246+
network,
1247+
&naddr,
1248+
DropReason::DeadConnection("Stale view".into()),
1249+
DropSource::NakamotoDownloadStateMachine,
1250+
);
12411251
continue;
12421252
}
12431253
Err(e) => {
12441254
debug!("Failed to handle next download response from unconfirmed downloader for {:?} in state {:?}: {:?}", &naddr, &downloader.state, &e);
1245-
neighbor_rpc.add_dead(network, &naddr);
1255+
neighbor_rpc.add_dead(
1256+
network,
1257+
&naddr,
1258+
DropReason::DeadConnection(format!(
1259+
"Failed to handle next download response: {e}"
1260+
)),
1261+
DropSource::NakamotoDownloadStateMachine,
1262+
);
12461263
continue;
12471264
}
12481265
};

‎stackslib/src/net/download/nakamoto/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,11 +236,11 @@ impl PeerNetwork {
236236
};
237237

238238
for broken in block_downloader.neighbor_rpc.take_broken() {
239-
self.deregister_and_ban_neighbor(&broken, DropReason::BrokenConnection(None));
239+
self.deregister_and_ban_neighbor(&broken.key, broken.reason, broken.source);
240240
}
241241

242242
for dead in block_downloader.neighbor_rpc.take_dead() {
243-
self.deregister_neighbor(&dead, DropReason::DeadConnection);
243+
self.deregister_neighbor(&dead.key, dead.reason, dead.source);
244244
}
245245

246246
self.block_downloader_nakamoto = Some(block_downloader);

‎stackslib/src/net/download/nakamoto/tenure_downloader.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use crate::net::inv::epoch2x::InvState;
5757
use crate::net::inv::nakamoto::{NakamotoInvStateMachine, NakamotoTenureInv};
5858
use crate::net::neighbors::rpc::NeighborRPC;
5959
use crate::net::neighbors::NeighborComms;
60-
use crate::net::p2p::{CurrentRewardSet, PeerNetwork};
60+
use crate::net::p2p::{CurrentRewardSet, DropReason, DropSource, PeerNetwork};
6161
use crate::net::server::HttpPeer;
6262
use crate::net::{Error as NetError, Neighbor, NeighborAddress, NeighborKey};
6363
use crate::util_lib::db::{DBConn, Error as DBError};
@@ -745,7 +745,12 @@ impl NakamotoTenureDownloader {
745745

746746
let Some(peerhost) = NeighborRPC::get_peer_host(network, &self.naddr) else {
747747
// no conversation open to this neighbor
748-
neighbor_rpc.add_dead(network, &self.naddr);
748+
neighbor_rpc.add_dead(
749+
network,
750+
&self.naddr,
751+
DropReason::DeadConnection("No authenticated connection open".into()),
752+
DropSource::NakamotoTenureDownloader,
753+
);
749754
return Err(NetError::PeerNotConnected);
750755
};
751756

‎stackslib/src/net/download/nakamoto/tenure_downloader_set.rs

Lines changed: 48 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use crate::chainstate::nakamoto::{
4242
NakamotoBlock, NakamotoBlockHeader, NakamotoChainState, NakamotoStagingBlocksConnRef,
4343
};
4444
use crate::chainstate::stacks::boot::RewardSet;
45-
use crate::chainstate::stacks::db::StacksChainState;
45+
use crate::chainstate::stacks::db::{blocks, StacksChainState};
4646
use crate::chainstate::stacks::{
4747
Error as chainstate_error, StacksBlockHeader, TenureChangePayload,
4848
};
@@ -62,7 +62,7 @@ use crate::net::inv::epoch2x::InvState;
6262
use crate::net::inv::nakamoto::{NakamotoInvStateMachine, NakamotoTenureInv};
6363
use crate::net::neighbors::rpc::NeighborRPC;
6464
use crate::net::neighbors::NeighborComms;
65-
use crate::net::p2p::{CurrentRewardSet, PeerNetwork};
65+
use crate::net::p2p::{CurrentRewardSet, DropReason, DropSource, PeerNetwork};
6666
use crate::net::server::HttpPeer;
6767
use crate::net::{Error as NetError, Neighbor, NeighborAddress, NeighborKey};
6868
use crate::util_lib::db::{DBConn, Error as DBError};
@@ -583,25 +583,33 @@ impl NakamotoTenureDownloaderSet {
583583
"Send request to {naddr} for tenure {} (state {})",
584584
&downloader.tenure_id_consensus_hash, &downloader.state
585585
);
586-
let Ok(sent) = downloader.send_next_download_request(network, neighbor_rpc) else {
587-
info!(
588-
"Downloader for tenure {} to {naddr} failed; this peer is dead",
589-
&downloader.tenure_id_consensus_hash,
590-
);
591-
Self::mark_failed_and_deprioritize_peer(
592-
&mut self.attempt_failed_tenures,
593-
&mut self.deprioritized_peers,
594-
&downloader.tenure_id_consensus_hash,
595-
naddr,
596-
);
597-
neighbor_rpc.add_dead(network, naddr);
598-
continue;
586+
match downloader.send_next_download_request(network, neighbor_rpc) {
587+
Ok(true) => {}
588+
Ok(false) => {
589+
// this downloader is dead or broken
590+
finished.push(naddr.clone());
591+
continue;
592+
}
593+
Err(e) => {
594+
info!(
595+
"Downloader for tenure {} to {naddr} failed; this peer is dead",
596+
&downloader.tenure_id_consensus_hash,
597+
);
598+
Self::mark_failed_and_deprioritize_peer(
599+
&mut self.attempt_failed_tenures,
600+
&mut self.deprioritized_peers,
601+
&downloader.tenure_id_consensus_hash,
602+
naddr,
603+
);
604+
neighbor_rpc.add_dead(
605+
network,
606+
naddr,
607+
DropReason::DeadConnection(format!("Download request failed: {e}")),
608+
DropSource::NakamotoTenureDownloader,
609+
);
610+
continue;
611+
}
599612
};
600-
if !sent {
601-
// this downloader is dead or broken
602-
finished.push(naddr.clone());
603-
continue;
604-
}
605613
}
606614

607615
// clear dead, broken, and done
@@ -631,32 +639,30 @@ impl NakamotoTenureDownloaderSet {
631639
};
632640
debug!("Got response from {naddr}");
633641

634-
let Ok(blocks_opt) = downloader
635-
.handle_next_download_response(response)
636-
.map_err(|e| {
642+
let blocks = match downloader.handle_next_download_response(response) {
643+
Ok(Some(blocks)) => blocks,
644+
Ok(None) => continue,
645+
Err(e) => {
637646
info!(
638647
"Failed to handle response from {naddr} on tenure {}: {e}",
639648
&downloader.tenure_id_consensus_hash,
640649
);
641-
e
642-
})
643-
else {
644-
debug!(
645-
"Failed to handle download response from {naddr} on tenure {}",
646-
&downloader.tenure_id_consensus_hash
647-
);
648-
Self::mark_failed_and_deprioritize_peer(
649-
&mut self.attempt_failed_tenures,
650-
&mut self.deprioritized_peers,
651-
&downloader.tenure_id_consensus_hash,
652-
&naddr,
653-
);
654-
neighbor_rpc.add_dead(network, &naddr);
655-
continue;
656-
};
657-
658-
let Some(blocks) = blocks_opt else {
659-
continue;
650+
Self::mark_failed_and_deprioritize_peer(
651+
&mut self.attempt_failed_tenures,
652+
&mut self.deprioritized_peers,
653+
&downloader.tenure_id_consensus_hash,
654+
&naddr,
655+
);
656+
neighbor_rpc.add_dead(
657+
network,
658+
&naddr,
659+
DropReason::DeadConnection(format!(
660+
"Failed to handle download response: {e}"
661+
)),
662+
DropSource::NakamotoTenureDownloader,
663+
);
664+
continue;
665+
}
660666
};
661667

662668
debug!(

‎stackslib/src/net/download/nakamoto/tenure_downloader_unconfirmed.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ use crate::net::inv::epoch2x::InvState;
6262
use crate::net::inv::nakamoto::{NakamotoInvStateMachine, NakamotoTenureInv};
6363
use crate::net::neighbors::rpc::NeighborRPC;
6464
use crate::net::neighbors::NeighborComms;
65-
use crate::net::p2p::{CurrentRewardSet, PeerNetwork};
65+
use crate::net::p2p::{CurrentRewardSet, DropReason, DropSource, PeerNetwork};
6666
use crate::net::server::HttpPeer;
6767
use crate::net::{Error as NetError, Neighbor, NeighborAddress, NeighborKey};
6868
use crate::util_lib::db::{DBConn, Error as DBError};
@@ -838,7 +838,12 @@ impl NakamotoUnconfirmedTenureDownloader {
838838

839839
let Some(peerhost) = NeighborRPC::get_peer_host(network, &self.naddr) else {
840840
// no conversation open to this neighbor
841-
neighbor_rpc.add_dead(network, &self.naddr);
841+
neighbor_rpc.add_dead(
842+
network,
843+
&self.naddr,
844+
DropReason::DeadConnection("No authenticated connection open".into()),
845+
DropSource::NakamotoUnconfirmedTenureDownloader,
846+
);
842847
return Err(NetError::PeerNotConnected);
843848
};
844849

‎stackslib/src/net/inv/epoch2x.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
1919
use std::io::{Read, Write};
2020
use std::net::SocketAddr;
2121

22+
use p2p::DropSource;
2223
use rand;
2324
use rand::seq::SliceRandom;
2425
use rand::{thread_rng, Rng};
@@ -2310,6 +2311,7 @@ impl PeerNetwork {
23102311
stats.done
23112312
);
23122313
if !stats.done {
2314+
// TODO: the result of this match doesn't seem to be used? is that intentional?
23132315
match network.inv_sync_run(&mut new_pins, sortdb, nk, stats, inv_state.request_timeout, ibd) {
23142316
Ok(d) => d,
23152317
Err(net_error::StaleView) => {
@@ -2676,12 +2678,22 @@ impl PeerNetwork {
26762678

26772679
// disconnect and ban broken peers
26782680
for broken in broken_neighbors.into_iter() {
2679-
self.deregister_and_ban_neighbor(&broken, DropReason::BannedConnection);
2681+
//substantial changes to the epoch2x sync would be required to get further detail about why the connection was broken. Just use "Unknown" for now.
2682+
self.deregister_and_ban_neighbor(
2683+
&broken,
2684+
DropReason::BrokenConnection("Unknown".into()),
2685+
DropSource::Epoch2xInventorySync,
2686+
);
26802687
}
26812688

26822689
// disconnect from dead connections
26832690
for dead in dead_neighbors.into_iter() {
2684-
self.deregister_neighbor(&dead, DropReason::DeadConnection);
2691+
//substantial changes to the epoch2x sync would be required to get further detail about why the connection is dead. Just use "Unknown" for now.
2692+
self.deregister_neighbor(
2693+
&dead,
2694+
DropReason::DeadConnection("Unknown".into()),
2695+
DropSource::Epoch2xInventorySync,
2696+
);
26852697
}
26862698

26872699
(done, throttled)

‎stackslib/src/net/inv/nakamoto.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ use crate::chainstate::nakamoto::NakamotoChainState;
2828
use crate::chainstate::stacks::db::StacksChainState;
2929
use crate::net::db::PeerDB;
3030
use crate::net::neighbors::comms::PeerNetworkComms;
31-
use crate::net::p2p::PeerNetwork;
31+
use crate::net::p2p::{DropSource, PeerNetwork};
3232
use crate::net::{
33-
DropReason, Error as NetError, GetNakamotoInvData, NackErrorCodes, NakamotoInvData,
34-
NeighborAddress, NeighborComms, NeighborKey, StacksMessage, StacksMessageType,
33+
DropNeighbor, DropReason, Error as NetError, GetNakamotoInvData, NackErrorCodes,
34+
NakamotoInvData, NeighborAddress, NeighborComms, NeighborKey, StacksMessage, StacksMessageType,
3535
};
3636
use crate::util_lib::db::Error as DBError;
3737

@@ -997,7 +997,12 @@ impl<NC: NeighborComms> NakamotoInvStateMachine<NC> {
997997
&naddr,
998998
&e
999999
);
1000-
self.comms.add_broken(network, &naddr);
1000+
self.comms.add_broken(
1001+
network,
1002+
&naddr,
1003+
DropReason::BrokenConnection(format!("Failed to finish inventory sync: {e}")),
1004+
DropSource::NakamotoInvStateMachine,
1005+
);
10011006
e
10021007
}) else {
10031008
continue;
@@ -1084,7 +1089,7 @@ impl PeerNetwork {
10841089
&mut self,
10851090
sortdb: &SortitionDB,
10861091
ibd: bool,
1087-
) -> (bool, Vec<NeighborKey>, Vec<NeighborKey>) {
1092+
) -> (bool, Vec<DropNeighbor>, Vec<DropNeighbor>) {
10881093
if self.inv_state_nakamoto.is_none() {
10891094
self.init_inv_sync_nakamoto();
10901095
}
@@ -1128,12 +1133,12 @@ impl PeerNetwork {
11281133

11291134
// disconnect and ban broken peers
11301135
for broken in broken_neighbors.into_iter() {
1131-
self.deregister_and_ban_neighbor(&broken, DropReason::BrokenConnection(None));
1136+
self.deregister_and_ban_neighbor(&broken.key, broken.reason, broken.source);
11321137
}
11331138

11341139
// disconnect from dead connections
11351140
for dead in dead_neighbors.into_iter() {
1136-
self.deregister_neighbor(&dead, DropReason::DeadConnection);
1141+
self.deregister_neighbor(&dead.key, dead.reason, dead.source);
11371142
}
11381143

11391144
learned

0 commit comments

Comments
 (0)