From d0589ea1e4ed210df67e3957a063a96f693cba6f Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Thu, 9 Apr 2020 14:06:36 -0400 Subject: [PATCH] feat(lnd): attempt openchannel when connect fails We currently attempt to connect to a peer's lnd node using their advertised listening addresses before opening a channel over lnd. If the connection attempt fails for any reason other than that we are already connected to the peer, then we fail the open channel attempt. However, this prematurely fails cases where a peer does not advertise listening addresses, such that connection attempts won't even be made and the `OpenChannel` call will fail right away, but is already connected to us. This could happen if a peer had previously connected to us. Instead, we first attempt to connect to the peer's lnd node but then proceed to attempt to open a channel regardless of where we were able to connect or not. If we aren't connected to that peer, the lnd `OpenChannel` call will fail right away and the end result will be the same. On occasions where we are already connected, the call will succeed. This also performs some minor refactoring to rename `lndUris` to `uris` so that it is more generic for supporting future swap clients that also advertise listening addresses for nodes. --- lib/lndclient/LndClient.ts | 13 ++++---- lib/swaps/SwapClient.ts | 4 +-- lib/swaps/SwapClientManager.ts | 11 ++----- test/jest/LndClient.spec.ts | 31 ++++--------------- test/jest/SwapClientManager.spec.ts | 18 ++--------- .../jest/__snapshots__/LndClient.spec.ts.snap | 2 -- .../SwapClientManager.spec.ts.snap | 2 -- 7 files changed, 20 insertions(+), 61 deletions(-) diff --git a/lib/lndclient/LndClient.ts b/lib/lndclient/LndClient.ts index e9f0aebbc..c934b885e 100644 --- a/lib/lndclient/LndClient.ts +++ b/lib/lndclient/LndClient.ts @@ -718,15 +718,14 @@ class LndClient extends SwapClient { * Opens a channel given peerPubKey and amount. */ public openChannel = async ( - { peerIdentifier: peerPubKey, units, lndUris, pushUnits = 0 }: - { peerIdentifier: string, units: number, lndUris: string[], pushUnits?: number }, + { peerIdentifier: peerPubKey, units, uris, pushUnits = 0 }: + { peerIdentifier: string, units: number, uris?: string[], pushUnits?: number }, ): Promise => { - const connectionEstablished = await this.connectPeerAddreses(lndUris); - if (connectionEstablished) { - await this.openChannelSync(peerPubKey, units, pushUnits); - } else { - throw new Error(`could not connect to lnd uris for ${peerPubKey}`); + if (uris) { + await this.connectPeerAddreses(uris); } + + await this.openChannelSync(peerPubKey, units, pushUnits); } /** diff --git a/lib/swaps/SwapClient.ts b/lib/swaps/SwapClient.ts index 248e9f3d8..4a09c4aff 100644 --- a/lib/swaps/SwapClient.ts +++ b/lib/swaps/SwapClient.ts @@ -278,12 +278,12 @@ abstract class SwapClient extends EventEmitter { * optional currency and optional lndUris. */ public abstract async openChannel( - { peerIdentifier, units, currency, lndUris, pushUnits }: + { peerIdentifier, units, currency, uris, pushUnits }: { peerIdentifier: string, units: number, currency?: string, - lndUris?: string[], + uris?: string[], pushUnits?: number, }, ): Promise; diff --git a/lib/swaps/SwapClientManager.ts b/lib/swaps/SwapClientManager.ts index 76be8e09d..2e6cadf91 100644 --- a/lib/swaps/SwapClientManager.ts +++ b/lib/swaps/SwapClientManager.ts @@ -413,16 +413,11 @@ class SwapClientManager extends EventEmitter { amount: pushAmount, }); + let uris: string[] | undefined; if (isLndClient(swapClient)) { - const lndUris = peer.getLndUris(currency); - if (!lndUris) { - throw new Error('unable to get lnd listening uris'); - } - await swapClient.openChannel({ peerIdentifier, units, lndUris, pushUnits }); - return; + uris = peer.getLndUris(currency); } - // fallback to raiden for all non-lnd currencies - await swapClient.openChannel({ peerIdentifier, units, currency }); + await swapClient.openChannel({ peerIdentifier, currency, units, uris, pushUnits }); } /** diff --git a/test/jest/LndClient.spec.ts b/test/jest/LndClient.spec.ts index 68930b2ed..e41db518d 100644 --- a/test/jest/LndClient.spec.ts +++ b/test/jest/LndClient.spec.ts @@ -50,7 +50,7 @@ describe('LndClient', () => { afterEach(async () => { jest.clearAllMocks(); - await lnd.close(); + lnd.close(); }); describe('openChannel', () => { @@ -63,25 +63,6 @@ describe('LndClient', () => { `${peerPubKey}@${externalIp2}`, ]; - test('it throws when connectPeer fails', async () => { - expect.assertions(3); - lnd['connectPeer'] = jest.fn().mockImplementation(() => { - throw new Error('connectPeer failed'); - }); - try { - await lnd.openChannel({ - units, - peerIdentifier: peerPubKey, - lndUris: [lndListeningUris[0]], - }); - } catch (e) { - expect(e).toMatchSnapshot(); - } - expect(lnd['connectPeer']).toHaveBeenCalledTimes(1); - expect(lnd['connectPeer']) - .toHaveBeenCalledWith(peerPubKey, externalIp1); - }); - test('it tries all 2 lnd uris when connectPeer to first one fails', async () => { expect.assertions(3); lnd['openChannelSync'] = jest.fn().mockReturnValue(Promise.resolve()); @@ -96,7 +77,7 @@ describe('LndClient', () => { await lnd.openChannel({ units, peerIdentifier: peerPubKey, - lndUris: lndListeningUris, + uris: lndListeningUris, }); expect(lnd['connectPeer']).toHaveBeenCalledTimes(2); expect(lnd['connectPeer']) @@ -116,7 +97,7 @@ describe('LndClient', () => { await lnd.openChannel({ units, peerIdentifier: peerPubKey, - lndUris: lndListeningUris, + uris: lndListeningUris, }); expect(lnd['connectPeer']).toHaveBeenCalledTimes(1); expect(lnd['connectPeer']) @@ -138,7 +119,7 @@ describe('LndClient', () => { units, pushUnits, peerIdentifier: peerPubKey, - lndUris: lndListeningUris, + uris: lndListeningUris, }); expect(lnd['connectPeer']).toHaveBeenCalledTimes(1); expect(lnd['connectPeer']) @@ -158,7 +139,7 @@ describe('LndClient', () => { await lnd.openChannel({ units, peerIdentifier: peerPubKey, - lndUris: lndListeningUris, + uris: lndListeningUris, }); expect(lnd['connectPeer']).toHaveBeenCalledTimes(1); expect(lnd['connectPeer']) @@ -177,7 +158,7 @@ describe('LndClient', () => { await lnd.openChannel({ units, peerIdentifier: peerPubKey, - lndUris: lndListeningUris, + uris: lndListeningUris, }); } catch (e) { expect(e).toMatchSnapshot(); diff --git a/test/jest/SwapClientManager.spec.ts b/test/jest/SwapClientManager.spec.ts index 400cc67da..0eaa39935 100644 --- a/test/jest/SwapClientManager.spec.ts +++ b/test/jest/SwapClientManager.spec.ts @@ -236,20 +236,6 @@ describe('Swaps.SwapClientManager', () => { } }); - test('it fails lnd without lndUris', async () => { - expect.assertions(1); - const currency = 'BTC'; - const amount = 16000000; - swapClientManager = new SwapClientManager(config, loggers, unitConverter); - peer.getLndUris = jest.fn().mockReturnValue(undefined); - await swapClientManager.init(db.models); - try { - await swapClientManager.openChannel({ peer, currency, amount }); - } catch (e) { - expect(e).toMatchSnapshot(); - } - }); - test('it opens a channel using lnd', async () => { const currency = 'BTC'; const amount = 16000000; @@ -268,7 +254,7 @@ describe('Swaps.SwapClientManager', () => { expect(mockLndOpenChannel).toHaveBeenCalledWith( expect.objectContaining({ units: amount, - lndUris: lndListeningUris, + uris: lndListeningUris, peerIdentifier: peerLndPubKey, }), ); @@ -295,6 +281,8 @@ describe('Swaps.SwapClientManager', () => { currency, units: expectedUnits, peerIdentifier: peerRaidenAddress, + // uris: undefined, + pushUnits: 0, }), ); }); diff --git a/test/jest/__snapshots__/LndClient.spec.ts.snap b/test/jest/__snapshots__/LndClient.spec.ts.snap index de6fce01b..b787c475c 100644 --- a/test/jest/__snapshots__/LndClient.spec.ts.snap +++ b/test/jest/__snapshots__/LndClient.spec.ts.snap @@ -1,7 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`LndClient openChannel it throws when connectPeer fails 1`] = `[Error: could not connect to lnd uris for 02f8895eb03c37b2665415be4d83b20228acc0abc55ebf6728565141c66cfc164a]`; - exports[`LndClient openChannel it throws when openchannel fails 1`] = `[Error: openChannelSync error]`; exports[`LndClient sendPayment it rejects upon sendPaymentSync error 1`] = ` diff --git a/test/jest/__snapshots__/SwapClientManager.spec.ts.snap b/test/jest/__snapshots__/SwapClientManager.spec.ts.snap index 111fa58c8..65c59b7cf 100644 --- a/test/jest/__snapshots__/SwapClientManager.spec.ts.snap +++ b/test/jest/__snapshots__/SwapClientManager.spec.ts.snap @@ -1,7 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Swaps.SwapClientManager openChannel it fails lnd without lndUris 1`] = `[Error: unable to get lnd listening uris]`; - exports[`Swaps.SwapClientManager openChannel it fails without peerSwapClientPubKey 1`] = `[Error: peer not connected to swap client]`; exports[`Swaps.SwapClientManager openChannel it fails without swap client 1`] = `