Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(swaps): change getRoutes to getRoute #1177

Merged
merged 1 commit into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions lib/lndclient/LndClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ class LndClient extends SwapClient {
return this.unaryCall<lndrpc.ListChannelsRequest, lndrpc.ListChannelsResponse>('listChannels', new lndrpc.ListChannelsRequest());
}

public getRoutes = async (units: number, destination: string, _currency: string, finalCltvDelta = this.lockBuffer) => {
public getRoute = async (units: number, destination: string, _currency: string, finalCltvDelta = this.lockBuffer) => {
const request = new lndrpc.QueryRoutesRequest();
request.setAmt(units);
request.setFinalCltvDelta(finalCltvDelta);
Expand All @@ -551,21 +551,27 @@ class LndClient extends SwapClient {
fee.setFixed(Math.floor(MAXFEE * request.getAmt()));
request.setFeeLimit(fee);

let route: lndrpc.Route | undefined;

try {
const routes = (await this.queryRoutes(request)).getRoutesList();
this.logger.debug(`got ${routes.length} route(s) to destination ${destination}: ${routes}, finalCltvDelta: ${finalCltvDelta}`);
return routes;
// QueryRoutes no longer returns more than one route
route = (await this.queryRoutes(request)).getRoutesList()[0];
} catch (err) {
if (typeof err.message === 'string' && (
err.message.includes('unable to find a path to destination') ||
err.message.includes('target not found')
if (typeof err.message !== 'string' || (
!err.message.includes('unable to find a path to destination') &&
!err.message.includes('target not found')
)) {
return [];
} else {
this.logger.error(`error calling queryRoutes to ${destination}, amount ${units}, finalCltvDelta ${finalCltvDelta}`, err);
throw err;
}
}

if (route) {
this.logger.debug(`found a route to ${destination} for ${units} units with finalCltvDelta ${finalCltvDelta}: ${route}`);
} else {
this.logger.debug(`could not find a route to ${destination} for ${units} units with finalCltvDelta ${finalCltvDelta}: ${route}`);
}
return route;
}

/**
Expand Down
10 changes: 5 additions & 5 deletions lib/raidenclient/RaidenClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class RaidenClient extends SwapClient {
// not implemented, raiden does not use invoices
}

public getRoutes = async (units: number, destination: string, currency: string) => {
public getRoute = async (units: number, destination: string, currency: string) => {
// a query routes call is not currently provided by raiden

/** A placeholder route value that assumes a fixed lock time of 100 Raiden's blocks. */
Expand All @@ -229,17 +229,17 @@ class RaidenClient extends SwapClient {
const balance = channel.balance;
if (balance >= units) {
this.logger.debug(`found a direct channel for ${currency} to ${destination} with ${balance} balance`);
return [placeholderRoute];
return placeholderRoute;
} else {
this.logger.warn(`direct channel found for ${currency} to ${destination} with balance of ${balance} is insufficient for ${units})`);
return []; // we have a direct channel but it doesn't have enough balance, return no routes
return undefined; // we have a direct channel but it doesn't have enough balance, return no route
}
}
}
this.logger.warn(`no direct channel found for ${currency} to ${destination}`);
return []; // no direct channels, return no routes
return undefined; // no direct channels, return no route
} else {
return [placeholderRoute];
return placeholderRoute;
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/swaps/SwapClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ abstract class SwapClient extends EventEmitter {
* @param destination the identifier for the receiving node
* @returns routes
*/
public abstract async getRoutes(units: number, destination: string, currency: string, finalCltvDelta?: number): Promise<Route[]>;
public abstract async getRoute(units: number, destination: string, currency: string, finalCltvDelta?: number): Promise<Route | undefined>;

/**
* @param units the amount of the invoice denominated in the smallest units supported by its currency
Expand Down
14 changes: 7 additions & 7 deletions lib/swaps/Swaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,14 @@ class Swaps extends EventEmitter {
throw SwapFailureReason.SwapClientNotSetup;
}

let routes;
let route: Route | undefined;
try {
routes = await swapClient.getRoutes(makerUnits, destination, makerCurrency);
route = await swapClient.getRoute(makerUnits, destination, makerCurrency);
} catch (err) {
throw SwapFailureReason.UnexpectedClientError;
}

if (routes.length === 0) {
if (!route) {
throw SwapFailureReason.NoRouteFound;
}
}
Expand Down Expand Up @@ -519,9 +519,9 @@ class Swaps extends EventEmitter {
return false;
}

let makerToTakerRoutes: Route[];
let makerToTakerRoute: Route | undefined;
try {
makerToTakerRoutes = await takerSwapClient.getRoutes(takerUnits, takerIdentifier, deal.takerCurrency, deal.takerCltvDelta);
makerToTakerRoute = await takerSwapClient.getRoute(takerUnits, takerIdentifier, deal.takerCurrency, deal.takerCltvDelta);
} catch (err) {
this.failDeal(deal, SwapFailureReason.UnexpectedClientError, err.message);
await this.sendErrorToPeer({
Expand All @@ -534,7 +534,7 @@ class Swaps extends EventEmitter {
return false;
}

if (makerToTakerRoutes.length === 0) {
if (!makerToTakerRoute) {
this.failDeal(deal, SwapFailureReason.NoRouteFound, 'Unable to find route to destination');
await this.sendErrorToPeer({
peer,
Expand Down Expand Up @@ -564,7 +564,7 @@ class Swaps extends EventEmitter {
if (height) {
this.logger.debug(`got ${takerCurrency} block height of ${height}`);

const routeAbsoluteTimeLock = makerToTakerRoutes[0].getTotalTimeLock();
const routeAbsoluteTimeLock = makerToTakerRoute.getTotalTimeLock();
const routeLockDuration = routeAbsoluteTimeLock - height;
const routeLockHours = Math.round(routeLockDuration / takerSwapClient.minutesPerBlock);
this.logger.debug(`found route to taker with total lock duration of ${routeLockDuration} ${takerCurrency} blocks (~${routeLockHours}h)`);
Expand Down
28 changes: 14 additions & 14 deletions test/integration/Swaps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('Swaps.Integration', () => {
let swapClientManager: any;
let peer: Peer;
let sandbox: SinonSandbox;
let getRoutesResponse;
let getRouteResponse;

before(async () => {
db = new DB(loggers.db);
Expand All @@ -116,23 +116,23 @@ describe('Swaps.Integration', () => {
pool = sandbox.createStubInstance(Pool) as any;
pool.addReputationEvent = () => Promise.resolve(true);
pool.getPeer = () => peer;
// queryRoutes response
getRoutesResponse = () => {
// getRoute response
getRouteResponse = () => {
return Promise.resolve({
getRoutesList: () => [1],
} as any);
getTotalTimeLock: () => 1,
});
};
swapClientManager = sandbox.createStubInstance(SwapClientManager) as any;
swapClientManager['swapClients'] = new Map<string, SwapClient>();
const btcSwapClient = sandbox.createStubInstance(SwapClient) as any;
btcSwapClient['removeInvoice'] = async () => {};
btcSwapClient.getRoutes = getRoutesResponse;
btcSwapClient.getRoute = getRouteResponse;
btcSwapClient.isConnected = () => true;
swapClientManager['swapClients'].set('BTC', btcSwapClient);
const ltcSwapClient = sandbox.createStubInstance(SwapClient) as any;
ltcSwapClient['removeInvoice'] = async () => {};
ltcSwapClient.isConnected = () => true;
ltcSwapClient.getRoutes = getRoutesResponse;
ltcSwapClient.getRoute = getRouteResponse;
swapClientManager['swapClients'].set('LTC', ltcSwapClient);
swapClientManager.get = (currency: string) => {
const client = swaps.swapClientManager['swapClients'].get(currency);
Expand Down Expand Up @@ -196,26 +196,26 @@ describe('Swaps.Integration', () => {

it('will reject if unable to retrieve routes', async () => {
const noRoutesFound = () => {
return Promise.resolve([]);
return Promise.resolve(undefined);
};
let btcSwapClient = swapClientManager.get('BTC');
btcSwapClient!.getRoutes = noRoutesFound;
btcSwapClient!.getRoute = noRoutesFound;
swapClientManager['swapClients'].set('BTC', btcSwapClient!);
let ltcSwapClient = swapClientManager.get('LTC');
ltcSwapClient!.getRoutes = noRoutesFound;
ltcSwapClient!.getRoute = noRoutesFound;
swapClientManager['swapClients'].set('LTC', ltcSwapClient!);
await expect(swaps.executeSwap(validMakerOrder(), validTakerOrder()))
expect(swaps.executeSwap(validMakerOrder(), validTakerOrder()))
.to.eventually.be.rejected.and.equal(SwapFailureReason.NoRouteFound);
const rejectsWithUnknownError = () => {
return Promise.reject('UNKNOWN');
};
btcSwapClient = swapClientManager.get('BTC');
btcSwapClient!.getRoutes = rejectsWithUnknownError;
btcSwapClient!.getRoute = rejectsWithUnknownError;
swapClientManager['swapClients'].set('BTC', btcSwapClient!);
ltcSwapClient = swapClientManager.get('LTC');
ltcSwapClient!.getRoutes = rejectsWithUnknownError;
ltcSwapClient!.getRoute = rejectsWithUnknownError;
swapClientManager['swapClients'].set('LTC', ltcSwapClient!);
await expect(swaps.executeSwap(validMakerOrder(), validTakerOrder()))
expect(swaps.executeSwap(validMakerOrder(), validTakerOrder()))
.to.eventually.be.rejected.and.equal(SwapFailureReason.UnexpectedClientError);
});

Expand Down
22 changes: 11 additions & 11 deletions test/jest/integration/Swaps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('Swaps', () => {
});

test('it rejects upon 0 maker to taker routes found', async () => {
lndBtc.getRoutes = jest.fn().mockReturnValue([]);
lndBtc.getRoute = jest.fn().mockReturnValue(undefined);
swapClientManager.get = jest.fn().mockImplementation((currency) => {
if (currency === takerCurrency) {
return lndBtc;
Expand All @@ -176,9 +176,9 @@ describe('Swaps', () => {
});

test('it rejects upon failed getHeight request', async () => {
lndBtc.getRoutes = jest.fn().mockReturnValue([
{ getTotalTimeLock: () => 1543845 },
]);
lndBtc.getRoute = jest.fn().mockReturnValue({
getTotalTimeLock: () => 1543845,
});
swapClientManager.get = jest.fn().mockImplementation((currency) => {
if (currency === takerCurrency) {
return lndBtc;
Expand Down Expand Up @@ -207,9 +207,9 @@ describe('Swaps', () => {
lndLtc.addInvoice = jest.fn().mockImplementation(() => {
throw new Error('addInvoice failure');
});
lndBtc.getRoutes = jest.fn().mockReturnValue([
{ getTotalTimeLock: () => 1543845 },
]);
lndBtc.getRoute = jest.fn().mockReturnValue({
getTotalTimeLock: () => 1543845,
});
lndBtc.getHeight = jest.fn().mockReturnValue(1543701);
swapClientManager.get = jest.fn().mockImplementation((currency) => {
if (currency === takerCurrency) {
Expand Down Expand Up @@ -237,9 +237,9 @@ describe('Swaps', () => {

test('it accepts deal', async () => {
const peerLndBtcPubKey = '02d9fb6c41686b7bee95958bde0ada72c249b8fa9928987c93d839225d6883e6c0';
lndBtc.getRoutes = jest.fn().mockReturnValue([
{ getTotalTimeLock: () => 1543845 },
]);
lndBtc.getRoute = jest.fn().mockReturnValue({
getTotalTimeLock: () => 1543845,
});
lndBtc.getHeight = jest.fn().mockReturnValue(1543701);
Object.defineProperty(lndBtc, 'minutesPerBlock', {
get: () => { return 10; },
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('Swaps', () => {
peer.sendPacket = jest.fn();
const dealAccepted = await swaps.acceptDeal(orderToAccept, swapRequestPacket, peer);
expect(dealAccepted).toEqual(true);
expect(lndBtc.getRoutes).toHaveBeenCalledWith(
expect(lndBtc.getRoute).toHaveBeenCalledWith(
1000,
peerLndBtcPubKey,
takerCurrency,
Expand Down