Fix a flaky test by unbanning a peer

domains/client/domain-operator/src/tests.rs

@@ -1629,7 +1629,10 @@ async fn test_domain_tx_propagate() -> Result<(), tokio::time::error::Elapsed> {
 
     async {
         while alice.sync_service.is_major_syncing() || bob.sync_service.is_major_syncing() {
-            tokio::time::sleep(std::time::Duration::from_millis(100)).await;
+            tokio::time::sleep(std::time::Duration::from_secs(1)).await;
+            // Unfortunately, this test contains a race condition where Alice sometimes bans Bob
+            // for making multiple requests for the same block.
+            alice.unban_peer(bob.addr.clone());
         }
     }
     .timeout()

domains/test/service/src/domain.rs

@@ -20,7 +20,7 @@ use pallet_transaction_payment_rpc::TransactionPaymentRuntimeApi;
 use sc_client_api::HeaderBackend;
 use sc_domains::RuntimeExecutor;
 use sc_network::service::traits::NetworkService;
-use sc_network::{NetworkRequest, NetworkStateInfo};
+use sc_network::{NetworkRequest, NetworkStateInfo, ReputationChange};
 use sc_network_sync::SyncingService;
 use sc_service::config::MultiaddrWithPeerId;
 use sc_service::{BasePath, Role, RpcHandlers, TFullBackend, TaskManager};
@@ -406,6 +406,35 @@ where
         let function = function.into();
         UncheckedExtrinsicFor::<Runtime>::new_unsigned(function)
     }
+
+    /// Give the peer at `addr` the minimum reputation, which will ban it.
+    // TODO: also ban/unban in the DSN
+    pub fn ban_peer(&self, addr: MultiaddrWithPeerId) {
+        // If unban_peer() has been called on the peer, we need to bump it twice
+        // to give it the minimal reputation.
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new_fatal("Peer banned by test (1)"),
+        );
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new_fatal("Peer banned by test (2)"),
+        );
+    }
+
+    /// Give the peer at `addr` a high reputation, which guarantees it is un-banned it.
+    pub fn unban_peer(&self, addr: MultiaddrWithPeerId) {
+        // If ReputationChange::new_fatal() has been called on the peer, we need to bump it twice
+        // to give it a positive reputation.
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new(i32::MAX, "Peer unbanned by test (1)"),
+        );
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new(i32::MAX, "Peer unbanned by test (2)"),
+        );
+    }
 }
 
 impl<Runtime, RuntimeApi> DomainNode<Runtime, RuntimeApi>

test/subspace-test-service/src/lib.rs

@@ -37,7 +37,9 @@ use sc_consensus::{BasicQueue, BlockImport, StateAction, Verifier as VerifierT};
 use sc_domains::ExtensionsFactory as DomainsExtensionFactory;
 use sc_network::config::{NetworkConfiguration, TransportConfig};
 use sc_network::service::traits::NetworkService;
-use sc_network::{multiaddr, NetworkWorker, NotificationMetrics, NotificationService};
+use sc_network::{
+    multiaddr, NetworkWorker, NotificationMetrics, NotificationService, ReputationChange,
+};
 use sc_service::config::{
     DatabaseSource, ExecutorConfiguration, KeystoreConfig, MultiaddrWithPeerId,
     OffchainWorkerConfig, RpcBatchRequestConfig, RpcConfiguration, WasmExecutionMethod,
@@ -848,6 +850,35 @@ impl MockConsensusNode {
             .free_balance(self.client.info().best_hash, account_id)
             .expect("Fail to get account free balance")
     }
+
+    /// Give the peer at `addr` the minimum reputation, which will ban it.
+    // TODO: also ban/unban in the DSN
+    pub fn ban_peer(&self, addr: MultiaddrWithPeerId) {
+        // If unban_peer() has been called on the peer, we need to bump it twice
+        // to give it the minimal reputation.
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new_fatal("Peer banned by test (1)"),
+        );
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new_fatal("Peer banned by test (2)"),
+        );
+    }
+
+    /// Give the peer at `addr` a high reputation, which guarantees it is un-banned it.
+    pub fn unban_peer(&self, addr: MultiaddrWithPeerId) {
+        // If ReputationChange::new_fatal() has been called on the peer, we need to bump it twice
+        // to give it a positive reputation.
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new(i32::MAX, "Peer unbanned by test (1)"),
+        );
+        self.network_service.report_peer(
+            addr.peer_id,
+            ReputationChange::new(i32::MAX, "Peer unbanned by test (2)"),
+        );
+    }
 }
 
 impl MockConsensusNode {