Skip to content

Commit

Permalink
feat: disable raiden currencies w/out direct chan
Browse files Browse the repository at this point in the history
This commit attempts to ensure that we have a direct channel with peers
for raiden currencies before exchanging orders with that peer for any
pairs involving that currency.

When a peer advertises a trading pair, either via initial handshake or
on a node state update, we check that we have a direct channel for any
raiden currencies before activating advertised pairs. If the trading
pair is not activated, then we do not send or accept orders for that
pair with the peer.

An existing timer with an interval of one minute is used for each peer
to periodically check if inactive currencies and pairs can be activated
by performing the same checks described above, including the direct
channel check for raiden currencies.

Closes #1027.
  • Loading branch information
sangaman committed Feb 11, 2020
1 parent d723096 commit e050cbd
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 54 deletions.
88 changes: 45 additions & 43 deletions lib/orderbook/OrderBook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ class OrderBook extends EventEmitter {
const { minQuantity } = this.thresholds;
return order.quantity >= minQuantity;
}

/**
* Checks that a currency advertised by a peer is known to us, has a swap client identifier,
* and that their token identifier matches ours.
Expand Down Expand Up @@ -583,7 +584,8 @@ class OrderBook extends EventEmitter {

this.emit('ownOrder.added', order);

this.broadcastOrder(order);
const outgoingOrder = OrderBook.createOutgoingOrder(order);
this.pool.broadcastOrder(outgoingOrder);
return true;
}

Expand Down Expand Up @@ -806,52 +808,62 @@ class OrderBook extends EventEmitter {
return !peer.disabledCurrencies.has(baseCurrency) && !peer.disabledCurrencies.has(quoteCurrency);
});

if (this.nosanityswaps) {
// we have disabled sanity checks, so assume all pairs should be activated
pairIdsToVerify.forEach(peer.activatePair);
return;
}

// identify the unique currencies we need to verify for specified trading pairs
const currenciesToVerify = new Set<string>();
/** A map between currencies we are verifying and whether the currency is swappable. */
const currenciesToVerify = new Map<string, boolean>();
pairIdsToVerify.forEach((pairId) => {
const [baseCurrency, quoteCurrency] = pairId.split('/');
if (!peer.verifiedCurrencies.has(baseCurrency)) {
currenciesToVerify.add(baseCurrency);
if (!peer.isCurrencyActive(baseCurrency)) {
currenciesToVerify.set(baseCurrency, true);
}
if (!peer.verifiedCurrencies.has(quoteCurrency)) {
currenciesToVerify.add(quoteCurrency);
if (!peer.isCurrencyActive(quoteCurrency)) {
currenciesToVerify.set(quoteCurrency, true);
}
});

const sanitySwapPromises: Promise<void>[] = [];
currenciesToVerify.forEach((_, currency) => {
if (!this.swaps.swapClientManager.hasRouteToPeer(currency, peer)) {
currenciesToVerify.set(currency, false);
}
});

// Set a time limit for all sanity swaps to complete.
const sanitySwapTimeout = setTimeoutPromise(OrderBook.MAX_SANITY_SWAP_TIME, false);
if (!this.nosanityswaps) {
const sanitySwapPromises: Promise<void>[] = [];

// Set a time limit for all sanity swaps to complete.
const sanitySwapTimeout = setTimeoutPromise(OrderBook.MAX_SANITY_SWAP_TIME, false);

currenciesToVerify.forEach((swappable, currency) => {
if (swappable && this.currencyInstances.has(currency)) {
// perform sanity swaps for each of the currencies that we support
const sanitySwapPromise = new Promise<void>(async (resolve) => {
// success resolves to true if the sanity swap succeeds before the timeout
const success = await Promise.race([this.swaps.executeSanitySwap(currency, peer), sanitySwapTimeout]);
if (!success) {
currenciesToVerify.set(currency, false);
}
resolve();
});
sanitySwapPromises.push(sanitySwapPromise);
}
});

currenciesToVerify.forEach((currency) => {
if (this.currencyInstances.has(currency)) {
// perform sanity swaps for each of the currencies that we support
const sanitySwapPromise = new Promise<void>(async (resolve) => {
// success resolves to true if the sanity swap succeeds before the timeout
const success = await Promise.race([this.swaps.executeSanitySwap(currency, peer), sanitySwapTimeout]);
if (success) {
peer.verifiedCurrencies.add(currency);
}
resolve();
});
sanitySwapPromises.push(sanitySwapPromise);
// wait for all sanity swaps to finish or timeout
await Promise.all(sanitySwapPromises);
}

// activate verified currencies
currenciesToVerify.forEach((swappable, currency) => {
if (swappable) {
peer.activateCurrency(currency);
}
});

// wait for all sanity swaps to finish or timeout
await Promise.all(sanitySwapPromises);

// activate pairs that have had both currencies verified
// activate pairs that have both currencies active
const activationPromises: Promise<void>[] = [];
pairIdsToVerify.forEach(async (pairId) => {
pairIdsToVerify.forEach((pairId) => {
const [baseCurrency, quoteCurrency] = pairId.split('/');
if (peer.verifiedCurrencies.has(baseCurrency) && peer.verifiedCurrencies.has(quoteCurrency)) {
if (peer.isCurrencyActive(baseCurrency) && peer.isCurrencyActive(quoteCurrency)) {
activationPromises.push(peer.activatePair(pairId));
}
});
Expand All @@ -876,16 +888,6 @@ class OrderBook extends EventEmitter {
await peer.sendOrders(outgoingOrders, reqId);
}

/**
* Create an outgoing order and broadcast it to all peers.
*/
private broadcastOrder = (order: OwnOrder) => {
if (this.swaps && this.swaps.isPairSupported(order.pairId)) {
const outgoingOrder = OrderBook.createOutgoingOrder(order);
this.pool.broadcastOrder(outgoingOrder);
}
}

public stampOwnOrder = (order: OwnLimitOrder): OwnOrder => {
const id = uuidv1();
// verify localId isn't duplicated. use global id if blank
Expand Down
24 changes: 17 additions & 7 deletions lib/p2p/Peer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ class Peer extends EventEmitter {
public active = false;
/** Timer to periodically call getNodes #402 */
public discoverTimer?: NodeJS.Timer;
/** Currencies that we have verified we can swap with this peer. */
public verifiedCurrencies = new Set<string>();
/**
* Currencies that we cannot swap because we are missing a swap client identifier or because the
* peer's token identifier for this currency does not match ours - for example this may happen
Expand All @@ -87,6 +85,8 @@ class Peer extends EventEmitter {
private status = PeerStatus.New;
/** Trading pairs advertised by this peer which we have verified that we can swap. */
private activePairs = new Set<string>();
/** Currencies that we have verified we can swap with this peer. */
private activeCurrencies = new Set<string>();
private socket?: Socket;
private readonly parser: Parser;
/** Timer to retry connection to peer after the previous attempt failed. */
Expand Down Expand Up @@ -388,17 +388,25 @@ class Peer extends EventEmitter {
await this.sendPacket(packet);
}

public disableCurrency = (currency: string) => {
if (!this.disabledCurrencies.has(currency)) {
this.disabledCurrencies.add(currency);
this.verifiedCurrencies.delete(currency);
public deactivateCurrency = (currency: string) => {
if (this.activeCurrencies.has(currency)) {
this.activeCurrencies.delete(currency);
this.activePairs.forEach((activePairId) => {
const [baseCurrency, quoteCurrency] = activePairId.split('/');
if (baseCurrency === currency || quoteCurrency === currency) {
this.deactivatePair(activePairId);
}
});
this.logger.debug(`disabled ${currency} for peer ${this.label}`);
this.logger.debug(`deactivated ${currency} for peer ${this.label}`);
}
}

public activateCurrency = (currency: string) => this.activeCurrencies.add(currency);

public disableCurrency = (currency: string) => {
if (!this.disabledCurrencies.has(currency)) {
this.disabledCurrencies.add(currency);
this.deactivateCurrency(currency);
}
}

Expand Down Expand Up @@ -435,6 +443,8 @@ class Peer extends EventEmitter {

public isPairActive = (pairId: string) => this.activePairs.has(pairId);

public isCurrencyActive = (currency: string) => this.activeCurrencies.has(currency);

/**
* Gets lnd client's listening uris for the provided currency.
* @param currency
Expand Down
21 changes: 21 additions & 0 deletions lib/swaps/SwapClientManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,27 @@ class SwapClientManager extends EventEmitter {
await swapClient.openChannel({ peerIdentifier, units, currency });
}

public hasRouteToPeer = (currency: string, peer: Peer) => {
const swapClient = this.get(currency);
if (!swapClient) {
return false;
}
let hasRoute = false;
if (isRaidenClient(swapClient)) {
// temporary check to ensure we have a direct channel
if (peer.raidenAddress && swapClient.getRoute(1, peer.raidenAddress, currency)) {
// a route found means we have a direct channel
hasRoute = true;
}
} else if (isLndClient(swapClient)) {
// lnd doesn't currently have a way to see if any route exists, regardless of balance
// for example, if we have a direct channel to peer but no balance in the channel and
// no other routes, QueryRoutes will return nothing as of lnd v0.8.1
hasRoute = true;
}
return hasRoute;
}

private bind = () => {
for (const [currency, swapClient] of this.swapClients.entries()) {
if (isLndClient(swapClient)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/swaps/Swaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,7 @@ class Swaps extends EventEmitter {
// something is wrong with swaps for this currency with this peer
if (failedCurrency) {
try {
this.pool.getPeer(deal.peerPubKey).disableCurrency(failedCurrency);
this.pool.getPeer(deal.peerPubKey).deactivateCurrency(failedCurrency);
} catch (err) {
this.logger.debug(`could not disable currency ${failedCurrency} for peer ${deal.peerPubKey}`);
}
Expand Down
24 changes: 21 additions & 3 deletions test/jest/Orderbook.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ jest.mock('../../lib/db/DB', () => {
});
const advertisedPairs = ['LTC/BTC', 'WETH/BTC'];
const mockActivatePair = jest.fn();
const mockActivateCurrency = jest.fn();
const mockGetIdentifier = jest.fn(() => 'pubkeyoraddress');
const tokenIdentifiers: any = {
BTC: 'bitcoin-regtest',
Expand All @@ -52,11 +53,17 @@ const tokenIdentifiers: any = {
const mockPeerGetTokenIdentifer = jest.fn((currency: string) => tokenIdentifiers[currency]);
jest.mock('../../lib/p2p/Peer', () => {
return jest.fn().mockImplementation(() => {
let currencyActive = false;
return {
advertisedPairs,
activatePair: mockActivatePair,
activateCurrency: (currency: string) => {
mockActivateCurrency(currency);
currencyActive = true;
},
disabledCurrencies: new Map(),
isPairActive: () => false,
isCurrencyActive: () => currencyActive,
getTokenIdentifier: mockPeerGetTokenIdentifer,
getIdentifier: mockGetIdentifier,
};
Expand All @@ -73,7 +80,14 @@ jest.mock('../../lib/p2p/Pool', () => {
});
});
jest.mock('../../lib/swaps/Swaps');
jest.mock('../../lib/swaps/SwapClientManager');
jest.mock('../../lib/swaps/SwapClientManager', () => {
return jest.fn().mockImplementation(() => {
return {
// temporary mock while raiden direct channel checks are required
hasRouteToPeer: jest.fn().mockReturnValue(true),
};
});
});
jest.mock('../../lib/Logger');
jest.mock('../../lib/nodekey/NodeKey');
const mockedNodeKey = <jest.Mock<NodeKey>><any>NodeKey;
Expand Down Expand Up @@ -149,9 +163,13 @@ describe('OrderBook', () => {
test('nosanityswaps enabled adds pairs and requests orders', async () => {
orderbook['nosanityswaps'] = true;
await orderbook['verifyPeerPairs'](peer);
expect(mockActivateCurrency).toHaveBeenCalledTimes(3);
expect(mockActivateCurrency).toHaveBeenCalledWith('BTC');
expect(mockActivateCurrency).toHaveBeenCalledWith('LTC');
expect(mockActivateCurrency).toHaveBeenCalledWith('WETH');
expect(mockActivatePair).toHaveBeenCalledTimes(2);
expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[0], expect.any(Number), expect.anything());
expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[1], expect.any(Number), expect.anything());
expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[0]);
expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[1]);
});

test('isPeerCurrencySupported returns true for a known currency with matching identifiers', async () => {
Expand Down

0 comments on commit e050cbd

Please sign in to comment.