fix: restrict reputation events in non-strict mode (#1808)

* fix: restrict reputation events in non-strict mode

This ensures we only add severe and manual reputation events to peers
when not running in strict mode by filtering both events that arise from
the peer itself as well as those originating from other sources, such
as failed swaps. Previously we only checked on events arising from the
peer, which, for example, would include packet delays but not a failure
to find routes.

Fixes #1802

* fix: only deactivate currencies in strict mode

Author: sangaman

Diff for: lib/p2p/Pool.ts

@@ -675,8 +675,19 @@ class Pool extends EventEmitter {
   }
 
   // A wrapper for [[NodeList.addReputationEvent]].
-  public addReputationEvent = (nodePubKey: string, event: ReputationEvent) => {
-    return this.nodes.addReputationEvent(nodePubKey, event);
+  public addReputationEvent = async (nodePubKey: string, event: ReputationEvent) => {
+    // when in strict mode, we add all reputation events
+    // otherwise, we only add manual or severe reputation events to prevent unintentional bans
+    if (this.strict
+      || event === ReputationEvent.ManualBan
+      || event === ReputationEvent.ManualUnban
+      || event === ReputationEvent.SwapAbuse
+      || event === ReputationEvent.SwapMisbehavior
+      || event === ReputationEvent.WireProtocolErr
+      || event === ReputationEvent.InvalidAuth) {
+      this.logger.debug(`Peer (${nodePubKey}): reputation event: ${ReputationEvent[event]}`);
+      await this.nodes.addReputationEvent(nodePubKey, event);
+    }
   }
 
   public sendToPeer = async (nodePubKey: string, packet: Packet) => {
@@ -931,19 +942,8 @@ class Pool extends EventEmitter {
     peer.once('close', () => this.handlePeerClose(peer));
 
     peer.on('reputation', async (event) => {
-      this.logger.debug(`Peer (${peer.label}): reputation event: ${ReputationEvent[event]}`);
       if (peer.nodePubKey) {
-        // when in strict mode, we add all reputation events
-        // otherwise, we only add manual or severe reputation events to prevent unintentional bans
-        if (this.strict
-          || event === ReputationEvent.ManualBan
-          || event === ReputationEvent.ManualUnban
-          || event === ReputationEvent.SwapAbuse
-          || event === ReputationEvent.SwapMisbehavior
-          || event === ReputationEvent.WireProtocolErr
-          || event === ReputationEvent.InvalidAuth) {
-          await this.addReputationEvent(peer.nodePubKey, event);
-        }
+        await this.addReputationEvent(peer.nodePubKey, event);
       }
     });
   }

Diff for: lib/swaps/Swaps.ts

@@ -1220,7 +1220,7 @@ class Swaps extends EventEmitter {
       case SwapFailureReason.SendPaymentFailure:
       case SwapFailureReason.NoRouteFound:
         // something is wrong with swaps for this currency with this peer
-        if (failedCurrency && !this.strict) { // only deactivate currencies due to failed swaps in strict mode
+        if (failedCurrency && this.strict) { // only deactivate currencies due to failed swaps in strict mode
           try {
             this.pool.getPeer(deal.peerPubKey).deactivateCurrency(failedCurrency);
           } catch (err) {

Diff for: test/integration/Swaps.spec.ts

@@ -85,7 +85,7 @@ describe('Swaps.Integration', () => {
     peer.getIdentifier = () => '1234567890';
     // pool
     pool = sandbox.createStubInstance(Pool) as any;
-    pool.addReputationEvent = () => Promise.resolve(true);
+    pool.addReputationEvent = () => Promise.resolve();
     pool.getPeer = () => peer;
     pool.tryGetPeer = () => peer;
     // getRoute response