refactor: NodeStateUpdate to include all fields

This commit refactors the `NodeStateUpdate` type and the logic around it to include all the fields from the `NodeState` type except those which should be fixed, namely `version` and `nodePubKey`. This is a follow-up to the fix in #876.
ExchangeUnion · Apr 13, 2019 · 0e47177 · 0e47177
1 parent cd489b8
commit 0e47177
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 50 deletions.
diff --git a/lib/Xud.ts b/lib/Xud.ts
@@ -178,7 +178,7 @@ class Xud extends EventEmitter {
     }
     this.raidenClient.on('connectionVerified', (newAddress) => {
       if (newAddress) {
-        this.pool.updateNodeState({ raidenAddress: newAddress });
+        this.pool.updateRaidenAddress(newAddress);
       }
     });
   }

diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts
@@ -195,7 +195,7 @@ class OrderBook extends EventEmitter {
     this.tradingPairs.set(pairInstance.id, new TradingPair(this.logger, pairInstance.id, this.nomatching));
 
     if (this.pool) {
-      this.pool.updateNodeState({ pairs: this.pairIds });
+      this.pool.updatePairs(this.pairIds);
     }
     return pairInstance;
   }
@@ -236,7 +236,7 @@ class OrderBook extends EventEmitter {
     this.tradingPairs.delete(pairId);
 
     if (this.pool) {
-      this.pool.updateNodeState({ pairs: this.pairIds });
+      this.pool.updatePairs(this.pairIds);
     }
     return pair.destroy();
   }

diff --git a/lib/p2p/Peer.ts b/lib/p2p/Peer.ts
@@ -841,17 +841,15 @@ class Peer extends EventEmitter {
     const nodeStateUpdate = packet.body!;
     this.logger.verbose(`received node state update packet from ${this.label}: ${JSON.stringify(nodeStateUpdate)}`);
 
-    const prevNodeState = this.nodeState;
-    if (prevNodeState) {
-      prevNodeState.pairs.forEach((pairId) => {
-        if (!nodeStateUpdate.pairs || !nodeStateUpdate.pairs.includes(pairId)) {
-          // a trading pair was in the old node state but not in the updated one
-          this.emit('pairDropped', pairId);
-        }
-      });
-    }
+    const prevNodeState = this.nodeState!;
+    prevNodeState.pairs.forEach((pairId) => {
+      if (!nodeStateUpdate.pairs.includes(pairId)) {
+        // a trading pair was in the old node state but not in the updated one
+        this.emit('pairDropped', pairId);
+      }
+    });
 
-    this.nodeState = { ...prevNodeState, ...nodeStateUpdate as NodeState };
+    this.nodeState = { ...nodeStateUpdate, nodePubKey: prevNodeState.nodePubKey, version: prevNodeState.version };
     this.emit('nodeStateUpdate');
   }
 

diff --git a/lib/p2p/Pool.ts b/lib/p2p/Pool.ts
@@ -103,7 +103,7 @@ class Pool extends EventEmitter {
   /**
    * Initialize the Pool by connecting to known nodes and listening to incoming peer connections, if configured to do so.
    */
-  public init = async (ownNodeState: NodeState, nodeKey: NodeKey): Promise<void> => {
+  public init = async (ownNodeState: Pick<NodeState, Exclude<keyof NodeState, 'addresses'>>, nodeKey: NodeKey): Promise<void> => {
     if (this.connected) {
       return;
     }
@@ -118,8 +118,7 @@ class Pool extends EventEmitter {
       }
     }
 
-    this.nodeState = ownNodeState;
-    this.nodeState.addresses = this.addresses;
+    this.nodeState = { ...ownNodeState, addresses: this.addresses };
     this.nodeKey = nodeKey;
 
     this.bindNodeList();
@@ -160,21 +159,34 @@ class Pool extends EventEmitter {
   }
 
   /**
-   * Updates the node state and sends node state update packet to currently connected
+   * Updates our active trading pairs and sends a node state update packet to currently connected
    * peers to notify them of the change.
    */
-  public updateNodeState = (nodeStateUpdate: NodeStateUpdate) => {
-    this.nodeState = { ...this.nodeState, ...nodeStateUpdate };
-    this.sendNodeStateUpdate(this.nodeState);
+  public updatePairs = (pairs: string[]) => {
+    this.nodeState.pairs = pairs;
+    this.sendNodeStateUpdate();
   }
 
+  /**
+   * Updates our raiden address and sends a node state update packet to currently connected
+   * peers to notify them of the change.
+   */
+  public updateRaidenAddress = (raidenAddress: string) => {
+    this.nodeState.raidenAddress = raidenAddress;
+    this.sendNodeStateUpdate();
+  }
+
+  /**
+   * Updates our lnd pub key for a given currency and sends a node state update packet to currently
+   * connected peers to notify them of the change.
+   */
   public updateLndPubKey = (currency: string, pubKey: string) => {
     this.nodeState.lndPubKeys[currency] = pubKey;
-    this.sendNodeStateUpdate(this.nodeState);
+    this.sendNodeStateUpdate();
   }
 
-  private sendNodeStateUpdate = (nodeStateUpdate: NodeStateUpdate) => {
-    const packet = new packets.NodeStateUpdatePacket(nodeStateUpdate);
+  private sendNodeStateUpdate = () => {
+    const packet = new packets.NodeStateUpdatePacket(this.nodeState);
     this.peers.forEach(async (peer) => {
       await peer.sendPacket(packet);
     });
@@ -210,7 +222,7 @@ class Pool extends EventEmitter {
   }
 
   private verifyReachability = () => {
-    this.nodeState.addresses!.forEach(async (address) => {
+    this.nodeState.addresses.forEach(async (address) => {
       const externalAddress = addressUtils.toString(address);
       this.logger.debug(`Verifying reachability of advertised address: ${externalAddress}`);
       try {

diff --git a/lib/p2p/packets/types/NodeStateUpdatePacket.ts b/lib/p2p/packets/types/NodeStateUpdatePacket.ts
@@ -25,6 +25,7 @@ class NodeStateUpdatePacket extends Packet<NodeStateUpdate> {
   private static validate = (obj: pb.NodeStateUpdatePacket.AsObject): boolean => {
     return !!(obj.id
       && obj.pairsList
+      && obj.lndPubKeysMap
       && obj.addressesList.every(addr => !!addr.host)
     );
   }
@@ -37,8 +38,8 @@ class NodeStateUpdatePacket extends Packet<NodeStateUpdate> {
       body: removeUndefinedProps({
         pairs: obj.pairsList,
         addresses: obj.addressesList,
-        raidenAddress: obj.raidenAddress || undefined,
-        lndPubKeys: obj.lndPubKeysMap ? convertKvpArrayToKvps(obj.lndPubKeysMap) : undefined,
+        raidenAddress: obj.raidenAddress,
+        lndPubKeys: convertKvpArrayToKvps(obj.lndPubKeysMap),
       }),
     });
   }
@@ -47,19 +48,17 @@ class NodeStateUpdatePacket extends Packet<NodeStateUpdate> {
     const msg = new pb.NodeStateUpdatePacket();
 
     msg.setId(this.header.id);
-    msg.setPairsList(this.body!.pairs!);
-    msg.setAddressesList(this.body!.addresses!.map((addr) => {
+    msg.setPairsList(this.body!.pairs);
+    msg.setAddressesList(this.body!.addresses.map((addr) => {
       const pbAddr = new pb.Address();
       pbAddr.setHost(addr.host);
       pbAddr.setPort(addr.port);
       return pbAddr;
     }));
-    msg.setRaidenAddress(this.body!.raidenAddress!);
-    if (this.body!.lndPubKeys) {
-      const map = msg.getLndPubKeysMap();
-      for (const currency in this.body!.lndPubKeys!) {
-        map.set(currency, this.body!.lndPubKeys![currency]);
-      }
+    msg.setRaidenAddress(this.body!.raidenAddress);
+    const map = msg.getLndPubKeysMap();
+    for (const currency in this.body!.lndPubKeys) {
+      map.set(currency, this.body!.lndPubKeys[currency]);
     }
 
     return msg.serializeBinary();

diff --git a/lib/p2p/packets/types/SessionInitPacket.ts b/lib/p2p/packets/types/SessionInitPacket.ts
@@ -56,8 +56,8 @@ class SessionInitPacket extends Packet<SessionInitPacketBody> {
           nodePubKey: obj.nodeState!.nodePubKey,
           pairs: obj.nodeState!.pairsList,
           addresses: obj.nodeState!.addressesList,
-          raidenAddress: obj.nodeState!.raidenAddress || undefined,
-          lndPubKeys: obj.nodeState!.lndPubKeysMap ? convertKvpArrayToKvps(obj.nodeState!.lndPubKeysMap) : undefined,
+          raidenAddress: obj.nodeState!.raidenAddress,
+          lndPubKeys: convertKvpArrayToKvps(obj.nodeState!.lndPubKeysMap),
         }),
       },
     });
@@ -74,13 +74,13 @@ class SessionInitPacket extends Packet<SessionInitPacketBody> {
       pbNodeState.setVersion(this.body!.nodeState.version);
       pbNodeState.setNodePubKey(this.body!.nodeState.nodePubKey);
       pbNodeState.setPairsList(this.body!.nodeState.pairs);
-      pbNodeState.setAddressesList(this.body!.nodeState.addresses!.map((addr) => {
+      pbNodeState.setAddressesList(this.body!.nodeState.addresses.map((addr) => {
         const pbAddr = new pb.Address();
         pbAddr.setHost(addr.host);
         pbAddr.setPort(addr.port);
         return pbAddr;
       }));
-      pbNodeState.setRaidenAddress(this.body!.nodeState.raidenAddress!);
+      pbNodeState.setRaidenAddress(this.body!.nodeState.raidenAddress);
       if (this.body!.nodeState.lndPubKeys) {
         setObjectToMap(this.body!.nodeState.lndPubKeys, pbNodeState.getLndPubKeysMap());
       }

diff --git a/lib/p2p/types.ts b/lib/p2p/types.ts
@@ -15,18 +15,13 @@ export type NodeConnectionInfo = {
 export type NodeState = {
   version: string;
   nodePubKey: string;
-  addresses?: Address[];
+  addresses: Address[];
   pairs: string[];
-  raidenAddress?: string;
+  raidenAddress: string;
   lndPubKeys: { [currency: string]: string | undefined };
 };
 
-export type NodeStateUpdate = {
-  addresses?: Address[];
-  pairs?: string[];
-  raidenAddress?: string;
-  lndPubKeys?: { [currency: string]: string | undefined };
-};
+export type NodeStateUpdate = Pick<NodeState, Exclude<keyof NodeState, 'version' | 'nodePubKey'>>;
 
 export type PoolConfig = {
   /** Whether or not to automatically detect and share current external ip address on startup. */

diff --git a/lib/raidenclient/RaidenClient.ts b/lib/raidenclient/RaidenClient.ts
@@ -74,7 +74,7 @@ type ChannelEvent = {
 class RaidenClient extends BaseClient {
   public readonly type = SwapClient.Raiden;
   public readonly cltvDelta: number = 0;
-  public address?: string;
+  public address = '';
   private port: number;
   private host: string;
 
@@ -160,7 +160,7 @@ class RaidenClient extends BaseClient {
     } else {
       try {
         channels = (await this.getChannels()).length;
-        address = this.address!;
+        address = this.address;
       } catch (err) {
         error = err.message;
       }

diff --git a/test/integration/Pool.spec.ts b/test/integration/Pool.spec.ts
@@ -58,6 +58,7 @@ describe('P2P Pool Tests', async () => {
       version: 'test',
       pairs: [],
       lndPubKeys: {},
+      raidenAddress: '',
     }, nodeKeyOne);
   });
 

diff --git a/test/unit/Parser.spec.ts b/test/unit/Parser.spec.ts
@@ -214,7 +214,6 @@ describe('Parser', () => {
     testValidPacket(new packets.SessionInitPacket(sessionInitPacketBody));
     testValidPacket(new packets.SessionInitPacket({ ...sessionInitPacketBody, nodeState: { ...nodeState, pairs: [] } }));
     testValidPacket(new packets.SessionInitPacket({ ...sessionInitPacketBody, nodeState: { ...nodeState, addresses: [] } }));
-    testValidPacket(new packets.SessionInitPacket({ ...sessionInitPacketBody, nodeState: removeUndefinedProps({ ...nodeState, raidenAddress: undefined }) }));
     testValidPacket(new packets.SessionInitPacket({ ...sessionInitPacketBody, nodeState: removeUndefinedProps({ ...nodeState, lndPubKeys: { ...nodeState.lndPubKeys, BTC: undefined } }) }));
     testValidPacket(new packets.SessionInitPacket({ ...sessionInitPacketBody, nodeState: removeUndefinedProps({ ...nodeState, lndPubKeys: { ...nodeState.lndPubKeys, LTC: undefined } }) }));
     testInvalidPacket(new packets.SessionInitPacket(sessionInitPacketBody, uuid()));
@@ -234,7 +233,6 @@ describe('Parser', () => {
     testValidPacket(new packets.NodeStateUpdatePacket(nodeStateUpdate));
     testValidPacket(new packets.NodeStateUpdatePacket({ ...nodeStateUpdate, pairs: [] }));
     testValidPacket(new packets.NodeStateUpdatePacket({ ...nodeStateUpdate, addresses: [] }));
-    testValidPacket(new packets.NodeStateUpdatePacket(removeUndefinedProps({ ...nodeStateUpdate, raidenAddress: undefined })));
     testValidPacket(new packets.NodeStateUpdatePacket(removeUndefinedProps({ ...nodeStateUpdate, lndPubKeys: { ...nodeStateUpdate.lndPubKeys, BTC: undefined } })));
     testValidPacket(new packets.NodeStateUpdatePacket(removeUndefinedProps({ ...nodeStateUpdate, lndPubKeys: { ...nodeStateUpdate.lndPubKeys, LTC: undefined } })));
     testInvalidPacket(new packets.NodeStateUpdatePacket(nodeStateUpdate, uuid()));