Skip to content

Commit

Permalink
Merge pull request #1163 from ExchangeUnion/fix/cltv-limit
Browse files Browse the repository at this point in the history
fix(swaps): set cltvLimit correctly
  • Loading branch information
sangaman authored Aug 21, 2019
2 parents 009865d + 31b41a1 commit bf870ad
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 17 deletions.
2 changes: 1 addition & 1 deletion lib/db/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export type CurrencyInstance = CurrencyAttributes & Sequelize.Instance<CurrencyA

/* SwapDeal */
export type SwapDealFactory = Pick<SwapDeal, Exclude<keyof SwapDeal,
'makerToTakerRoutes' | 'price' | 'pairId' | 'isBuy' | 'takerUnits' | 'makerUnits'>>;
'takerMaxTimeLock' | 'price' | 'pairId' | 'isBuy' | 'takerUnits' | 'makerUnits'>>;

export type SwapDealAttributes = SwapDealFactory & {
/** The internal db node id of the counterparty peer for this swap deal. */
Expand Down
6 changes: 4 additions & 2 deletions lib/lndclient/LndClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,10 +361,11 @@ class LndClient extends SwapClient {
rHash: deal.rHash,
destination: deal.takerPubKey!,
amount: deal.takerAmount,
finalCltvDelta: deal.takerCltvDelta,
// Enforcing the maximum duration/length of the payment by
// specifying the cltvLimit.
finalCltvDelta: deal.takerCltvDelta,
cltvLimit: deal.makerCltvDelta,
// TODO: investigate why we need to add 3 blocks - if not lnd says route not found
cltvLimit: deal.takerMaxTimeLock! + 3,
});
}
const preimage = await this.executeSendRequest(request);
Expand Down Expand Up @@ -408,6 +409,7 @@ class LndClient extends SwapClient {
private executeSendRequest = async (
request: lndrpc.SendRequest,
): Promise<string> => {
this.logger.trace(`sending payment with ${JSON.stringify(request.toObject())}`);
let sendPaymentResponse: lndrpc.SendResponse;
try {
sendPaymentResponse = await this.sendPaymentSync(request);
Expand Down
12 changes: 7 additions & 5 deletions lib/raidenclient/RaidenClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,26 +165,28 @@ class RaidenClient extends SwapClient {
public sendPayment = async (deal: SwapDeal): Promise<string> => {
assert(deal.state === SwapState.Active);
assert(deal.destination);
let amount = 0;
let tokenAddress;
let amount: number;
let tokenAddress: string;
let lock_timeout: number | undefined;
if (deal.role === SwapRole.Maker) {
// we are the maker paying the taker
amount = deal.takerUnits;
tokenAddress = this.tokenAddresses.get(deal.takerCurrency);
tokenAddress = this.tokenAddresses.get(deal.takerCurrency)!;
} else {
// we are the taker paying the maker
amount = deal.makerUnits;
tokenAddress = this.tokenAddresses.get(deal.makerCurrency);
tokenAddress = this.tokenAddresses.get(deal.makerCurrency)!;
lock_timeout = deal.makerCltvDelta!;
}
if (!tokenAddress) {
throw(errors.TOKEN_ADDRESS_NOT_FOUND);
}
const tokenPaymentResponse = await this.tokenPayment({
amount,
lock_timeout,
token_address: tokenAddress,
target_address: deal.destination!,
secret_hash: deal.rHash,
lock_timeout: deal.makerCltvDelta,
});
return this.sanitizeTokenPaymentResponse(tokenPaymentResponse);
}
Expand Down
10 changes: 6 additions & 4 deletions lib/swaps/Swaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import SwapRepository from './SwapRepository';
import { OwnOrder, PeerOrder } from '../orderbook/types';
import assert from 'assert';
import { SwapDealInstance } from '../db/types';
import { SwapDeal, SwapSuccess, SanitySwap, ResolveRequest } from './types';
import { SwapDeal, SwapSuccess, SanitySwap, ResolveRequest, Route } from './types';
import { generatePreimageAndHash, setTimeoutPromise } from '../utils/utils';
import { PacketType } from '../p2p/packets';
import SwapClientManager from './SwapClientManager';
Expand Down Expand Up @@ -519,8 +519,9 @@ class Swaps extends EventEmitter {
return false;
}

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

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

const routeAbsoluteTimeLock = deal.makerToTakerRoutes[0].getTotalTimeLock();
const routeAbsoluteTimeLock = makerToTakerRoutes[0].getTotalTimeLock();
this.logger.debug(`choosing a route with total time lock of ${routeAbsoluteTimeLock}`);
const routeTimeLock = routeAbsoluteTimeLock - height;
this.logger.debug(`route time lock: ${routeTimeLock}`);
deal.takerMaxTimeLock = routeTimeLock;

const makerClientLockBuffer = this.swapClientManager.get(makerCurrency)!.lockBuffer;
this.logger.debug(`maker client lock buffer: ${makerClientLockBuffer}`);
Expand Down
4 changes: 2 additions & 2 deletions lib/swaps/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ export type SwapDeal = {
rHash: string;
/** The hex-encoded preimage. */
rPreimage?: string;
/** The routes the maker should use to send to the taker. */
makerToTakerRoutes?: Route[];
/** The maximum time lock from the maker to the taker in blocks. */
takerMaxTimeLock?: number;
/** The identifier for the payment channel network node we should pay to complete the swap. */
destination?: string;
/** The time when we created this swap deal locally. */
Expand Down
2 changes: 1 addition & 1 deletion test/jest/LndClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ describe('LndClient', () => {
amount: deal.takerAmount,
destination: deal.takerPubKey,
rHash: deal.rHash,
cltvLimit: deal.makerCltvDelta,
cltvLimit: deal.takerMaxTimeLock + 3,
finalCltvDelta: deal.takerCltvDelta,
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/jest/integration/Swaps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe('Swaps', () => {
expect(dealAccepted).toEqual(false);
});

test('it rejects upon 0 makerToTakerRoutes found', async () => {
test('it rejects upon 0 maker to taker routes found', async () => {
lndBtc.getRoutes = jest.fn().mockReturnValue([]);
swapClientManager.get = jest.fn().mockImplementation((currency) => {
if (currency === takerCurrency) {
Expand Down
2 changes: 1 addition & 1 deletion test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,6 @@ export const getValidDeal = () => {
state: 0,
role: 1,
createTime: 1559120485138,
makerToTakerRoutes: [{ getTotalTimeLock: () => {} }],
takerMaxTimeLock: 100,
};
};

0 comments on commit bf870ad

Please sign in to comment.