Skip to content

Commit

Permalink
Merge pull request #1069 from ExchangeUnion/fix/swap-amount-units
Browse files Browse the repository at this point in the history
fix: swap sent/received amounts in satoshis
  • Loading branch information
sangaman authored Jul 8, 2019
2 parents affc0bf + 2aa4ede commit 495dd0c
Show file tree
Hide file tree
Showing 16 changed files with 126 additions and 75 deletions.
4 changes: 2 additions & 2 deletions docs/api.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/constants/enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ export enum SwapPhase {
* could still fail due to no route with sufficient capacity, lack of cooperation from the
* receiver or any intermediary node along the route, or an unexpected error from swap client.
*/
SendingAmount = 3,
SendingPayment = 3,
/** We have received the agreed amount of the swap, and the preimage is now known to both sides. */
AmountReceived = 4,
PaymentReceived = 4,
/** The swap has been formally completed and both sides have confirmed they've received payment. */
SwapCompleted = 5,
}
Expand Down
3 changes: 2 additions & 1 deletion lib/db/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export type CurrencyAttributes = CurrencyFactory & {
export type CurrencyInstance = CurrencyAttributes & Sequelize.Instance<CurrencyAttributes>;

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

export type SwapDealAttributes = SwapDealFactory & {
/** The internal db node id of the counterparty peer for this swap deal. */
Expand Down
4 changes: 2 additions & 2 deletions lib/lndclient/LndClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,11 @@ class LndClient extends SwapClient {
if (deal.role === SwapRole.Taker) {
// we are the taker paying the maker
request.setFinalCltvDelta(deal.makerCltvDelta!);
request.setAmt(deal.makerAmount);
request.setAmt(deal.makerUnits);
} else {
// we are the maker paying the taker
request.setFinalCltvDelta(deal.takerCltvDelta);
request.setAmt(deal.takerAmount);
request.setAmt(deal.takerUnits);
}

try {
Expand Down
8 changes: 4 additions & 4 deletions lib/orderbook/OrderBook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class OrderBook extends EventEmitter {
}
});
this.swaps.on('swap.failed', (deal) => {
if (deal.role === SwapRole.Maker && (deal.phase === SwapPhase.SwapAgreed || deal.phase === SwapPhase.SendingAmount)) {
if (deal.role === SwapRole.Maker && (deal.phase === SwapPhase.SwapAgreed || deal.phase === SwapPhase.SendingPayment)) {
// if our order is the maker and the swap failed after it was agreed to but before it was executed
// we must release the hold on the order that we set when we agreed to the deal
this.removeOrderHold(deal.orderId, deal.pairId, deal.quantity!);
Expand Down Expand Up @@ -376,13 +376,13 @@ class OrderBook extends EventEmitter {

if (!this.nobalancechecks) {
// check if sufficient outbound channel capacity exists
const { outboundCurrency, outboundAmount } = Swaps.calculateInboundOutboundAmounts(order.quantity, order.price, order.isBuy, order.pairId);
const { outboundCurrency, outboundUnits } = Swaps.calculateInboundOutboundAmounts(order.quantity, order.price, order.isBuy, order.pairId);
const swapClient = this.swaps.swapClientManager.get(outboundCurrency);
if (!swapClient) {
throw swapsErrors.SWAP_CLIENT_NOT_FOUND(outboundCurrency);
}
if (outboundAmount > swapClient.maximumOutboundCapacity) {
throw errors.INSUFFICIENT_OUTBOUND_BALANCE(outboundCurrency, outboundAmount, swapClient.maximumOutboundCapacity);
if (outboundUnits > swapClient.maximumOutboundCapacity) {
throw errors.INSUFFICIENT_OUTBOUND_BALANCE(outboundCurrency, outboundUnits, swapClient.maximumOutboundCapacity);
}
}

Expand Down
8 changes: 4 additions & 4 deletions lib/proto/xudrpc.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions lib/proto/xudrpc_pb.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/raidenclient/RaidenClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ class RaidenClient extends SwapClient {
let tokenAddress;
if (deal.role === SwapRole.Maker) {
// we are the maker paying the taker
amount = deal.takerAmount;
amount = deal.takerUnits;
tokenAddress = this.tokenAddresses.get(deal.takerCurrency);
} else {
// we are the taker paying the maker
amount = deal.makerAmount;
amount = deal.makerUnits;
tokenAddress = this.tokenAddresses.get(deal.makerCurrency);
}
if (!tokenAddress) {
Expand Down
61 changes: 35 additions & 26 deletions lib/swaps/Swaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,15 @@ class Swaps extends EventEmitter {
* @returns An object with the calculated maker and taker values.
*/
private static calculateMakerTakerAmounts = (quantity: number, price: number, isBuy: boolean, pairId: string) => {
const { inboundCurrency, inboundAmount, outboundCurrency, outboundAmount } =
const { inboundCurrency, inboundAmount, inboundUnits, outboundCurrency, outboundAmount, outboundUnits } =
Swaps.calculateInboundOutboundAmounts(quantity, price, isBuy, pairId);
return {
makerCurrency: inboundCurrency,
makerAmount: inboundAmount,
makerUnits: inboundUnits,
takerCurrency: outboundCurrency,
takerAmount: outboundAmount,
takerUnits: outboundUnits,
};
}

Expand All @@ -102,16 +104,20 @@ class Swaps extends EventEmitter {
*/
public static calculateInboundOutboundAmounts = (quantity: number, price: number, isBuy: boolean, pairId: string) => {
const [baseCurrency, quoteCurrency] = pairId.split('/');
const baseCurrencyAmount = Math.round(quantity * Swaps.UNITS_PER_CURRENCY[baseCurrency]);
const baseCurrencyAmount = quantity;
const quoteCurrencyAmount = price > 0 && price < Number.POSITIVE_INFINITY ?
Math.round(quantity * price * Swaps.UNITS_PER_CURRENCY[quoteCurrency]) :
Math.round(quantity * price) :
0; // if price is zero or infinity, this is a market order and we can't know the quote currency amount
const baseCurrencyUnits = Math.floor(baseCurrencyAmount * Swaps.UNITS_PER_CURRENCY[baseCurrency]);
const quoteCurrencyUnits = Math.floor(quoteCurrencyAmount * Swaps.UNITS_PER_CURRENCY[quoteCurrency]);

const inboundCurrency = isBuy ? baseCurrency : quoteCurrency;
const inboundAmount = isBuy ? baseCurrencyAmount : quoteCurrencyAmount;
const inboundUnits = isBuy ? baseCurrencyUnits : quoteCurrencyUnits;
const outboundCurrency = isBuy ? quoteCurrency : baseCurrency;
const outboundAmount = isBuy ? quoteCurrencyAmount : baseCurrencyAmount;
return { inboundCurrency, inboundAmount, outboundCurrency, outboundAmount };
const outboundUnits = isBuy ? quoteCurrencyUnits : baseCurrencyUnits;
return { inboundCurrency, inboundAmount, inboundUnits, outboundCurrency, outboundAmount, outboundUnits };
}

public init = async () => {
Expand Down Expand Up @@ -242,7 +248,7 @@ class Swaps extends EventEmitter {
throw SwapFailureReason.SwapClientNotSetup;
}

const { makerCurrency, makerAmount } = Swaps.calculateMakerTakerAmounts(taker.quantity, maker.price, maker.isBuy, maker.pairId);
const { makerCurrency, makerUnits } = Swaps.calculateMakerTakerAmounts(taker.quantity, maker.price, maker.isBuy, maker.pairId);

const swapClient = this.swapClientManager.get(makerCurrency)!;

Expand All @@ -254,7 +260,7 @@ class Swaps extends EventEmitter {

let routes;
try {
routes = await swapClient.getRoutes(makerAmount, destination);
routes = await swapClient.getRoutes(makerUnits, destination);
} catch (err) {
throw SwapFailureReason.UnexpectedClientError;
}
Expand Down Expand Up @@ -359,7 +365,7 @@ class Swaps extends EventEmitter {
const peer = this.pool.getPeer(maker.peerPubKey);

const quantity = Math.min(maker.quantity, taker.quantity);
const { makerCurrency, makerAmount, takerCurrency, takerAmount } =
const { makerCurrency, makerAmount, makerUnits, takerCurrency, takerAmount, takerUnits } =
Swaps.calculateMakerTakerAmounts(quantity, maker.price, maker.isBuy, maker.pairId);
const clientType = this.swapClientManager.get(makerCurrency)!.type;
const destination = peer.getIdentifier(clientType, makerCurrency)!;
Expand All @@ -382,6 +388,8 @@ class Swaps extends EventEmitter {
makerCurrency,
takerAmount,
makerAmount,
takerUnits,
makerUnits,
destination,
peerPubKey: peer.nodePubKey!,
localId: taker.localId,
Expand Down Expand Up @@ -433,7 +441,8 @@ class Swaps extends EventEmitter {

const { quantity, price, isBuy } = orderToAccept;

const { makerCurrency, makerAmount, takerCurrency, takerAmount } = Swaps.calculateMakerTakerAmounts(quantity, price, isBuy, requestBody.pairId);
const { makerCurrency, makerAmount, makerUnits, takerCurrency, takerAmount, takerUnits } =
Swaps.calculateMakerTakerAmounts(quantity, price, isBuy, requestBody.pairId);

const takerSwapClient = this.swapClientManager.get(takerCurrency);
if (!takerSwapClient) {
Expand All @@ -459,6 +468,8 @@ class Swaps extends EventEmitter {
takerAmount,
makerCurrency,
takerCurrency,
makerUnits,
takerUnits,
destination: takerPubKey,
peerPubKey: peer.nodePubKey!,
localId: orderToAccept.localId,
Expand Down Expand Up @@ -487,7 +498,7 @@ class Swaps extends EventEmitter {
}

try {
deal.makerToTakerRoutes = await takerSwapClient.getRoutes(takerAmount, takerPubKey, deal.takerCltvDelta);
deal.makerToTakerRoutes = await takerSwapClient.getRoutes(takerUnits, takerPubKey, deal.takerCltvDelta);
} catch (err) {
this.failDeal(deal, SwapFailureReason.UnexpectedClientError, err.message);
await this.sendErrorToPeer({
Expand Down Expand Up @@ -562,7 +573,7 @@ class Swaps extends EventEmitter {

const makerSwapClient = this.swapClientManager.get(makerCurrency)!;
try {
await makerSwapClient.addInvoice(deal.rHash, deal.makerAmount, deal.makerCltvDelta);
await makerSwapClient.addInvoice(deal.rHash, deal.makerUnits, deal.makerCltvDelta);
} catch (err) {
this.failDeal(deal, SwapFailureReason.UnexpectedClientError, `could not add invoice for while accepting deal: ${err.message}`);
await this.sendErrorToPeer({
Expand Down Expand Up @@ -639,7 +650,7 @@ class Swaps extends EventEmitter {
}

try {
await takerSwapClient.addInvoice(deal.rHash, deal.takerAmount, takerSwapClient.cltvDelta);
await takerSwapClient.addInvoice(deal.rHash, deal.takerUnits, takerSwapClient.cltvDelta);
} catch (err) {
this.failDeal(deal, SwapFailureReason.UnexpectedClientError, err.message);
await this.sendErrorToPeer({
Expand All @@ -652,7 +663,7 @@ class Swaps extends EventEmitter {
}

try {
this.setDealPhase(deal, SwapPhase.SendingAmount);
this.setDealPhase(deal, SwapPhase.SendingPayment);
await makerSwapClient.sendPayment(deal);
// TODO: check preimage from payment response vs deal.preImage

Expand Down Expand Up @@ -686,12 +697,12 @@ class Swaps extends EventEmitter {

switch (deal.role) {
case SwapRole.Maker:
expectedAmount = deal.makerAmount;
expectedAmount = deal.makerUnits;
source = 'Taker';
destination = 'Maker';
break;
case SwapRole.Taker:
expectedAmount = deal.takerAmount;
expectedAmount = deal.takerUnits;
source = 'Maker';
destination = 'Taker';
break;
Expand All @@ -701,8 +712,6 @@ class Swaps extends EventEmitter {
return false;
}

// TODO: convert amount to satoshis 1E-8

if (amount < expectedAmount) {
this.logger.error(`received ${amount}, expected ${expectedAmount}`);
this.failDeal(deal, SwapFailureReason.InvalidResolveRequest, `Amount sent from ${source} to ${destination} is too small`);
Expand Down Expand Up @@ -779,9 +788,9 @@ class Swaps extends EventEmitter {
const swapClient = this.swapClientManager.get(deal.takerCurrency)!;

try {
this.setDealPhase(deal, SwapPhase.SendingAmount);
this.setDealPhase(deal, SwapPhase.SendingPayment);
deal.rPreimage = await swapClient.sendPayment(deal);
this.setDealPhase(deal, SwapPhase.AmountReceived);
this.setDealPhase(deal, SwapPhase.PaymentReceived);
return deal.rPreimage;
} catch (err) {
this.failDeal(deal, SwapFailureReason.SendPaymentFailure, err.message);
Expand All @@ -793,7 +802,7 @@ class Swaps extends EventEmitter {
assert(htlcCurrency === undefined || htlcCurrency === deal.takerCurrency, 'incoming htlc does not match expected deal currency');
this.logger.debug('Executing taker code to resolve hash');

this.setDealPhase(deal, SwapPhase.AmountReceived);
this.setDealPhase(deal, SwapPhase.PaymentReceived);
return deal.rPreimage!;
}
}
Expand Down Expand Up @@ -900,18 +909,18 @@ class Swaps extends EventEmitter {
assert(deal.phase === SwapPhase.SwapCreated, 'SwapAgreed can be only be set after SwapCreated');
this.logger.debug('Sending swap response to peer ');
break;
case SwapPhase.SendingAmount:
case SwapPhase.SendingPayment:
assert(deal.role === SwapRole.Taker && deal.phase === SwapPhase.SwapRequested ||
deal.role === SwapRole.Maker && deal.phase === SwapPhase.SwapAgreed,
'SendingAmount can only be set after SwapRequested (taker) or SwapAgreed (maker)');
'SendingPayment can only be set after SwapRequested (taker) or SwapAgreed (maker)');
deal.executeTime = Date.now();
break;
case SwapPhase.AmountReceived:
assert(deal.phase === SwapPhase.SendingAmount, 'AmountReceived can be only be set after SendingAmount');
this.logger.debug(`Amount received for deal with payment hash ${deal.rPreimage}`);
case SwapPhase.PaymentReceived:
assert(deal.phase === SwapPhase.SendingPayment, 'PaymentReceived can be only be set after SendingPayment');
this.logger.debug(`Payment received for deal with payment hash ${deal.rPreimage}`);
break;
case SwapPhase.SwapCompleted:
assert(deal.phase === SwapPhase.AmountReceived, 'SwapCompleted can be only be set after AmountReceived');
assert(deal.phase === SwapPhase.PaymentReceived, 'SwapCompleted can be only be set after PaymentReceived');
deal.completeTime = Date.now();
deal.state = SwapState.Completed;
this.logger.debug(`Swap completed. preimage = ${deal.rPreimage}`);
Expand All @@ -922,7 +931,7 @@ class Swaps extends EventEmitter {

deal.phase = newPhase;

if (deal.phase === SwapPhase.AmountReceived) {
if (deal.phase === SwapPhase.PaymentReceived) {
const wasMaker = deal.role === SwapRole.Maker;
const swapSuccess = {
orderId: deal.orderId,
Expand Down
12 changes: 8 additions & 4 deletions lib/swaps/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,20 @@ export type SwapDeal = {
quantity?: number;
/** The trading pair for the swap. The pairId together with the orderId are needed to find the maker order in the order book. */
pairId: string;
/** The number of satoshis (or equivalent) the taker is expecting to receive. */
/** The amount the taker is expecting to receive denominated in satoshis. */
takerAmount: number;
/** The number of the smallest base units of the currency (like satoshis or wei) the maker is expecting to receive. */
takerUnits: number;
/** The currency the taker is expecting to receive. */
takerCurrency: string;
/** Taker's lnd pubkey on the taker currency's network. */
takerPubKey?: string;
/** The CLTV delta from the current height that should be used to set the timelock for the final hop when sending to taker. */
takerCltvDelta: number;
/** The number of satoshis (or equivalent) the maker is expecting to receive. */
/** The amount the maker is expecting to receive denominated in satoshis. */
makerAmount: number;
/** The number of the smallest base units of the currency (like satoshis or wei) the maker is expecting to receive. */
makerUnits: number;
/** The currency the maker is expecting to receive. */
makerCurrency: string;
/** The CLTV delta from the current height that should be used to set the timelock for the final hop when sending to maker. */
Expand All @@ -63,9 +67,9 @@ export type SwapDeal = {

/** The result of a successful swap. */
export type SwapSuccess = Pick<SwapDeal, 'orderId' | 'localId' | 'pairId' | 'rHash' | 'peerPubKey' | 'price' | 'rPreimage' | 'role'> & {
/** The amount of satoshis (or equivalent) received. */
/** The amount received denominated in satoshis. */
amountReceived: number;
/** The amount of satoshis (or equivalent) sent. */
/** The amount sent denominated in satoshis. */
amountSent: number;
/** The ticker symbol of the currency received. */
currencyReceived: string;
Expand Down
Loading

0 comments on commit 495dd0c

Please sign in to comment.