From 3979f62aa2d5d726378848c7c038b0d097f7a5e7 Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Fri, 12 Jan 2024 16:26:00 +0200 Subject: [PATCH] p2p: peer discouragement was added; peer eviction tests were refactored; ban tests were rewritten; PeermanagerConfig was moved to a separate file; some cleanup --- dns-server/src/crawler_p2p/crawler/mod.rs | 18 +- .../src/crawler_p2p/crawler/tests/mod.rs | 22 +- .../crawler_manager/tests/mock_manager.rs | 6 +- .../crawler_p2p/crawler_manager/tests/mod.rs | 10 +- dns-server/src/main.rs | 5 +- node-lib/src/config_files/mod.rs | 4 + node-lib/src/config_files/p2p.rs | 17 +- node-lib/src/options.rs | 4 +- .../src/block_announcement.rs | 3 +- p2p/p2p-test-utils/src/lib.rs | 16 + p2p/src/ban_config.rs | 54 + p2p/src/config.rs | 15 +- p2p/src/error.rs | 2 + p2p/src/lib.rs | 1 + p2p/src/peer_manager/config.rs | 141 ++ p2p/src/peer_manager/mod.rs | 248 +- p2p/src/peer_manager/peerdb/mod.rs | 68 +- p2p/src/peer_manager/peerdb/storage.rs | 9 + p2p/src/peer_manager/peerdb/storage_impl.rs | 35 +- p2p/src/peer_manager/peerdb/storage_load.rs | 12 +- p2p/src/peer_manager/peerdb/tests.rs | 104 +- p2p/src/peer_manager/peers_eviction/mod.rs | 40 +- p2p/src/peer_manager/peers_eviction/tests.rs | 2059 +++++++---------- .../tests/addr_list_response_caching.rs | 52 +- p2p/src/peer_manager/tests/addresses.rs | 3 +- p2p/src/peer_manager/tests/ban.rs | 893 ++++--- p2p/src/peer_manager/tests/connections.rs | 192 +- p2p/src/peer_manager/tests/discouragement.rs | 503 ++++ p2p/src/peer_manager/tests/eviction.rs | 77 +- p2p/src/peer_manager/tests/mod.rs | 1 + p2p/src/peer_manager/tests/peer_types.rs | 3 +- p2p/src/peer_manager/tests/ping.rs | 3 +- p2p/src/peer_manager/tests/utils.rs | 87 +- p2p/src/peer_manager/tests/whitelist.rs | 7 +- p2p/src/peer_manager_event.rs | 2 +- p2p/src/sync/tests/block_announcement.rs | 9 +- p2p/src/sync/tests/block_response.rs | 21 +- p2p/src/sync/tests/header_list_request.rs | 3 +- p2p/src/sync/tests/header_list_response.rs | 3 +- p2p/src/sync/tests/network_sync.rs | 6 +- p2p/src/sync/tests/tx_announcement.rs | 6 +- p2p/src/testing_utils.rs | 65 +- p2p/src/tests/helpers/mod.rs | 7 + p2p/src/tests/peer_discovery_on_stale_tip.rs | 5 +- wallet/wallet-node-client/tests/call_tests.rs | 3 +- wallet/wallet-test-node/src/lib.rs | 3 +- 46 files changed, 2866 insertions(+), 1981 deletions(-) create mode 100644 p2p/src/ban_config.rs create mode 100644 p2p/src/peer_manager/config.rs create mode 100644 p2p/src/peer_manager/tests/discouragement.rs diff --git a/dns-server/src/crawler_p2p/crawler/mod.rs b/dns-server/src/crawler_p2p/crawler/mod.rs index 1a8f2eb29f..b5cc7ce591 100644 --- a/dns-server/src/crawler_p2p/crawler/mod.rs +++ b/dns-server/src/crawler_p2p/crawler/mod.rs @@ -39,13 +39,13 @@ use common::{chain::ChainConfig, primitives::time::Time}; use crypto::random::{seq::IteratorRandom, Rng}; use logging::log; use p2p::{ - config::{BanDuration, BanThreshold}, error::P2pError, net::types::PeerInfo, peer_manager::{ADDR_RATE_BUCKET_SIZE, ADDR_RATE_INITIAL_SIZE, MAX_ADDR_RATE_PER_SECOND}, types::{bannable_address::BannableAddress, peer_id::PeerId, socket_address::SocketAddress}, utils::rate_limiter::RateLimiter, }; +use utils::make_config_setting; use crate::crawler_p2p::crawler::address_data::AddressStateTransitionTo; @@ -53,24 +53,16 @@ use self::address_data::{AddressData, AddressState}; /// How many outbound connection attempts can be made per heartbeat const MAX_CONNECTS_PER_HEARTBEAT: usize = 25; -const DEFAULT_BAN_THRESHOLD: u32 = 100; -const DEFAULT_BAN_DURATION: Duration = Duration::from_secs(60 * 60 * 24); -#[derive(Clone)] +make_config_setting!(BanThreshold, u32, 100); +make_config_setting!(BanDuration, Duration, Duration::from_secs(60 * 60 * 24)); + +#[derive(Default, Clone)] pub struct CrawlerConfig { pub ban_threshold: BanThreshold, pub ban_duration: BanDuration, } -impl Default for CrawlerConfig { - fn default() -> Self { - Self { - ban_threshold: BanThreshold::from(DEFAULT_BAN_THRESHOLD), - ban_duration: BanDuration::from(DEFAULT_BAN_DURATION), - } - } -} - /// The `Crawler` is the component that communicates with Mintlayer peers using p2p, /// and based on the results, commands the DNS server to add/remove ip addresses. /// The `Crawler` emits events that communicate whether addresses were reached or, diff --git a/dns-server/src/crawler_p2p/crawler/tests/mod.rs b/dns-server/src/crawler_p2p/crawler/tests/mod.rs index ac9ec6b53c..df3419f09e 100644 --- a/dns-server/src/crawler_p2p/crawler/tests/mod.rs +++ b/dns-server/src/crawler_p2p/crawler/tests/mod.rs @@ -27,7 +27,7 @@ use common::{ primitives::{time::Time, user_agent::mintlayer_core_user_agent}, }; use p2p::{ - config::{BanDuration, BanThreshold, NodeType}, + config::NodeType, error::{DialError, P2pError, ProtocolError}, net::types::PeerInfo, testing_utils::TEST_PROTOCOL_VERSION, @@ -312,8 +312,8 @@ fn ban_misbehaved_peer(#[case] seed: Seed) { let chain_config = common::chain::config::create_mainnet(); let mut crawler = test_crawler( CrawlerConfig { - ban_duration: BanDuration::new(BAN_DURATION), - ban_threshold: BanThreshold::new(ban_threshold), + ban_duration: BAN_DURATION.into(), + ban_threshold: ban_threshold.into(), }, BTreeSet::new(), BTreeMap::new(), @@ -442,8 +442,8 @@ fn ban_misbehaved_peers_with_same_address(#[case] seed: Seed) { let chain_config = common::chain::config::create_mainnet(); let mut crawler = test_crawler( CrawlerConfig { - ban_duration: BanDuration::new(BAN_DURATION), - ban_threshold: BanThreshold::new(ban_threshold), + ban_duration: BAN_DURATION.into(), + ban_threshold: ban_threshold.into(), }, BTreeSet::new(), BTreeMap::new(), @@ -589,8 +589,8 @@ fn ban_on_misbehavior_during_handshake(#[case] seed: Seed) { let chain_config = common::chain::config::create_mainnet(); let mut crawler = test_crawler( CrawlerConfig { - ban_duration: BanDuration::new(BAN_DURATION), - ban_threshold: BanThreshold::new(ban_threshold), + ban_duration: BAN_DURATION.into(), + ban_threshold: ban_threshold.into(), }, BTreeSet::new(), BTreeMap::new(), @@ -665,8 +665,8 @@ fn no_ban_on_connection_error(#[case] seed: Seed) { let chain_config = common::chain::config::create_mainnet(); let mut crawler = test_crawler( CrawlerConfig { - ban_duration: BanDuration::new(BAN_DURATION), - ban_threshold: BanThreshold::new(ban_threshold), + ban_duration: BAN_DURATION.into(), + ban_threshold: ban_threshold.into(), }, BTreeSet::new(), BTreeMap::new(), @@ -712,8 +712,8 @@ const BAN_THRESHOLD: u32 = 100; fn make_config() -> CrawlerConfig { CrawlerConfig { - ban_duration: BanDuration::new(BAN_DURATION), - ban_threshold: BanThreshold::new(BAN_THRESHOLD), + ban_duration: BAN_DURATION.into(), + ban_threshold: BAN_THRESHOLD.into(), } } diff --git a/dns-server/src/crawler_p2p/crawler_manager/tests/mock_manager.rs b/dns-server/src/crawler_p2p/crawler_manager/tests/mock_manager.rs index 85d9435671..bf16f9ae90 100644 --- a/dns-server/src/crawler_p2p/crawler_manager/tests/mock_manager.rs +++ b/dns-server/src/crawler_p2p/crawler_manager/tests/mock_manager.rs @@ -35,7 +35,7 @@ use common::{ time_getter::TimeGetter, }; use p2p::{ - config::{BanDuration, BanThreshold, NodeType, P2pConfig}, + config::{NodeType, P2pConfig}, error::{DialError, P2pError}, message::{AnnounceAddrRequest, PeerManagerMessage}, net::{ @@ -295,8 +295,8 @@ pub fn test_crawler( default_p2p_port: 3031, }; let crawler_config = CrawlerConfig { - ban_duration: BanDuration::default(), - ban_threshold: BanThreshold::default(), + ban_duration: Default::default(), + ban_threshold: Default::default(), }; let state = MockStateRef { diff --git a/dns-server/src/crawler_p2p/crawler_manager/tests/mod.rs b/dns-server/src/crawler_p2p/crawler_manager/tests/mod.rs index 61c3768930..4ac42cdc58 100644 --- a/dns-server/src/crawler_p2p/crawler_manager/tests/mod.rs +++ b/dns-server/src/crawler_p2p/crawler_manager/tests/mod.rs @@ -19,16 +19,18 @@ use std::time::Duration; use chainstate::ban_score::BanScore; use p2p::{ - config::{BanDuration, BanThreshold}, error::{P2pError, ProtocolError}, types::socket_address::SocketAddress, }; use p2p_test_utils::{expect_no_recv, expect_recv}; use crate::{ - crawler_p2p::crawler_manager::tests::mock_manager::{ - advance_time, assert_banned_addresses, assert_known_addresses, test_crawler, - ErraticNodeConnectError, + crawler_p2p::{ + crawler::{BanDuration, BanThreshold}, + crawler_manager::tests::mock_manager::{ + advance_time, assert_banned_addresses, assert_known_addresses, test_crawler, + ErraticNodeConnectError, + }, }, dns_server::DnsServerCommand, }; diff --git a/dns-server/src/main.rs b/dns-server/src/main.rs index 5004300797..03a55ed30f 100644 --- a/dns-server/src/main.rs +++ b/dns-server/src/main.rs @@ -58,8 +58,9 @@ async fn run(config: Arc) -> Result P2pConfigFile { max_inbound_connections, ban_threshold, ban_duration, + discouragement_threshold, + discouragement_duration, max_clock_diff, outbound_connection_timeout, ping_check_period, @@ -212,6 +214,8 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile { max_inbound_connections, ban_threshold, ban_duration, + discouragement_threshold, + discouragement_duration, max_clock_diff, outbound_connection_timeout, ping_check_period, diff --git a/node-lib/src/config_files/p2p.rs b/node-lib/src/config_files/p2p.rs index 8c85d58b00..7d9fe36892 100644 --- a/node-lib/src/config_files/p2p.rs +++ b/node-lib/src/config_files/p2p.rs @@ -19,8 +19,9 @@ use common::primitives::user_agent::mintlayer_core_user_agent; use serde::{Deserialize, Serialize}; use p2p::{ + ban_config::BanConfig, config::{NodeType, P2pConfig}, - peer_manager::PeerManagerConfig, + peer_manager::config::PeerManagerConfig, types::ip_or_socket_address::IpOrSocketAddress, }; @@ -76,6 +77,10 @@ pub struct P2pConfigFile { pub ban_threshold: Option, /// Duration of bans in seconds. pub ban_duration: Option, + /// The score threshold after which a peer becomes discouraged. + pub discouragement_threshold: Option, + /// Duration of discouragement in seconds. + pub discouragement_duration: Option, /// Maximum acceptable time difference between this node and the remote peer (in seconds). /// If a large difference is detected, the peer will be disconnected. pub max_clock_diff: Option, @@ -106,6 +111,8 @@ impl From for P2pConfig { max_inbound_connections, ban_threshold, ban_duration, + discouragement_threshold, + discouragement_duration, max_clock_diff, outbound_connection_timeout, ping_check_period, @@ -122,8 +129,12 @@ impl From for P2pConfig { boot_nodes: boot_nodes.unwrap_or_default(), reserved_nodes: reserved_nodes.unwrap_or_default(), whitelisted_addresses: whitelisted_addresses.unwrap_or_default(), - ban_threshold: ban_threshold.into(), - ban_duration: ban_duration.map(Duration::from_secs).into(), + ban_config: BanConfig { + ban_threshold: ban_threshold.into(), + ban_duration: ban_duration.map(Duration::from_secs).into(), + discouragement_threshold: discouragement_threshold.into(), + discouragement_duration: discouragement_duration.map(Duration::from_secs).into(), + }, max_clock_diff: max_clock_diff.map(Duration::from_secs).into(), outbound_connection_timeout: outbound_connection_timeout .map(|t| Duration::from_secs(t.into())) diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 8e6a36d625..4c34ecef92 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -136,7 +136,9 @@ pub struct RunOptions { #[clap(long)] pub p2p_max_inbound_connections: Option, - /// The p2p score threshold after which a peer is baned. + // TODO: add all the options related to banning/discouragement thresholds and durations, + // for completeness. + /// The p2p score threshold after which a peer is banned. #[clap(long)] pub p2p_ban_threshold: Option, diff --git a/p2p/backend-test-suite/src/block_announcement.rs b/p2p/backend-test-suite/src/block_announcement.rs index 74b70a02df..b6fe969d1e 100644 --- a/p2p/backend-test-suite/src/block_announcement.rs +++ b/p2p/backend-test-suite/src/block_announcement.rs @@ -179,8 +179,7 @@ where boot_nodes: Vec::new(), reserved_nodes: Vec::new(), whitelisted_addresses: Default::default(), - ban_threshold: Default::default(), - ban_duration: Default::default(), + ban_config: Default::default(), outbound_connection_timeout: Default::default(), ping_check_period: Default::default(), ping_timeout: Default::default(), diff --git a/p2p/p2p-test-utils/src/lib.rs b/p2p/p2p-test-utils/src/lib.rs index ed2a08f7ce..9aab799d3f 100644 --- a/p2p/p2p-test-utils/src/lib.rs +++ b/p2p/p2p-test-utils/src/lib.rs @@ -221,6 +221,7 @@ macro_rules! expect_no_recv { }; } +/// Wait until the specified value is received from the channel. pub async fn wait_for_recv(receiver: &mut mpsc::UnboundedReceiver, value: &T) { let wait_loop = async { loop { @@ -232,3 +233,18 @@ pub async fn wait_for_recv(receiver: &mut mpsc::UnboundedReceiver, val expect_future_val!(wait_loop); } + +/// Wait until the sender stops putting messages into the channel. +pub async fn wait_for_no_recv(receiver: &mut mpsc::UnboundedReceiver) { + let wait_loop = async { + loop { + let wait_result = + tokio::time::timeout(Duration::from_millis(100), receiver.recv()).await; + if wait_result.is_err() { + break; + } + } + }; + + expect_future_val!(wait_loop); +} diff --git a/p2p/src/ban_config.rs b/p2p/src/ban_config.rs new file mode 100644 index 0000000000..e2a09652c6 --- /dev/null +++ b/p2p/src/ban_config.rs @@ -0,0 +1,54 @@ +// Copyright (c) 2021-2024 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::time::Duration; + +use utils::make_config_setting; + +make_config_setting!(BanThreshold, u32, 100); +// TODO: initially, the ban duration was 24h, but later it was changed to 30 min because automatic +// banning can be dangerous. E.g. it's possible for an attacker to force the node to use malicious +// Tor exit nodes by making it ban all honest ones. Using a smaller interval seemed less dangerous, +// so we chose 30 min. But even automatic disconnection of seemingly misbehaving peers may lead +// to network splitting; e.g. if in a future version some previously incorrect behavior becomes +// valid, the network may be split between old and new nodes, because the old ones would ban peers +// that exhibit such behavior. Also, 30 min doesn't make much sense for manual banning. So, we +// should probably get rid of automatic banning and return the old duration for the manual banning. +// But if we decide to keep the auto-ban functionality: +// a) the config names should be changed to AutoBanDuration, AutoBanThreshold; +// b) manual banning should have a separate ManualBanDuration; or event better, the ban duration +// should be specified by the user via RPC and the config should become ManualBanDefaultDuration. +// c) there should be the ability to turn auto-banning off, e.g. by setting the threshold to 0; +// by default, auto banning should be turned off. +make_config_setting!(BanDuration, Duration, Duration::from_secs(60 * 30)); +make_config_setting!(DiscouragementThreshold, u32, 100); +make_config_setting!( + DiscouragementDuration, + Duration, + Duration::from_secs(60 * 60 * 24) +); + +/// Settings related to banning and discouragement. +#[derive(Default, Debug, Clone)] +pub struct BanConfig { + /// The score threshold after which a peer is banned. + pub ban_threshold: BanThreshold, + /// The duration of a ban. + pub ban_duration: BanDuration, + /// The score threshold after which a peer becomes discouraged. + pub discouragement_threshold: DiscouragementThreshold, + /// The duration of discouragement. + pub discouragement_duration: DiscouragementDuration, +} diff --git a/p2p/src/config.rs b/p2p/src/config.rs index 68152c556f..8ca2b012d4 100644 --- a/p2p/src/config.rs +++ b/p2p/src/config.rs @@ -21,15 +21,12 @@ use p2p_types::ip_or_socket_address::IpOrSocketAddress; use utils::make_config_setting; use crate::{ + ban_config::BanConfig, net::types::services::{Service, Services}, - peer_manager::PeerManagerConfig, + peer_manager::config::PeerManagerConfig, protocol::ProtocolConfig, }; -make_config_setting!(BanThreshold, u32, 100); -// BanDuration is only 30 mins as a compromise between banning for a longer period of time -// and not banning at all with alternative approaches such as discouragement -make_config_setting!(BanDuration, Duration, Duration::from_secs(60 * 30)); make_config_setting!(OutboundConnectionTimeout, Duration, Duration::from_secs(10)); make_config_setting!(NodeTypeSetting, NodeType, NodeType::Full); make_config_setting!(AllowDiscoverPrivateIps, bool, false); @@ -85,10 +82,8 @@ pub struct P2pConfig { pub reserved_nodes: Vec, /// Optional list of whitelisted addresses. Such addresses cannot be automatically banned. pub whitelisted_addresses: Vec, - /// The score threshold after which a peer is banned. - pub ban_threshold: BanThreshold, - /// Duration of bans in seconds. - pub ban_duration: BanDuration, + /// Settings related to banning and discouragement. + pub ban_config: BanConfig, /// The outbound connection timeout value in seconds. pub outbound_connection_timeout: OutboundConnectionTimeout, /// How often send ping requests to peers @@ -108,7 +103,7 @@ pub struct P2pConfig { pub user_agent: UserAgent, /// A timeout after which a peer is disconnected. pub sync_stalling_timeout: SyncStallingTimeout, - /// Various limits used internally by the peer manager; these should only be overridden in tests. + /// Various settings used internally by the peer manager. pub peer_manager_config: PeerManagerConfig, /// Various limits related to the protocol; these should only be overridden in tests. pub protocol_config: ProtocolConfig, diff --git a/p2p/src/error.rs b/p2p/src/error.rs index 6787982450..77cbcefd59 100644 --- a/p2p/src/error.rs +++ b/p2p/src/error.rs @@ -88,6 +88,8 @@ pub enum PeerError { }, #[error("Address {0} is banned")] BannedAddress(String), + #[error("Address {0} is discouraged")] + DiscouragedAddress(String), #[error("PeerManager has too many peers")] TooManyPeers, #[error("Connection to address {0} already pending")] diff --git a/p2p/src/lib.rs b/p2p/src/lib.rs index 77856573f2..2843a3aea5 100644 --- a/p2p/src/lib.rs +++ b/p2p/src/lib.rs @@ -13,6 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +pub mod ban_config; pub mod config; pub mod error; pub mod interface; diff --git a/p2p/src/peer_manager/config.rs b/p2p/src/peer_manager/config.rs new file mode 100644 index 0000000000..739914cbbe --- /dev/null +++ b/p2p/src/peer_manager/config.rs @@ -0,0 +1,141 @@ +// Copyright (c) 2021-2024 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::time::Duration; + +use utils::make_config_setting; + +use super::{ + peerdb::config::PeerDbConfig, + peers_eviction::{ + OutboundBlockRelayConnectionMinAge, OutboundFullRelayConnectionMinAge, + PreservedInboundCountAddressGroup, PreservedInboundCountNewBlocks, + PreservedInboundCountNewTransactions, PreservedInboundCountPing, + }, +}; + +make_config_setting!(MaxInboundConnections, usize, 128); +make_config_setting!(OutboundFullRelayCount, usize, 8); +make_config_setting!(OutboundFullRelayExtraCount, usize, 1); +make_config_setting!(OutboundBlockRelayCount, usize, 2); +make_config_setting!(OutboundBlockRelayExtraCount, usize, 1); +make_config_setting!(StaleTipTimeDiff, Duration, Duration::from_secs(30 * 60)); +make_config_setting!(MainLoopTickInterval, Duration, Duration::from_secs(1)); +make_config_setting!( + FeelerConnectionsInterval, + Duration, + Duration::from_secs(2 * 60) +); +make_config_setting!(EnableFeelerConnections, bool, true); +make_config_setting!(ForceDnsQueryIfNoGlobalAddressesKnown, bool, false); +make_config_setting!(AllowSameIpConnections, bool, false); + +// TODO: this name is too generic, because not all peer manager settings are contained here. +// PeerManagerInternalConfig would be a better name. +// Alternatively, we may want to actually put all peer manager settings here. If we do this, +// it might be better to revise the entire structure of p2p config and make it more hierarchical. +// E.g. we may have separate structs for backend, sync manager and peer manager settings at the +// top level; then, eviction settings in PeerManagerConfig may go to their separate struct, etc. +#[derive(Default, Debug, Clone)] +pub struct PeerManagerConfig { + /// Maximum allowed number of inbound connections. + pub max_inbound_connections: MaxInboundConnections, + + /// The number of inbound peers to preserve based on the address group. + pub preserved_inbound_count_address_group: PreservedInboundCountAddressGroup, + /// The number of inbound peers to preserve based on ping. + pub preserved_inbound_count_ping: PreservedInboundCountPing, + /// The number of inbound peers to preserve based on the last time they sent us new blocks. + pub preserved_inbound_count_new_blocks: PreservedInboundCountNewBlocks, + /// The number of inbound peers to preserve based on the last time they sent us new transactions. + pub preserved_inbound_count_new_transactions: PreservedInboundCountNewTransactions, + + /// The desired maximum number of full relay outbound connections. + /// Note that this limit may be exceeded temporarily by up to outbound_full_relay_extra_count + /// connections. + pub outbound_full_relay_count: OutboundFullRelayCount, + /// The number of extra full relay connections that we may establish when a stale tip + /// is detected. + pub outbound_full_relay_extra_count: OutboundFullRelayExtraCount, + + /// The desired maximum number of block relay outbound connections. + /// Note that this limit may be exceeded temporarily by up to outbound_block_relay_extra_count + /// connections. + pub outbound_block_relay_count: OutboundBlockRelayCount, + /// The number of extra block relay connections that we will establish and evict regularly. + pub outbound_block_relay_extra_count: OutboundBlockRelayExtraCount, + + /// Outbound block relay connections younger than this age will not be taken into account + /// during eviction. + /// Note that extra block relay connections are established and evicted on a regular basis + /// during normal operation. So, this interval basically determines how often those extra + /// connections will come and go. + pub outbound_block_relay_connection_min_age: OutboundBlockRelayConnectionMinAge, + /// Outbound full relay connections younger than this age will not be taken into account + /// during eviction. + /// Note that extra full relay connections are established if the current tip becomes stale. + pub outbound_full_relay_connection_min_age: OutboundFullRelayConnectionMinAge, + + /// The time after which the tip will be considered stale. + pub stale_tip_time_diff: StaleTipTimeDiff, + + /// How often the main loop should be woken up when no other events occur. + pub main_loop_tick_interval: MainLoopTickInterval, + + /// Whether feeler connections should be enabled. + pub enable_feeler_connections: EnableFeelerConnections, + /// The minimum interval between feeler connections. + pub feeler_connections_interval: FeelerConnectionsInterval, + + /// If true, the node will perform an early dns query if the peer db doesn't contain + /// any global addresses at startup (more precisely, the ones for which is_global_unicast_ip + /// returns true). + /// + /// Note that this is mainly needed to speed up the startup of the test + /// nodes in build-tools/p2p-test. Those nodes always start with a boot_nodes + /// list that contains addresses of their siblings, which are always some private ips; + /// therefore, they all will have peers from the beginning, which will prevent normal + /// dns queries, but will be unable to obtain blocks until stale_tip_time_diff has elapsed, + /// which is 30 min by default. Setting this option to true will force the peer manager + /// to perform an early dns query in such a situation. + pub force_dns_query_if_no_global_addresses_known: ForceDnsQueryIfNoGlobalAddressesKnown, + + /// If true, multiple connections to the same ip address (but using a different port) will + /// always be allowed. + /// + /// Normally, the peer manager won't always allow connecting to the same ip using a different + /// port; e.g. if an inbound connection exists, a new outbound connection to the same ip + /// will only be allowed if it's manual. This may be inconvenient for some (legacy) unit tests, + /// so they can set this option to true to override this behavior. + /// TODO: consider rewriting tests that need this option and remove it. + pub allow_same_ip_connections: AllowSameIpConnections, + + /// Peer db configuration. + pub peerdb_config: PeerDbConfig, +} + +impl PeerManagerConfig { + pub fn total_preserved_inbound_count(&self) -> usize { + *self.preserved_inbound_count_address_group + + *self.preserved_inbound_count_ping + + *self.preserved_inbound_count_new_blocks + + *self.preserved_inbound_count_new_transactions + } + + /// The desired maximum number of automatic outbound connections. + pub fn outbound_full_and_block_relay_count(&self) -> usize { + *self.outbound_full_relay_count + *self.outbound_block_relay_count + } +} diff --git a/p2p/src/peer_manager/mod.rs b/p2p/src/peer_manager/mod.rs index 278efb3429..6563795c10 100644 --- a/p2p/src/peer_manager/mod.rs +++ b/p2p/src/peer_manager/mod.rs @@ -17,6 +17,7 @@ mod addr_list_response_cache; pub mod address_groups; +pub mod config; pub mod dns_seed; pub mod peer_context; pub mod peerdb; @@ -45,10 +46,7 @@ use common::{ }; use crypto::random::{make_pseudo_rng, seq::IteratorRandom, Rng}; use logging::log; -use utils::{ - bloom_filters::rolling_bloom_filter::RollingBloomFilter, ensure, make_config_setting, - set_flag::SetFlag, -}; +use utils::{bloom_filters::rolling_bloom_filter::RollingBloomFilter, ensure, set_flag::SetFlag}; use crate::{ config::P2pConfig, @@ -81,122 +79,9 @@ use self::{ address_groups::AddressGroup, dns_seed::{DefaultDnsSeed, DnsSeed}, peer_context::{PeerContext, SentPing}, - peerdb::{config::PeerDbConfig, storage::PeerDbStorage}, - peers_eviction::{ - OutboundBlockRelayConnectionMinAge, OutboundFullRelayConnectionMinAge, - PreservedInboundCountAddressGroup, PreservedInboundCountNewBlocks, - PreservedInboundCountNewTransactions, PreservedInboundCountPing, - }, + peerdb::storage::PeerDbStorage, }; -#[derive(Default, Debug, Clone)] -pub struct PeerManagerConfig { - /// Maximum allowed number of inbound connections. - pub max_inbound_connections: MaxInboundConnections, - - /// The number of inbound peers to preserve based on the address group. - pub preserved_inbound_count_address_group: PreservedInboundCountAddressGroup, - /// The number of inbound peers to preserve based on ping. - pub preserved_inbound_count_ping: PreservedInboundCountPing, - /// The number of inbound peers to preserve based on the last time they sent us new blocks. - pub preserved_inbound_count_new_blocks: PreservedInboundCountNewBlocks, - /// The number of inbound peers to preserve based on the last time they sent us new transactions. - pub preserved_inbound_count_new_transactions: PreservedInboundCountNewTransactions, - - /// The desired maximum number of full relay outbound connections. - /// Note that this limit may be exceeded temporarily by up to outbound_full_relay_extra_count - /// connections. - pub outbound_full_relay_count: OutboundFullRelayCount, - /// The number of extra full relay connections that we may establish when a stale tip - /// is detected. - pub outbound_full_relay_extra_count: OutboundFullRelayExtraCount, - - /// The desired maximum number of block relay outbound connections. - /// Note that this limit may be exceeded temporarily by up to outbound_block_relay_extra_count - /// connections. - pub outbound_block_relay_count: OutboundBlockRelayCount, - /// The number of extra block relay connections that we will establish and evict regularly. - pub outbound_block_relay_extra_count: OutboundBlockRelayExtraCount, - - /// Outbound block relay connections younger than this age will not be taken into account - /// during eviction. - /// Note that extra block relay connections are established and evicted on a regular basis - /// during normal operation. So, this interval basically determines how often those extra - /// connections will come and go. - pub outbound_block_relay_connection_min_age: OutboundBlockRelayConnectionMinAge, - /// Outbound full relay connections younger than this age will not be taken into account - /// during eviction. - /// Note that extra full relay connections are established if the current tip becomes stale. - pub outbound_full_relay_connection_min_age: OutboundFullRelayConnectionMinAge, - - /// The time after which the tip will be considered stale. - pub stale_tip_time_diff: StaleTipTimeDiff, - - /// How often the main loop should be woken up when no other events occur. - pub main_loop_tick_interval: MainLoopTickInterval, - - /// Whether feeler connections should be enabled. - pub enable_feeler_connections: EnableFeelerConnections, - /// The minimum interval between feeler connections. - pub feeler_connections_interval: FeelerConnectionsInterval, - - /// If true, the node will perform an early dns query if the peer db doesn't contain - /// any global addresses at startup (more precisely, the ones for which is_global_unicast_ip - /// returns true). - /// - /// Note that this is mainly needed to speed up the startup of the test - /// nodes in build-tools/p2p-test. Those nodes always start with a boot_nodes - /// list that contains addresses of their siblings, which are always some private ips; - /// therefore, they all will have peers from the beginning, which will prevent normal - /// dns queries, but will be unable to obtain blocks until stale_tip_time_diff has elapsed, - /// which is 30 min by default. Setting this option to true will force the peer manager - /// to perform an early dns query in such a situation. - pub force_dns_query_if_no_global_addresses_known: ForceDnsQueryIfNoGlobalAddressesKnown, - - /// If true, multiple connections to the same ip address (but using a different port) will - /// always be allowed. - /// - /// Normally, the peer manager won't always allow connecting to the same ip using a different - /// port; e.g. if an inbound connection exists, a new outbound connection to the same ip - /// will only be allowed if it's manual. This may be inconvenient for some (legacy) unit tests, - /// so they can set this option to true to override this behavior. - /// TODO: consider rewriting tests that need this option and remove it. - pub allow_same_ip_connections: AllowSameIpConnections, - - /// Peer db configuration. - pub peerdb_config: PeerDbConfig, -} - -impl PeerManagerConfig { - pub fn total_preserved_inbound_count(&self) -> usize { - *self.preserved_inbound_count_address_group - + *self.preserved_inbound_count_ping - + *self.preserved_inbound_count_new_blocks - + *self.preserved_inbound_count_new_transactions - } - - /// The desired maximum number of automatic outbound connections. - pub fn outbound_full_and_block_relay_count(&self) -> usize { - *self.outbound_full_relay_count + *self.outbound_block_relay_count - } -} - -make_config_setting!(MaxInboundConnections, usize, 128); -make_config_setting!(OutboundFullRelayCount, usize, 8); -make_config_setting!(OutboundFullRelayExtraCount, usize, 1); -make_config_setting!(OutboundBlockRelayCount, usize, 2); -make_config_setting!(OutboundBlockRelayExtraCount, usize, 1); -make_config_setting!(StaleTipTimeDiff, Duration, Duration::from_secs(30 * 60)); -make_config_setting!(MainLoopTickInterval, Duration, Duration::from_secs(1)); -make_config_setting!( - FeelerConnectionsInterval, - Duration, - Duration::from_secs(2 * 60) -); -make_config_setting!(EnableFeelerConnections, bool, true); -make_config_setting!(ForceDnsQueryIfNoGlobalAddressesKnown, bool, false); -make_config_setting!(AllowSameIpConnections, bool, false); - /// Lower bound for how often [`PeerManager::heartbeat()`] is called pub const HEARTBEAT_INTERVAL_MIN: Duration = Duration::from_secs(5); /// Upper bound for how often [`PeerManager::heartbeat()`] is called @@ -544,13 +429,8 @@ where /// Adjust peer score /// - /// If the peer is known, update its existing peer score and report - /// if it should be disconnected when score reached the threshold. - /// Unknown peers are reported as to be disconnected. - /// - /// If peer is banned, it is removed from the connected peers - /// and its address is marked as banned. - fn adjust_peer_score(&mut self, peer_id: PeerId, score: u32) { + /// Discourage and/or ban the peer if the score reaches the corresponding threshold. + fn adjust_peer_score(&mut self, peer_id: PeerId, score_adjustment: u32) { let peer = match self.peers.get(&peer_id) { Some(peer) => peer, None => return, @@ -558,7 +438,7 @@ where if self.is_whitelisted_node(peer.peer_role, &peer.peer_address) { log::info!( - "Not adjusting peer score for the whitelisted peer {peer_id}, adjustment {score}", + "Not adjusting peer score for the whitelisted peer {peer_id}, adjustment {score_adjustment}", ); return; } @@ -568,27 +448,31 @@ where None => return, }; - peer.score = peer.score.saturating_add(score); + let new_score = peer.score.saturating_add(score_adjustment); + let peer_address = peer.peer_address.as_bannable(); + peer.score = new_score; log::info!( - "Adjusting peer score for peer {peer_id}, adjustment {score}, new score {}", - peer.score + "Adjusting peer score for peer {peer_id}, adjustment {score_adjustment}, new score {new_score}", ); if let Some(o) = self.observer.as_mut() { o.on_peer_ban_score_adjustment(peer.peer_address, peer.score) } - if peer.score >= *self.p2p_config.ban_threshold { - let address = peer.peer_address.as_bannable(); - self.ban(address); + if new_score >= *self.p2p_config.ban_config.ban_threshold { + self.ban(peer_address); + } + + if new_score >= *self.p2p_config.ban_config.discouragement_threshold { + self.discourage(peer_address); } } /// Adjust peer score after a failed handshake. /// /// Note that currently intermediate scores are not stored in the peer db, so this call will - /// only make any effect if the passed score is bigger than the ban threshold. + /// only make any effect if the passed score is bigger than the threshold. fn adjust_peer_score_on_failed_handshake(&mut self, peer_address: SocketAddress, score: u32) { let whitelisted_node = self.pending_outbound_connects @@ -610,10 +494,15 @@ where o.on_peer_ban_score_adjustment(peer_address, score); } - if score >= *self.p2p_config.ban_threshold { + if score >= *self.p2p_config.ban_config.ban_threshold { let address = peer_address.as_bannable(); self.ban(address); } + + if score >= *self.p2p_config.ban_config.discouragement_threshold { + let address = peer_address.as_bannable(); + self.discourage(address); + } } fn ban(&mut self, address: BannableAddress) { @@ -629,7 +518,11 @@ where }) .collect::>(); - log::info!("Ban {:?}, disconnect peers: {:?}", address, to_disconnect); + log::info!( + "Banning {:?}, the following peers will be disconnected: {:?}", + address, + to_disconnect + ); self.peerdb.ban(address); @@ -642,6 +535,16 @@ where } } + fn discourage(&mut self, address: BannableAddress) { + log::info!("Discouraging {address:?}"); + + self.peerdb.discourage(address); + + if let Some(o) = self.observer.as_mut() { + o.on_peer_discouragement(address); + } + } + /// Try to initiate a new outbound connection /// /// This function doesn't block on the call but sends a command to the @@ -661,11 +564,17 @@ where self.maybe_reject_because_already_connected(&address, peer_role)?; let bannable_address = address.as_bannable(); + let is_reserved = self.peerdb.is_reserved_node(&address); + let is_banned = self.peerdb.is_address_banned(&bannable_address); + let is_discouraged = self.peerdb.is_address_discouraged(&bannable_address); ensure!( - !self.peerdb.is_address_banned(&bannable_address) - || self.peerdb.is_reserved_node(&address), + !is_banned || is_reserved, P2pError::PeerError(PeerError::BannedAddress(address.to_string())), ); + ensure!( + !is_discouraged || is_reserved, + P2pError::PeerError(PeerError::DiscouragedAddress(address.to_string())), + ); self.peer_connectivity_handle.connect(address, local_services_override)?; @@ -789,7 +698,11 @@ where // Ok and for outbound ones we've already called it in try_connect. But new connections // might have appeared since try_connect was called, so the call below is not redundant. self.maybe_reject_because_already_connected(address, peer_role)?; - + + // Note: for outbound connections, ban and discouragement statuses have already been + // checked in try_connect, so when we do the checks here, they'll only work for + // inbound connections. And since inbound connections from discouraged addresses are + // allowed, we only check the banned status here. ensure!( !self.peerdb.is_address_banned(&address.as_bannable()), P2pError::PeerError(PeerError::BannedAddress(address.to_string())), @@ -853,7 +766,13 @@ where && !self.pending_disconnects.contains_key(&peer.info.peer_id) }) .map(|peer| { - peers_eviction::EvictionCandidate::new(peer, &self.peer_eviction_random_state, now) + let addr = peer.peer_address.as_bannable(); + peers_eviction::EvictionCandidate::new( + peer, + &self.peer_eviction_random_state, + now, + self.peerdb.is_address_banned_or_discouraged(&addr), + ) }) .collect() } @@ -865,6 +784,7 @@ where if let Some(peer_id) = peers_eviction::select_for_eviction_inbound( self.eviction_candidates(PeerRole::Inbound), &self.p2p_config.peer_manager_config, + &mut make_pseudo_rng(), ) { log::info!("inbound peer {peer_id} is selected for eviction"); self.disconnect(peer_id, PeerDisconnectionDbAction::Keep, None); @@ -880,6 +800,7 @@ where self.eviction_candidates(PeerRole::OutboundBlockRelay), &self.p2p_config.peer_manager_config, self.time_getter.get_time(), + &mut make_pseudo_rng(), ) { log::info!("block relay peer {peer_id} is selected for eviction"); self.disconnect(peer_id, PeerDisconnectionDbAction::Keep, None); @@ -892,6 +813,7 @@ where self.eviction_candidates(PeerRole::OutboundFullRelay), &self.p2p_config.peer_manager_config, self.time_getter.get_time(), + &mut make_pseudo_rng(), ) { log::info!("full relay peer {peer_id} is selected for eviction"); self.disconnect(peer_id, PeerDisconnectionDbAction::Keep, None); @@ -1227,7 +1149,7 @@ where /// establish new connections. After that it updates the peer scores and discards any records /// that no longer need to be stored. fn heartbeat(&mut self) { - // Expired banned addresses are dropped here, keep this call! + // Expired banned and discouraged addresses are dropped here. self.peerdb.heartbeat(); self.establish_new_connections(); @@ -1360,7 +1282,9 @@ where && cur_feeler_conn_count == 0 && now >= self.next_feeler_connection_time { - if let Some(address) = self.peerdb.select_non_reserved_address_from_new_addr_table() { + if let Some(address) = + self.peerdb.select_non_reserved_outbound_address_from_new_addr_table() + { self.connect(address, OutboundConnectType::Feeler); self.next_feeler_connection_time = Self::choose_next_feeler_connection_time(&self.p2p_config, now); @@ -1399,13 +1323,15 @@ where self.peerdb.peer_discovered(address); - let peer_ids = self - .subscribed_to_peer_addresses - .iter() - .cloned() - .choose_multiple(&mut make_pseudo_rng(), PEER_ADDRESS_RESEND_COUNT); - for new_peer_id in peer_ids { - self.announce_address(new_peer_id, address); + if !self.peerdb.is_address_banned_or_discouraged(&address.as_bannable()) { + let peer_ids = self + .subscribed_to_peer_addresses + .iter() + .cloned() + .choose_multiple(&mut make_pseudo_rng(), PEER_ADDRESS_RESEND_COUNT); + for new_peer_id in peer_ids { + self.announce_address(new_peer_id, address); + } } } } @@ -1428,10 +1354,23 @@ where .get_or_create(peer, now, || { self.peerdb .known_addresses() - .map(SocketAddress::as_peer_address) - .filter(|address| Self::is_peer_address_valid(address, &self.p2p_config)) + .filter_map(|address| { + let peer_addr = address.as_peer_address(); + let bannable_addr = address.as_bannable(); + if Self::is_peer_address_valid(&peer_addr, &self.p2p_config) + && !self.peerdb.is_address_banned_or_discouraged(&bannable_addr) + { + Some(peer_addr) + } else { + None + } + }) .choose_multiple(&mut make_pseudo_rng(), max_addr_count) }) + // Note: some of the addresses may have become banned or discouraged after they've been + // cached. It's not clear whether it's better to filter them out here, which will + // reveal to peers what addresses we've ban or discouraged, or keep them as is. + // But it's probably not that important. .clone(); assert!(addresses.len() <= max_addr_count); @@ -2122,6 +2061,7 @@ where pub trait Observer { fn on_peer_ban_score_adjustment(&mut self, address: SocketAddress, new_score: u32); fn on_peer_ban(&mut self, address: BannableAddress); + fn on_peer_discouragement(&mut self, address: BannableAddress); // This will be called at the end of "heartbeat" function. fn on_heartbeat(&mut self); // This will be called for both incoming and outgoing connections. @@ -2133,10 +2073,10 @@ pub trait PeerManagerInterface { fn peers(&self) -> &BTreeMap; #[cfg(test)] - fn peerdb(&self) -> &dyn peerdb::PeerDbInterface; + fn peer_db(&self) -> &dyn peerdb::PeerDbInterface; #[cfg(test)] - fn peerdb_mut(&mut self) -> &mut dyn peerdb::PeerDbInterface; + fn peer_db_mut(&mut self) -> &mut dyn peerdb::PeerDbInterface; } impl PeerManagerInterface for PeerManager @@ -2151,12 +2091,12 @@ where } #[cfg(test)] - fn peerdb(&self) -> &dyn peerdb::PeerDbInterface { + fn peer_db(&self) -> &dyn peerdb::PeerDbInterface { &self.peerdb } #[cfg(test)] - fn peerdb_mut(&mut self) -> &mut dyn peerdb::PeerDbInterface { + fn peer_db_mut(&mut self) -> &mut dyn peerdb::PeerDbInterface { &mut self.peerdb } } diff --git a/p2p/src/peer_manager/peerdb/mod.rs b/p2p/src/peer_manager/peerdb/mod.rs index f6268f2be7..6e327376df 100644 --- a/p2p/src/peer_manager/peerdb/mod.rs +++ b/p2p/src/peer_manager/peerdb/mod.rs @@ -58,7 +58,7 @@ use super::{ }; pub use storage::StorageVersion; -pub use storage_load::open_storage; +pub use storage_load::{open_storage, CURRENT_STORAGE_VERSION}; pub struct PeerDb { /// P2P configuration @@ -82,6 +82,9 @@ pub struct PeerDb { /// Banned addresses along with the ban expiration time. banned_addresses: BTreeMap, + /// Discouraged addresses along with the discouragement expiration time. + discouraged_addresses: BTreeMap, + /// Anchor addresses anchor_addresses: BTreeSet, @@ -103,6 +106,7 @@ impl PeerDb { let LoadedStorage { known_addresses, banned_addresses, + discouraged_addresses, anchor_addresses, salt, } = LoadedStorage::load_storage(&storage, &p2p_config.peer_manager_config.peerdb_config)?; @@ -177,6 +181,7 @@ impl PeerDb { reserved_nodes, address_tables, banned_addresses, + discouraged_addresses, anchor_addresses, p2p_config, time_getter, @@ -239,6 +244,7 @@ impl PeerDb { && !cur_outbound_conn_addr_groups .contains(&AddressGroup::from_peer_address(&addr.as_peer_address())) && !self.banned_addresses.contains_key(&addr.as_bannable()) + && !self.discouraged_addresses.contains_key(&addr.as_bannable()) && additional_filter(addr) } None => { @@ -289,7 +295,9 @@ impl PeerDb { addr_group_to_addr_map.values().copied().collect() } - pub fn select_non_reserved_address_from_new_addr_table(&self) -> Option { + pub fn select_non_reserved_outbound_address_from_new_addr_table( + &self, + ) -> Option { let now = self.time_getter.get_time(); self.address_tables @@ -298,7 +306,7 @@ impl PeerDb { Some(addr_data) => { addr_data.connect_now(now) && !addr_data.reserved() - && !self.banned_addresses.contains_key(&addr.as_bannable()) + && !self.is_address_banned_or_discouraged(&addr.as_bannable()) } None => { debug_assert!(false, "Address {addr} not found in self.addresses"); @@ -344,7 +352,7 @@ impl PeerDb { }); self.banned_addresses.retain(|addr, banned_till| { - let banned = now <= *banned_till; + let banned = now < *banned_till; if !banned { update_db(&self.storage, |tx| tx.del_banned_address(addr)) @@ -353,6 +361,17 @@ impl PeerDb { banned }); + + self.discouraged_addresses.retain(|addr, discouraged_till| { + let discouraged = now < *discouraged_till; + + if !discouraged { + update_db(&self.storage, |tx| tx.del_discouraged_address(addr)) + .expect("removing discouraged address is expected to succeed"); + } + + discouraged + }); } /// Add a new peer address @@ -539,24 +558,48 @@ impl PeerDb { /// Changes the address state to banned pub fn ban(&mut self, address: BannableAddress) { - let ban_till = (self.time_getter.get_time() + *self.p2p_config.ban_duration) + let ban_till = (self.time_getter.get_time() + *self.p2p_config.ban_config.ban_duration) .expect("Ban duration is expected to be valid"); update_db(&self.storage, |tx| { tx.add_banned_address(&address, ban_till) }) - .expect("adding banned address is expected to succeed (ban_peer)"); + .expect("adding banned address is expected to succeed"); self.banned_addresses.insert(address, ban_till); } pub fn unban(&mut self, address: &BannableAddress) { update_db(&self.storage, |tx| tx.del_banned_address(address)) - .expect("adding banned address is expected to succeed (ban_peer)"); + .expect("removing banned address is expected to succeed"); self.banned_addresses.remove(address); } + /// Checks if the given address is discouraged + pub fn is_address_discouraged(&self, address: &BannableAddress) -> bool { + self.discouraged_addresses.contains_key(address) + } + + /// Changes the address state to discouraged + pub fn discourage(&mut self, address: BannableAddress) { + // TODO: do we need to prolong the discouragement if the peer continues misbehaving? + let discourage_till = (self.time_getter.get_time() + + *self.p2p_config.ban_config.discouragement_duration) + .expect("Discouragement duration is expected to be valid"); + + update_db(&self.storage, |tx| { + tx.add_discouraged_address(&address, discourage_till) + }) + .expect("adding discouraged address is expected to succeed"); + + self.discouraged_addresses.insert(address, discourage_till); + } + + pub fn is_address_banned_or_discouraged(&self, address: &BannableAddress) -> bool { + self.is_address_banned(address) || self.is_address_discouraged(address) + } + pub fn anchors(&self) -> &BTreeSet { &self.anchor_addresses } @@ -596,10 +639,21 @@ impl PeerDb { } pub trait PeerDbInterface { + fn is_address_banned(&self, address: &BannableAddress) -> bool; + fn is_address_discouraged(&self, address: &BannableAddress) -> bool; + fn peer_discovered(&mut self, address: SocketAddress); } impl PeerDbInterface for PeerDb { + fn is_address_banned(&self, address: &BannableAddress) -> bool { + self.is_address_banned(address) + } + + fn is_address_discouraged(&self, address: &BannableAddress) -> bool { + self.is_address_discouraged(address) + } + fn peer_discovered(&mut self, address: SocketAddress) { self.peer_discovered(address) } diff --git a/p2p/src/peer_manager/peerdb/storage.rs b/p2p/src/peer_manager/peerdb/storage.rs index 2aa82ff16b..ecd16c0314 100644 --- a/p2p/src/peer_manager/peerdb/storage.rs +++ b/p2p/src/peer_manager/peerdb/storage.rs @@ -47,6 +47,8 @@ pub trait PeerDbStorageRead { fn get_banned_addresses(&self) -> crate::Result>; + fn get_discouraged_addresses(&self) -> crate::Result>; + fn get_anchor_addresses(&self) -> crate::Result>; } @@ -67,6 +69,13 @@ pub trait PeerDbStorageWrite { fn add_banned_address(&mut self, address: &BannableAddress, time: Time) -> crate::Result<()>; fn del_banned_address(&mut self, address: &BannableAddress) -> crate::Result<()>; + fn add_discouraged_address( + &mut self, + address: &BannableAddress, + time: Time, + ) -> crate::Result<()>; + fn del_discouraged_address(&mut self, address: &BannableAddress) -> crate::Result<()>; + fn add_anchor_address(&mut self, address: &SocketAddress) -> crate::Result<()>; fn del_anchor_address(&mut self, address: &SocketAddress) -> crate::Result<()>; } diff --git a/p2p/src/peer_manager/peerdb/storage_impl.rs b/p2p/src/peer_manager/peerdb/storage_impl.rs index 1857e164db..c2e280f9c4 100644 --- a/p2p/src/peer_manager/peerdb/storage_impl.rs +++ b/p2p/src/peer_manager/peerdb/storage_impl.rs @@ -42,9 +42,14 @@ storage::decl_schema! { /// Table for known addresses pub DBKnownAddresses: Map, - /// Table for banned addresses vs when they can be unbanned (Duration is timestamp since UNIX Epoch) + /// Table for banned addresses vs the time when they should be unbanned + /// (Duration is a timestamp since UNIX Epoch) pub DBBannedAddresses: Map, + /// Table for discouraged addresses vs the time when the discouragement should expire + /// (Duration is a timestamp since UNIX Epoch) + pub DBDiscouragedAddresses: Map, + /// Table for anchor peers addresses pub DBAnchorAddresses: Map, } @@ -95,6 +100,21 @@ impl<'st, B: storage::Backend> PeerDbStorageWrite for PeerDbStoreTxRw<'st, B> { Ok(self.storage().get_mut::().del(address.to_string())?) } + fn add_discouraged_address( + &mut self, + address: &BannableAddress, + time: Time, + ) -> crate::Result<()> { + Ok(self + .storage() + .get_mut::() + .put(address.to_string(), time.as_duration_since_epoch())?) + } + + fn del_discouraged_address(&mut self, address: &BannableAddress) -> crate::Result<()> { + Ok(self.storage().get_mut::().del(address.to_string())?) + } + fn add_anchor_address(&mut self, address: &SocketAddress) -> crate::Result<()> { Ok(self.storage().get_mut::().put(address.to_string(), ())?) } @@ -159,6 +179,19 @@ impl<'st, B: storage::Backend> PeerDbStorageRead for PeerDbStoreTxRo<'st, B> { itertools::process_results(iter, |iter| iter.collect::>()) } + fn get_discouraged_addresses(&self) -> crate::Result> { + let map = self.storage().get::(); + let iter = map.prefix_iter_decoded(&())?.map(|(addr_str, dur)| { + let addr = addr_str.parse::().map_err(|err| { + P2pError::InvalidStorageState(format!( + "Error parsing address from {addr_str:?}: {err}" + )) + })?; + Ok((addr, Time::from_duration_since_epoch(dur))) + }); + itertools::process_results(iter, |iter| iter.collect::>()) + } + fn get_anchor_addresses(&self) -> crate::Result> { let map = self.storage().get::(); let iter = map.prefix_iter_decoded(&())?.map(|(addr_str, _)| { diff --git a/p2p/src/peer_manager/peerdb/storage_load.rs b/p2p/src/peer_manager/peerdb/storage_load.rs index 9d0c112db6..49245f0317 100644 --- a/p2p/src/peer_manager/peerdb/storage_load.rs +++ b/p2p/src/peer_manager/peerdb/storage_load.rs @@ -32,11 +32,12 @@ use super::{ storage_impl::PeerDbStorageImpl, }; -const CURRENT_STORAGE_VERSION: StorageVersion = StorageVersion::new(2); +pub const CURRENT_STORAGE_VERSION: StorageVersion = StorageVersion::new(3); pub struct LoadedStorage { pub known_addresses: BTreeMap, pub banned_addresses: BTreeMap, + pub discouraged_addresses: BTreeMap, pub anchor_addresses: BTreeSet, pub salt: Salt, } @@ -52,7 +53,7 @@ impl LoadedStorage { match version { None => Self::init_storage(storage, peerdb_config), - Some(CURRENT_STORAGE_VERSION) => Self::load_storage_v2(storage), + Some(CURRENT_STORAGE_VERSION) => Self::load_storage_v3(storage), Some(version) => Err(P2pError::PeerDbStorageVersionMismatch { expected_version: CURRENT_STORAGE_VERSION, actual_version: version, @@ -74,12 +75,13 @@ impl LoadedStorage { Ok(LoadedStorage { known_addresses: BTreeMap::new(), banned_addresses: BTreeMap::new(), + discouraged_addresses: BTreeMap::new(), anchor_addresses: BTreeSet::new(), salt, }) } - fn load_storage_v2(storage: &S) -> crate::Result { + fn load_storage_v3(storage: &S) -> crate::Result { let tx = storage.transaction_ro()?; let known_addresses = tx.get_known_addresses()?.iter().copied().collect::>(); @@ -87,6 +89,9 @@ impl LoadedStorage { let banned_addresses = tx.get_banned_addresses()?.iter().copied().collect::>(); + let discouraged_addresses = + tx.get_discouraged_addresses()?.iter().copied().collect::>(); + let anchor_addresses = tx.get_anchor_addresses()?.iter().copied().collect::>(); let salt = tx @@ -96,6 +101,7 @@ impl LoadedStorage { Ok(LoadedStorage { known_addresses, banned_addresses, + discouraged_addresses, anchor_addresses, salt, }) diff --git a/p2p/src/peer_manager/peerdb/tests.rs b/p2p/src/peer_manager/peerdb/tests.rs index 32bd3a50f2..57e076be82 100644 --- a/p2p/src/peer_manager/peerdb/tests.rs +++ b/p2p/src/peer_manager/peerdb/tests.rs @@ -24,14 +24,12 @@ use itertools::Itertools; use rstest::rstest; use ::test_utils::random::{make_seedable_rng, Seed}; -use common::{ - chain::config::create_unit_test_config, primitives::user_agent::mintlayer_core_user_agent, -}; +use common::chain::config::create_unit_test_config; use crypto::random::Rng; use p2p_test_utils::P2pBasicTestTimeGetter; use crate::{ - config::P2pConfig, + ban_config::BanConfig, peer_manager::{ peerdb::{ address_data::{self, PURGE_REACHABLE_FAIL_COUNT, PURGE_UNREACHABLE_TIME}, @@ -41,8 +39,8 @@ use crate::{ peerdb_common::Transactional, }, testing_utils::{ - peerdb_inmemory_store, test_p2p_config, test_p2p_config_with_peer_db_config, - TestAddressMaker, + peerdb_inmemory_store, test_p2p_config, test_p2p_config_with_ban_config, + test_p2p_config_with_peer_db_config, TestAddressMaker, }, }; @@ -64,28 +62,13 @@ fn unban_peer() { let chain_config = create_unit_test_config(); let mut peerdb = PeerDb::<_>::new( &chain_config, - Arc::new(P2pConfig { + Arc::new(test_p2p_config_with_ban_config(BanConfig { ban_duration: Duration::from_secs(60).into(), + discouragement_duration: Duration::from_secs(600).into(), - bind_addresses: Default::default(), - socks5_proxy: None, - disable_noise: Default::default(), - boot_nodes: Default::default(), - reserved_nodes: Default::default(), - whitelisted_addresses: Default::default(), ban_threshold: Default::default(), - outbound_connection_timeout: Default::default(), - ping_check_period: Default::default(), - ping_timeout: Default::default(), - peer_handshake_timeout: Default::default(), - max_clock_diff: Default::default(), - node_type: Default::default(), - allow_discover_private_ips: Default::default(), - user_agent: mintlayer_core_user_agent(), - sync_stalling_timeout: Default::default(), - peer_manager_config: Default::default(), - protocol_config: Default::default(), - }), + discouragement_threshold: Default::default(), + })), time_getter.get_time_getter(), db_store, ) @@ -94,15 +77,84 @@ fn unban_peer() { let address = TestAddressMaker::new_random_address(); peerdb.ban(address.as_bannable()); + // The address is banned. assert!(peerdb.is_address_banned(&address.as_bannable())); let banned_addresses = peerdb.storage.transaction_ro().unwrap().get_banned_addresses().unwrap(); assert_eq!(banned_addresses.len(), 1); + assert_eq!(banned_addresses[0].0, address.as_bannable()); + + // But not discouraged. + assert!(!peerdb.is_address_discouraged(&address.as_bannable())); + let discouraged_addresses = + peerdb.storage.transaction_ro().unwrap().get_discouraged_addresses().unwrap(); + assert_eq!(discouraged_addresses.len(), 0); + + time_getter.advance_time(Duration::from_secs(120)); + + // Banned addresses are updated in the `heartbeat` function + peerdb.heartbeat(); + + // The address is no longer banned. + assert!(!peerdb.is_address_banned(&address.as_bannable())); + let banned_addresses = peerdb.storage.transaction_ro().unwrap().get_banned_addresses().unwrap(); + assert_eq!(banned_addresses.len(), 0); + + // And still not discouraged. + assert!(!peerdb.is_address_discouraged(&address.as_bannable())); + let discouraged_addresses = + peerdb.storage.transaction_ro().unwrap().get_discouraged_addresses().unwrap(); + assert_eq!(discouraged_addresses.len(), 0); + + assert_addr_consistency(&peerdb); +} + +#[tracing::instrument] +#[test] +fn undiscourage_peer() { + let db_store = peerdb_inmemory_store(); + let time_getter = P2pBasicTestTimeGetter::new(); + let chain_config = create_unit_test_config(); + let mut peerdb = PeerDb::<_>::new( + &chain_config, + Arc::new(test_p2p_config_with_ban_config(BanConfig { + discouragement_duration: Duration::from_secs(60).into(), + ban_duration: Duration::from_secs(600).into(), + + ban_threshold: Default::default(), + discouragement_threshold: Default::default(), + })), + time_getter.get_time_getter(), + db_store, + ) + .unwrap(); + + let address = TestAddressMaker::new_random_address(); + peerdb.discourage(address.as_bannable()); + + // The address is discouraged. + assert!(peerdb.is_address_discouraged(&address.as_bannable())); + let discouraged_addresses = + peerdb.storage.transaction_ro().unwrap().get_discouraged_addresses().unwrap(); + assert_eq!(discouraged_addresses.len(), 1); + assert_eq!(discouraged_addresses[0].0, address.as_bannable()); + + // But not banned. + assert!(!peerdb.is_address_banned(&address.as_bannable())); + let banned_addresses = peerdb.storage.transaction_ro().unwrap().get_banned_addresses().unwrap(); + assert_eq!(banned_addresses.len(), 0); time_getter.advance_time(Duration::from_secs(120)); - // Banned addresses updated in the `heartbeat` function + // Discouraged addresses are updated in the `heartbeat` function peerdb.heartbeat(); + // The address is no longer discouraged. + assert!(!peerdb.is_address_discouraged(&address.as_bannable())); + let discouraged_addresses = + peerdb.storage.transaction_ro().unwrap().get_discouraged_addresses().unwrap(); + assert_eq!(discouraged_addresses.len(), 0); + + // And still not banned. assert!(!peerdb.is_address_banned(&address.as_bannable())); let banned_addresses = peerdb.storage.transaction_ro().unwrap().get_banned_addresses().unwrap(); assert_eq!(banned_addresses.len(), 0); diff --git a/p2p/src/peer_manager/peers_eviction/mod.rs b/p2p/src/peer_manager/peers_eviction/mod.rs index 1793364051..5d029b4ad8 100644 --- a/p2p/src/peer_manager/peers_eviction/mod.rs +++ b/p2p/src/peer_manager/peers_eviction/mod.rs @@ -16,12 +16,12 @@ use std::{collections::BTreeMap, hash::Hasher, time::Duration}; use common::primitives::time::Time; -use crypto::random::Rng; +use crypto::random::{seq::IteratorRandom, Rng}; use utils::make_config_setting; use crate::{net::types::PeerRole, types::peer_id::PeerId}; -use super::{address_groups::AddressGroup, peer_context::PeerContext, PeerManagerConfig}; +use super::{address_groups::AddressGroup, config::PeerManagerConfig, peer_context::PeerContext}; #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] struct NetGroupKeyed(u64); @@ -67,6 +67,9 @@ pub struct EvictionCandidate { /// The time since which we've been expecting a block from the peer; if None, we're not /// expecting any blocks from it. expecting_blocks_since: Option