Skip to content

Commit

Permalink
feat(swaps): define behavior for swap errors
Browse files Browse the repository at this point in the history
This commit adds logic to handle swap failures according to the reason
for the failure. Certain failures suggest a persistent, underlying issue
with swaps for that peer and trading pair, and in this case we drop
all orders for that peer and trading pair to avoid subsequent failures.
We also penalize the peer for swaps that fail due to potential
misbehavior or due to a timeout. Timeouts result in delays that degrade
the trading experience moreso than regular swap failures.

Closes #661.
  • Loading branch information
sangaman committed Feb 22, 2019
1 parent 907ee16 commit 1641a32
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 10 deletions.
15 changes: 10 additions & 5 deletions lib/constants/enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export enum SwapPhase {
SendingAmount = 3,
/** We have received the agreed amount of the swap, and the preimage is now known to both sides. */
AmountReceived = 4,
/* The swap has been formally completed and both sides have confirmed they've received payment. */
/** The swap has been formally completed and both sides have confirmed they've received payment. */
SwapCompleted = 5,
}

Expand All @@ -65,10 +65,15 @@ export enum ReputationEvent {
ManualBan = 0,
ManualUnban = 1,
PacketTimeout = 2,
SwapFailure = 3,
SwapSuccess = 4,
WireProtocolErr = 5,
InvalidAuth = 6,
WireProtocolErr = 3,
InvalidAuth = 4,
SwapSuccess = 5,
/** When a swap is accepted and is attempted to be executed but fails. */
SwapFailure = 6,
/** When a swap is accepted and then fails due to exceeding time limits. */
SwapTimeout = 7,
/** When a swap fails due to unexpected or possibly malicious behavior. */
SwapMisbehavior = 8,
}

export enum SwapFailureReason {
Expand Down
3 changes: 2 additions & 1 deletion lib/orderbook/TradingPair.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ class TradingPair {
}

/**
* Remove all orders given a peer pubKey.
* Remove all of a peer's orders.
* @param peerPubKey the node pub key of the peer
*/
public removePeerOrders = (peerPubKey?: string): PeerOrder[] => {
// if incoming peerPubKey is undefined or empty, don't even try to find it in order queues
Expand Down
2 changes: 2 additions & 0 deletions lib/p2p/NodeList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export const reputationEventWeight = {
[ReputationEvent.SwapSuccess]: 1,
[ReputationEvent.WireProtocolErr]: -5,
[ReputationEvent.InvalidAuth]: -20,
[ReputationEvent.SwapTimeout]: -15,
[ReputationEvent.SwapMisbehavior]: -20,
};

// TODO: inform node about getting banned
Expand Down
27 changes: 25 additions & 2 deletions lib/p2p/Peer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Peer extends EventEmitter {
private packetCount = 0;
private network = new Network(NetworkMagic.TestNet); // TODO: inject from constructor to support more networks
private framer: Framer;
private deactivatedPairs = new Set<string>();
/** Interval to check required responses from peer. */
private static readonly STALL_INTERVAL = 5000;
/** Interval for pinging peers. */
Expand Down Expand Up @@ -114,8 +115,15 @@ class Peer extends EventEmitter {
return this.nodeState ? this.nodeState.addresses : undefined;
}

/** Returns a list of active trading pairs supported by this peer. */
public get pairs(): string[] | undefined {
return this.nodeState ? this.nodeState.pairs : undefined;
if (this.nodeState) {
const activePairs = this.nodeState.pairs.filter((pair) => {
return !this.deactivatedPairs.has(pair);
});
return activePairs;
}
return undefined;
}

public get connected(): boolean {
Expand Down Expand Up @@ -339,6 +347,21 @@ class Peer extends EventEmitter {
this.sendPacket(packet);
}

/**
* Manually deactivates a trading pair with this peer.
*/
public deactivatePair = (pairId: string) => {
if (!this.nodeState) {
throw new Error('cannot deactivate a trading pair before handshake is complete');
}
const index = this.nodeState.pairs.indexOf(pairId);
if (index >= 0) {
this.deactivatedPairs.add(pairId);
this.emit('pairDropped', pairId);
}
// TODO: schedule a timer to see whether this pair can be reactivated
}

private sendRaw = (data: Buffer) => {
if (this.socket) {
this.socket.write(data);
Expand Down Expand Up @@ -805,7 +828,7 @@ class Peer extends EventEmitter {
if (prevNodeState) {
prevNodeState.pairs.forEach((pairId) => {
if (!nodeStateUpdate.pairs || !nodeStateUpdate.pairs.includes(pairId)) {
// a trading pair was in the old handshake state but not in the updated one
// a trading pair was in the old node state but not in the updated one
this.emit('pairDropped', pairId);
}
});
Expand Down
38 changes: 36 additions & 2 deletions lib/swaps/Swaps.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SwapPhase, SwapRole, SwapState, SwapFailureReason } from '../constants/enums';
import { SwapPhase, SwapRole, SwapState, SwapFailureReason, ReputationEvent } from '../constants/enums';
import Peer from '../p2p/Peer';
import { Models } from '../db/DB';
import * as packets from '../p2p/packets/types';
Expand Down Expand Up @@ -673,9 +673,43 @@ class Swaps extends EventEmitter {
if (errorMessage) {
deal.errorMessage = deal.errorMessage ? deal.errorMessage + '; ' + errorMessage : errorMessage;
}
this.logger.debug('new deal error message: ' + deal.errorMessage);
this.logger.debug(`new deal error message for ${deal.rHash}: + ${deal.errorMessage}`);
return;
}
if (errorMessage) {
this.logger.debug(`deal ${deal.rHash} failed due to ${SwapFailureReason[failureReason]}`);
} else {
this.logger.debug(`deal ${deal.rHash} failed due to ${SwapFailureReason[failureReason]}: ${errorMessage}`);
}

switch (failureReason) {
case SwapFailureReason.SwapTimedOut:
// additional penalty as timeouts cause costly delays and possibly stuck HTLC outputs
void this.pool.addReputationEvent(deal.peerPubKey, ReputationEvent.SwapTimeout);
/* falls through */
case SwapFailureReason.SendPaymentFailure:
case SwapFailureReason.NoRouteFound:
case SwapFailureReason.SwapClientNotSetup:
// something is wrong with swaps for this trading pair and peer, drop this pair
try {
this.pool.getPeer(deal.peerPubKey).deactivatePair(deal.pairId);
} catch (err) {
this.logger.debug(`could not drop trading pair ${deal.pairId} for peer ${deal.peerPubKey}`);
}
void this.pool.addReputationEvent(deal.peerPubKey, ReputationEvent.SwapFailure);
break;
case SwapFailureReason.InvalidResolveRequest:
case SwapFailureReason.DealTimedOut:
case SwapFailureReason.InvalidSwapPacketReceived:
case SwapFailureReason.PaymentHashReuse:
// peer misbehaving, penalize the peer
void this.pool.addReputationEvent(deal.peerPubKey, ReputationEvent.SwapMisbehavior);
break;
default:
// do nothing, the swap failed for an innocuous reason
break;
}

assert(deal.state === SwapState.Active, 'deal is not Active. Can not change deal state');
deal.state = SwapState.Error;
deal.failureReason = failureReason;
Expand Down

0 comments on commit 1641a32

Please sign in to comment.