Skip to content

Commit

Permalink
refactor(swaps): change getRoutes to getRoute (#1177)
Browse files Browse the repository at this point in the history 
refactor(swaps): change getRoutes to getRoute
  • Loading branch information
@sangaman
sangaman authored Sep 11, 2019
2 parents 73e5c2f + fdf9f74 commit b7861b8
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 47 deletions.
24 changes: 15 additions & 9 deletions lib/lndclient/LndClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,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 @@ -631,21 +631,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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit b7861b8

Please sign in to comment.