Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Commit

Permalink
Merge pull request #345 from lumberj/hotfix/sendmax
Browse files Browse the repository at this point in the history 
[FIX] Correctly determine when to set SendMax (RLJS-272)
  • Loading branch information
@geertweening
geertweening committed Apr 13, 2015
2 parents c48335f + 0057873 commit 9a8332c
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 43 deletions.
72 changes: 47 additions & 25 deletions api/lib/rest-to-tx-converter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
'use strict';
var _ = require('lodash');
var ripple = require('ripple-lib');
var bignum = require('bignumber.js');
var BigNumber = require('bignumber.js');
var utils = require('./utils');

function RestToTxConverter() {}
Expand All @@ -19,12 +19,38 @@ function RestToTxConverter() {}
* @param {ripple-lib Transaction} transaction
*/
RestToTxConverter.prototype.convert = function(payment, callback) {
function isSendMaxRequired() {
var src = payment.source_account;

var srcAmt = payment.source_amount;
var dstAmt = payment.destination_amount;

// Don't set SendMax for XRP->XRP payment
if (!srcAmt || srcAmt.currency === 'XRP' && dstAmt.currency === 'XRP') {
return false;
}

// only set send max when:
// - source and destination currencies are same
// and source issuer is not source account
// - source amount and destination issuers are different
//
if (srcAmt.currency === dstAmt.currency) {
if (srcAmt.issuer !== src) {
return true;
}
} else if (srcAmt.issuer !== dstAmt.issuer) {
return true;
}
return false;
}

try {
// Convert blank issuer to sender's address
// (Ripple convention for 'any issuer')
// https://ripple.com/build/transactions/
// #special-issuer-values-for-sendmax-and-amount
// https://ripple.com/build/ripple-rest/#counterparties-in-payments
// Convert blank issuer to sender's address
// (Ripple convention for 'any issuer')
// https://ripple.com/build/transactions/
// #special-issuer-values-for-sendmax-and-amount
// https://ripple.com/build/ripple-rest/#counterparties-in-payments
if (payment.source_amount && payment.source_amount.currency !== 'XRP'
&& payment.source_amount.issuer === '') {
payment.source_amount.issuer = payment.source_account;
Expand All @@ -43,11 +69,11 @@ RestToTxConverter.prototype.convert = function(payment, callback) {
// Uppercase currency codes
if (payment.source_amount) {
payment.source_amount.currency =
payment.source_amount.currency.toUpperCase();
payment.source_amount.currency.toUpperCase();
}
if (payment.destination_amount) {
payment.destination_amount.currency =
payment.destination_amount.currency.toUpperCase();
payment.destination_amount.currency.toUpperCase();
}
/* Construct payment */
var transaction = new ripple.Transaction();
Expand All @@ -70,24 +96,20 @@ RestToTxConverter.prototype.convert = function(payment, callback) {
if (payment.destination_tag) {
transaction.destinationTag(parseInt(payment.destination_tag, 10));
}

// SendMax
if (payment.source_amount) {
// Only set send max if source and destination currencies are different
if (!(payment.source_amount.currency
=== payment.destination_amount.currency
&& payment.source_amount.issuer
=== payment.destination_amount.issuer)) {
var max_value = bignum(payment.source_amount.value)
.plus(payment.source_slippage || 0).toString();
if (payment.source_amount.currency === 'XRP') {
transaction.sendMax(utils.xrpToDrops(max_value));
} else {
transaction.sendMax({
value: max_value,
currency: payment.source_amount.currency,
issuer: payment.source_amount.issuer
});
}
if (isSendMaxRequired()) {
var max_value = new BigNumber(payment.source_amount.value)
.plus(payment.source_slippage || 0).toString();

if (payment.source_amount.currency === 'XRP') {
transaction.sendMax(utils.xrpToDrops(max_value));
} else {
transaction.sendMax({
value: max_value,
currency: payment.source_amount.currency,
issuer: payment.source_amount.issuer
});
}
}

Expand Down
8 changes: 6 additions & 2 deletions test/fixtures/addresses.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
'use strict';

module.exports = {
VALID: 'r3GgMwvgvP8h4yVWvjH1dPZNvC37TjzBBE',
ISSUER: 'r3PDtZSa5LiYp1Ysn1vMuMzB59RzV3W9QH',
SECRET: 'shsWGZcmZz6YsWWmcnpfr6fLTdtFV',
INVALID: 'rxxLy6UWsjzxsQrTATf1bwDYSaJMoTGvfY2Q',
COUNTERPARTY: 'rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B'
};
COUNTERPARTY: 'rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B',
VALID2: 'rHrGzm86u7fwsickgTghsS9D7sUuC3VKwE',
ISSUER2: 'rBXMZX16zyqthrECN4AY4wxbug7335tCKh'
};
35 changes: 26 additions & 9 deletions test/unit/fixtures/rest-converter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@ module.exports.paymentRestXRP = {
}
};

module.exports.paymentRestXRPtoXRP = {
'source_account': addresses.VALID,
'source_amount': {
'value': '1',
'currency': 'XRP',
'issuer': ''
},
'destination_account': addresses.COUNTERPARTY,
'destination_amount': {
'value': '1',
'currency': 'XRP',
'issuer': ''
}
};

module.exports.paymentRestComplex = {
'source_account': addresses.VALID,
'source_tag': '',
Expand Down Expand Up @@ -118,26 +133,28 @@ module.exports.exportsPaymentRestIssuers = function(options) {
_.defaults(options, {
sourceAccount: addresses.VALID,
destinationAccount: addresses.COUNTERPARTY,
sourceIssuer: addresses.VALID,
destinationIssuer: addresses.COUNTERPARTY,
sourceValue: '10'
sourceIssuer: '',
destinationIssuer: '',
sourceValue: '10',
destinationCurrency: 'USD',
sourceCurrency: 'USD'
});

return {
source_account: options.sourceAccount,
source_tag: '',
source_amount: {
value: options.sourceValue,
currency: 'USD',
currency: options.sourceCurrency,
issuer: options.sourceIssuer
},
source_slippage: options.sourceSlippage,
destination_account: options.destinationAccount,
destination_tag: '',
destination_amount: {
destination_account: options.destinationAccount,
destination_tag: '',
destination_amount: {
value: '10',
currency: 'USD',
issuer: options.destinationIssuer
currency: options.destinationCurrency,
issuer: options.destinationIssuer
},
invoice_id: '',
paths: '[]',
Expand Down
64 changes: 57 additions & 7 deletions test/unit/rest-converter-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,24 @@ suite('unit - converter - Rest to Tx', function() {
});
});

test('convert() -- payment with XRP', function(done) {
test('convert() -- payment with XRP and no source amount', function(done) {
restToTxConverter.convert(fixtures.paymentRestXRP, function(err, transaction) {
assert.strictEqual(err, null);
assert.deepEqual(transaction.summary(), fixtures.paymentTxXRP);
assert.strictEqual(transaction.tx_json.SendMax, undefined);
done();
});
});

test('convert() -- payment XRP to XRP', function(done) {
restToTxConverter.convert(fixtures.paymentRestXRPtoXRP, function(err, transaction) {
assert.strictEqual(err, null);
assert.strictEqual(transaction.tx_json.SendMax, undefined);
done();
});
});


test('convert() -- payment with additional flags', function(done) {
restToTxConverter.convert(fixtures.paymentRestComplex, function(err, transaction) {
assert.strictEqual(err, null);
Expand All @@ -46,34 +56,74 @@ suite('unit - converter - Rest to Tx', function() {

test('convert() -- payment with currency that has different issuers for source and destination amount', function(done) {
restToTxConverter.convert(fixtures.exportsPaymentRestIssuers({
sourceIssuer: addresses.VALID,
destinationIssuer: addresses.COUNTERPARTY
sourceAccount: addresses.VALID,
destinationAccount: addresses.COUNTERPARTY,
sourceIssuer: addresses.VALID2,
destinationIssuer: addresses.ISSUER2
}), function(err, transaction) {
assert.strictEqual(err, null);
assert.deepEqual(transaction.tx_json.SendMax, {
value: '10',
currency: 'USD',
issuer: addresses.VALID
issuer: addresses.VALID2
});
done();
});
});

test('convert() -- payment with currency that has different issuers for source and destination amount and a source_slippage of 0.1', function(done) {
restToTxConverter.convert(fixtures.exportsPaymentRestIssuers({
sourceIssuer: addresses.VALID,
destinationIssuer: addresses.COUNTERPARTY,
sourceAccount: addresses.VALID,
destinationAccount: addresses.COUNTERPARTY,
sourceIssuer: addresses.VALID2,
destinationIssuer: addresses.ISSUER2,
sourceSlippage: '0.1',
sourceAmount: '10'
}), function(err, transaction) {
assert.strictEqual(err, null);
assert.deepEqual(transaction.tx_json.SendMax, {
value: '10.1',
currency: 'USD',
issuer: addresses.VALID
issuer: addresses.VALID2
});
done();
});
});

test('convert() -- payment with same currency for source and destination, no issuer for source amount', function(done) {
restToTxConverter.convert(fixtures.exportsPaymentRestIssuers({
sourceIssuer: '',
destinationAccount: addresses.COUNTERPARTY,
destinationIssuer: addresses.COUNTERPARTY
}), function(err, transaction) {
assert.strictEqual(err, null);
assert.strictEqual(transaction.tx_json.SendMax, undefined);
done();
});
});

test('convert() -- payment with same currency for source and destination, no issuer for source and destination amount', function(done) {
restToTxConverter.convert(fixtures.exportsPaymentRestIssuers({
sourceIssuer: '',
destinationAccount: addresses.COUNTERPARTY,
destinationIssuer: ''
}), function(err, transaction) {
assert.strictEqual(err, null);
assert.strictEqual(transaction.tx_json.SendMax, undefined);
done();
});
});

test('convert() -- payment with same currency for source and destination, no issuer for destination amount', function(done) {
restToTxConverter.convert(fixtures.exportsPaymentRestIssuers({
sourceIssuer: addresses.VALID,
destinationAccount: addresses.COUNTERPARTY,
destinationIssuer: ''
}), function(err, transaction) {
assert.strictEqual(transaction.tx_json.SendMax, undefined);
done(err);
});
});


});

0 comments on commit 9a8332c

Please sign in to comment.