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

[FIX] Correctly determine when to set SendMax (RLJS-272) #345

Merged
merged 1 commit into from
Apr 13, 2015
+136 −43
Merged

[FIX] Correctly determine when to set SendMax (RLJS-272) #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 47 additions & 25 deletions api/lib/rest-to-tx-converter.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
});
});


});