From d5ed9b6cf5b4b83d9c5ce4b2b4039680604fab2f Mon Sep 17 00:00:00 2001
From: Elliot Lee <github.public@intelliot.com>
Date: Mon, 23 Sep 2019 12:00:08 -0700
Subject: [PATCH] Improve error message when signing fails

When there are multiple representations of the same value
(for example, trailing zeros) the verification will fail.

This points users to the error.data object for details about
the failure, and adds a diff so that the cause of the
discrepancy can be seen at a glance.
---
 src/transaction/sign.ts | 71 +++++++++++++++++++++++++++++++++++++++--
 test/api-test.js        | 31 ++++++++++++++++++
 2 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/src/transaction/sign.ts b/src/transaction/sign.ts
index cd80a27a..ae90fb8d 100644
--- a/src/transaction/sign.ts
+++ b/src/transaction/sign.ts
@@ -60,6 +60,72 @@ function signWithKeypair(
   }
 }
 
+/**
+ * Compares two objects and creates a diff.
+ *
+ * @param a An object to compare.
+ * @param b The other object to compare with.
+ *
+ * @returns An object containing the differences between the two objects.
+ */
+function objectDiff(a: object, b: object): object {
+  const diffs = {}
+
+  // Compare two items and push non-matches to object
+  const compare = function (i1: any, i2: any, k: string): void {
+    const type1 = Object.prototype.toString.call(i1)
+    const type2 = Object.prototype.toString.call(i2)
+    if (type2 === '[object Undefined]') {
+      diffs[k] = null // Indicate that the item has been removed
+      return
+    }
+    if (type1 !== type2) {
+      diffs[k] = i2 // Indicate that the item has changed types
+      return
+    }
+    if (type1 === '[object Object]') {
+      const objDiff = objectDiff(i1, i2)
+      if (Object.keys(objDiff).length > 0) {
+        diffs[k] = objDiff
+      }
+      return
+    }
+    if (type1 === '[object Array]') {
+      if (!isEqual(i1, i2)) {
+        diffs[k] = i2 // If arrays do not match, add second item to diffs
+      }
+      return
+    }
+    if (type1 === '[object Function]') {
+      if (i1.toString() !== i2.toString()) {
+        diffs[k] = i2 // If functions differ, add second one to diffs
+      }
+      return
+    }
+    if (i1 !== i2) {
+      diffs[k] = i2
+    }
+  }
+
+  // Check items in first object
+  for (const key in a) {
+    if (a.hasOwnProperty(key)) {
+      compare(a[key], b[key], key)
+    }
+  }
+
+  // Get items that are in the second object but not the first
+  for (const key in b) {
+    if (b.hasOwnProperty(key)) {
+      if (!a[key] && a[key] !== b[key]) {
+        diffs[key] = b[key]
+      }
+    }
+  }
+
+  return diffs
+}
+
 /**
  *  Decode a serialized transaction, remove the fields that are added during the signing process,
  *  and verify that it matches the transaction prior to signing.
@@ -93,11 +159,12 @@ function checkTxSerialization(serialized: string, tx: utils.TransactionJSON): vo
 
   if (!isEqual(decoded, tx)) {
     const error = new utils.common.errors.ValidationError(
-      'Serialized transaction does not match original txJSON'
+      'Serialized transaction does not match original txJSON. See `error.data`'
     )
     error.data = {
       decoded,
-      tx
+      tx,
+      diff: objectDiff(tx, decoded)
     }
     throw error
   }
diff --git a/test/api-test.js b/test/api-test.js
index 4d070c8e..b20f7a6b 100644
--- a/test/api-test.js
+++ b/test/api-test.js
@@ -2679,6 +2679,37 @@ describe('RippleAPI', function () {
     );
   });
 
+  it('sign - throws when encoded tx does not match decoded tx - prepared order', async function () {
+    const order = {
+      direction: 'sell',
+      quantity: {
+        currency: 'USD',
+        counterparty: 'rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B',
+        value: '3.140000'
+      },
+      totalPrice: {
+        currency: 'XRP',
+        value: '31415'
+      }
+    };
+    const prepared = await this.api.prepareOrder('r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59', order, {
+      sequence: 123
+    });
+    const secret = 'shsWGZcmZz6YsWWmcnpfr6fLTdtFV';
+    try {
+      this.api.sign(prepared.txJSON, secret);
+      return Promise.reject(new Error('api.sign should have thrown'));
+    } catch (error) {
+      assert.equal(error.name, 'ValidationError');
+      assert.equal(error.message, 'Serialized transaction does not match original txJSON. See `error.data`');
+      assert.deepEqual(error.data.diff, {
+        TakerGets: {
+          value: '3.14'
+        }
+      });
+    }
+  });
+
   it('sign - throws when encoded tx does not match decoded tx - AccountSet', function () {
     const secret = 'shsWGZcmZz6YsWWmcnpfr6fLTdtFV';
     const request = {