From 0e47177b3809248ae1379402b852dc3d58dc3c50 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Fri, 12 Apr 2019 23:54:06 -0400 Subject: [PATCH 1/2] 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. --- lib/Xud.ts | 2 +- lib/orderbook/OrderBook.ts | 4 +-- lib/p2p/Peer.ts | 18 +++++----- lib/p2p/Pool.ts | 34 +++++++++++++------ .../packets/types/NodeStateUpdatePacket.ts | 19 +++++------ lib/p2p/packets/types/SessionInitPacket.ts | 8 ++--- lib/p2p/types.ts | 11 ++---- lib/raidenclient/RaidenClient.ts | 4 +-- test/integration/Pool.spec.ts | 1 + test/unit/Parser.spec.ts | 2 -- 10 files changed, 53 insertions(+), 50 deletions(-) diff --git a/lib/Xud.ts b/lib/Xud.ts index a97351061..e85999ccf 100644 --- 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 index ea414eeca..d1c6e3064 100644 --- 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 index 895112692..4f8ba4f75 100644 --- 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 index b208c66af..fb36cc766 100644 --- 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 => { + public init = async (ownNodeState: Pick>, nodeKey: NodeKey): Promise => { 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 index 076c1ed1c..b0d8fd9fd 100644 --- a/lib/p2p/packets/types/NodeStateUpdatePacket.ts +++ b/lib/p2p/packets/types/NodeStateUpdatePacket.ts @@ -25,6 +25,7 @@ class NodeStateUpdatePacket extends Packet { 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 { 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 { 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 index 50ef5c899..81a4ed1f5 100644 --- a/lib/p2p/packets/types/SessionInitPacket.ts +++ b/lib/p2p/packets/types/SessionInitPacket.ts @@ -56,8 +56,8 @@ class SessionInitPacket extends Packet { 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 { 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 index ff3fa8c22..45157ac3a 100644 --- 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>; 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 index 7a9bfabbe..aefb1dee6 100644 --- 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 index 27d6c4941..cb2abba17 100644 --- 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 index b7fe2b919..620d4176e 100644 --- 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())); From 7112be0b2522cf020eded0ec77d9bb8275726a01 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Sat, 13 Apr 2019 00:45:09 -0400 Subject: [PATCH 2/2] test: add NodeStateUpdate sanity test --- test/p2p/sanity.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/p2p/sanity.spec.ts b/test/p2p/sanity.spec.ts index fa45baffc..df8e66475 100644 --- a/test/p2p/sanity.spec.ts +++ b/test/p2p/sanity.spec.ts @@ -73,6 +73,17 @@ describe('P2P Sanity Tests', () => { expect(listPeersResult[0].nodePubKey).to.equal(nodeTwo.nodePubKey); }); + it('should update the node state', (done) => { + const raidenAddress = '0xbb9bc244d798123fde783fcc1c72d3bb8c189413'; + const nodeTwoPeer = nodeOne['pool'].getPeer(nodeTwo.nodePubKey); + nodeTwoPeer.on('nodeStateUpdate', () => { + expect(nodeTwoPeer['nodeState']!.raidenAddress).to.equal(raidenAddress); + done(); + }); + + nodeTwo['pool'].updateRaidenAddress(raidenAddress); + }); + it('should fail connecting to the same node', async () => { await expect(nodeOne.service.connect({ nodeUri: nodeTwoUri, retryConnecting: false })) .to.be.rejectedWith('already connected');