Skip to content

Commit

Permalink
feat(lnd): attempt openchannel when connect fails
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sangaman committed Apr 13, 2020
1 parent 06fb107 commit d0589ea
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 61 deletions.
13 changes: 6 additions & 7 deletions lib/lndclient/LndClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> => {
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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/swaps/SwapClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
Expand Down
11 changes: 3 additions & 8 deletions lib/swaps/SwapClientManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

/**
Expand Down
31 changes: 6 additions & 25 deletions test/jest/LndClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('LndClient', () => {

afterEach(async () => {
jest.clearAllMocks();
await lnd.close();
lnd.close();
});

describe('openChannel', () => {
Expand All @@ -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());
Expand All @@ -96,7 +77,7 @@ describe('LndClient', () => {
await lnd.openChannel({
units,
peerIdentifier: peerPubKey,
lndUris: lndListeningUris,
uris: lndListeningUris,
});
expect(lnd['connectPeer']).toHaveBeenCalledTimes(2);
expect(lnd['connectPeer'])
Expand All @@ -116,7 +97,7 @@ describe('LndClient', () => {
await lnd.openChannel({
units,
peerIdentifier: peerPubKey,
lndUris: lndListeningUris,
uris: lndListeningUris,
});
expect(lnd['connectPeer']).toHaveBeenCalledTimes(1);
expect(lnd['connectPeer'])
Expand All @@ -138,7 +119,7 @@ describe('LndClient', () => {
units,
pushUnits,
peerIdentifier: peerPubKey,
lndUris: lndListeningUris,
uris: lndListeningUris,
});
expect(lnd['connectPeer']).toHaveBeenCalledTimes(1);
expect(lnd['connectPeer'])
Expand All @@ -158,7 +139,7 @@ describe('LndClient', () => {
await lnd.openChannel({
units,
peerIdentifier: peerPubKey,
lndUris: lndListeningUris,
uris: lndListeningUris,
});
expect(lnd['connectPeer']).toHaveBeenCalledTimes(1);
expect(lnd['connectPeer'])
Expand All @@ -177,7 +158,7 @@ describe('LndClient', () => {
await lnd.openChannel({
units,
peerIdentifier: peerPubKey,
lndUris: lndListeningUris,
uris: lndListeningUris,
});
} catch (e) {
expect(e).toMatchSnapshot();
Expand Down
18 changes: 3 additions & 15 deletions test/jest/SwapClientManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -268,7 +254,7 @@ describe('Swaps.SwapClientManager', () => {
expect(mockLndOpenChannel).toHaveBeenCalledWith(
expect.objectContaining({
units: amount,
lndUris: lndListeningUris,
uris: lndListeningUris,
peerIdentifier: peerLndPubKey,
}),
);
Expand All @@ -295,6 +281,8 @@ describe('Swaps.SwapClientManager', () => {
currency,
units: expectedUnits,
peerIdentifier: peerRaidenAddress,
// uris: undefined,
pushUnits: 0,
}),
);
});
Expand Down
2 changes: 0 additions & 2 deletions test/jest/__snapshots__/LndClient.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -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`] = `
Expand Down
2 changes: 0 additions & 2 deletions test/jest/__snapshots__/SwapClientManager.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -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`] = `
Expand Down

0 comments on commit d0589ea

Please sign in to comment.