Merge pull request #357 from lumberj/task/sendmax-update

Update SendMax logic to match rippled 0.28 (RLJS-336)
ripple · Apr 20, 2015 · 69363d6 · 69363d6
2 parents 0863b5a + 36d8477
commit 69363d6
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 27 deletions.
diff --git a/api/lib/rest-to-tx-converter.js b/api/lib/rest-to-tx-converter.js
@@ -19,28 +19,15 @@ function RestToTxConverter() {}
  *  @param {ripple-lib Transaction} transaction
  */
 RestToTxConverter.prototype.convert = function(payment, callback) {
-  function isSendMaxRequired() {
-    var src = payment.source_account;
-
+  function isSendMaxAllowed() {
     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 && srcAmt.issuer !== dstAmt.issuer) {
-        return true;
-      }
-    }
-    return false;
+    // temREDUNDANT_SEND_MAX removed in:
+    // https://github.com/ripple/rippled/commit/
+    //  c522ffa6db2648f1d8a987843e7feabf1a0b7de8/
+    return srcAmt && !(srcAmt.currency === 'XRP' && dstAmt.currency === 'XRP');
   }
 
   try {
@@ -96,7 +83,7 @@ RestToTxConverter.prototype.convert = function(payment, callback) {
     }
 
     // SendMax
-    if (isSendMaxRequired()) {
+    if (isSendMaxAllowed()) {
       var max_value = new BigNumber(payment.source_amount.value)
         .plus(payment.source_slippage || 0).toString();
 

diff --git a/test/unit/fixtures/rest-converter.js b/test/unit/fixtures/rest-converter.js
@@ -114,6 +114,11 @@ module.exports.paymentTxComplex = {
       currency: 'USD',
       issuer: addresses.VALID
     },
+    SendMax: {
+      value: '10',
+      currency: 'USD',
+      issuer: addresses.VALID
+    },
     Destination: addresses.COUNTERPARTY
   },
   clientID: undefined,

diff --git a/test/unit/rest-converter-test.js b/test/unit/rest-converter-test.js
@@ -34,6 +34,22 @@ suite('unit - converter - Rest to Tx', function() {
     });
   });
 
+  test('convert() -- payment XRP to non-XRP', function(done) {
+    restToTxConverter.convert(fixtures.exportsPaymentRestIssuers({
+      sourceAccount: addresses.VALID,
+      sourceAmount: '10',
+      sourceCurrency: 'XRP',
+      destinationAccount: addresses.COUNTERPARTY,
+      sourceIssuer: '',
+      destinationIssuer: addresses.ISSUER
+    }), function(err, transaction) {
+      if (err) {
+        return done(err);
+      }
+      assert.strictEqual(transaction.tx_json.SendMax, '10000000');
+      done();
+    });
+  });
 
   test('convert() -- payment with additional flags', function(done) {
     restToTxConverter.convert(fixtures.paymentRestComplex, function(err, transaction) {
@@ -50,8 +66,14 @@ suite('unit - converter - Rest to Tx', function() {
       sourceIssuer: addresses.ISSUER,
       destinationIssuer: addresses.ISSUER
     }), function(err, transaction) {
-      assert.strictEqual(err, null);
-      assert.strictEqual(transaction.tx_json.SendMax, undefined);
+      if (err) {
+        return done(err);
+      }
+      assert.deepEqual(transaction.tx_json.SendMax, {
+        value: '10',
+        currency: 'USD',
+        issuer: addresses.ISSUER
+      });
       done();
     });
   });
@@ -99,8 +121,14 @@ suite('unit - converter - Rest to Tx', function() {
       sourceIssuer: '',
       destinationIssuer: addresses.ISSUER2
     }), function(err, transaction) {
-      assert.strictEqual(err, null);
-      assert.strictEqual(transaction.tx_json.SendMax, undefined);
+      if (err) {
+        return done(err);
+      }
+      assert.deepEqual(transaction.tx_json.SendMax, {
+        value: '10',
+        currency: 'USD',
+        issuer: addresses.VALID
+      });
       done();
     });
   });
@@ -112,8 +140,14 @@ suite('unit - converter - Rest to Tx', function() {
       destinationAccount: addresses.COUNTERPARTY,
       destinationIssuer: ''
     }), function(err, transaction) {
-      assert.strictEqual(err, null);
-      assert.strictEqual(transaction.tx_json.SendMax, undefined);
+      if (err) {
+        return done(err);
+      }
+      assert.deepEqual(transaction.tx_json.SendMax, {
+        value: '10',
+        currency: 'USD',
+        issuer: addresses.VALID
+      });
       done();
     });
   });
@@ -128,7 +162,11 @@ suite('unit - converter - Rest to Tx', function() {
       if (err) {
         return done(err);
       }
-      assert.strictEqual(transaction.tx_json.SendMax, undefined);
+      assert.deepEqual(transaction.tx_json.SendMax, {
+        value: '10',
+        currency: 'USD',
+        issuer: addresses.VALID
+      });
       done();
     });
   });
@@ -143,7 +181,11 @@ suite('unit - converter - Rest to Tx', function() {
       if (err) {
         return done(err);
       }
-      assert.strictEqual(transaction.tx_json.SendMax, undefined);
+      assert.deepEqual(transaction.tx_json.SendMax, {
+        value: '10',
+        currency: 'USD',
+        issuer: addresses.VALID
+      });
       done();
     });
   });