Fix transaction summary for transactions that fail with remoteError

2025-11-20 04:05:52 +00:00 · 2015-05-15 17:25:32 -07:00
parent 4ecbf31898
commit 5e714f6143
4 changed files with 268 additions and 74 deletions
--- a/src/transaction.js
+++ b/src/transaction.js
@@ -1,6 +1,7 @@
 'use strict';

 var util = require('util');
+var lodash = require('lodash');
 var EventEmitter = require('events').EventEmitter;
 var utils = require('./utils');
 var sjcl = require('./utils').sjcl;
@@ -143,7 +144,11 @@ Transaction.set_clear_flags = {
 Transaction.MEMO_TYPES = {
 };

-Transaction.ASCII_REGEX = /^[\x00-\x7F]*$/;
+/* eslint-disable max-len */
+
+// URL characters per RFC 3986
+Transaction.MEMO_REGEX = /^[0-9a-zA-Z-\.\_\~\:\/\?\#\[\]\@\!\$\&\'\(\)\*\+\,\;\=\%]+$/;
+/* eslint-enable max-len */

 Transaction.formats = require('./binformat').tx;

@@ -813,17 +818,17 @@ Transaction.prototype.setFlags = function(flags) {
 /**
 * Add a Memo to transaction.
 *
- * @param {String} memoType
- * - describes what the data represents, needs to be valid ASCII
- * * @param {String} memoFormat
- * - describes what format the data is in, MIME type, needs to be valid ASCII
- * @param {String} memoData
+ * @param [String] memoType
+ * - describes what the data represents, must contain valid URL characters
+ * @param [String] memoFormat
+ * - describes what format the data is in, MIME type, must contain valid URL
+ * - characters
+ * @param [String] memoData
 * - data for the memo, can be any JS object. Any object other than string will
 *   be stringified (JSON) for transport
 */

 Transaction.prototype.addMemo = function(memoType, memoFormat, memoData) {
-
  if (typeof memoType === 'object') {
    var opts = memoType;
    memoType = opts.memoType;
@@ -831,26 +836,19 @@ Transaction.prototype.addMemo = function(memoType, memoFormat, memoData) {
    memoData = opts.memoData;
  }

-  if (!/(undefined|string)/.test(typeof memoType)) {
-    throw new Error('MemoType must be a string');
-  } else if (!Transaction.ASCII_REGEX.test(memoType)) {
-    throw new Error('MemoType must be valid ASCII');
-  }
-
-  if (!/(undefined|string)/.test(typeof memoFormat)) {
-    throw new Error('MemoFormat must be a string');
-  } else if (!Transaction.ASCII_REGEX.test(memoFormat)) {
-    throw new Error('MemoFormat must be valid ASCII');
-  }
-
  function convertStringToHex(string) {
    var utf8String = sjcl.codec.utf8String.toBits(string);
    return sjcl.codec.hex.fromBits(utf8String).toUpperCase();
  }

  var memo = {};
+  var memoRegex = Transaction.MEMO_REGEX;

  if (memoType) {
+    if (!(lodash.isString(memoType) && memoRegex.test(memoType))) {
+      throw new Error(
+        'MemoType must be a string containing only valid URL characters');
+    }
    if (Transaction.MEMO_TYPES[memoType]) {
      // XXX Maybe in the future we want a schema validator for
      // memo types
@@ -860,6 +858,11 @@ Transaction.prototype.addMemo = function(memoType, memoFormat, memoData) {
  }

  if (memoFormat) {
+    if (!(lodash.isString(memoFormat) && memoRegex.test(memoFormat))) {
+      throw new Error(
+        'MemoFormat must be a string containing only valid URL characters');
+    }
+
    memo.MemoFormat = convertStringToHex(memoFormat);
  }

@@ -1211,8 +1214,9 @@ Transaction.prototype.abort = function() {
 * @return {Object} transaction summary
 */

+Transaction.prototype.getSummary =
 Transaction.prototype.summary = function() {
-  var result = {
+  var txSummary = {
    tx_json: this.tx_json,
    clientID: this._clientID,
    submittedIDs: this.submittedIDs,
@@ -1221,21 +1225,24 @@ Transaction.prototype.summary = function() {
    initialSubmitIndex: this.initialSubmitIndex,
    lastLedgerSequence: this.lastLedgerSequence,
    state: this.state,
-    server: this._server ? this._server._opts.url : undefined,
    finalized: this.finalized
  };

  if (this.result) {
-    result.result = {
+    var transaction_hash = this.result.tx_json
+    ? this.result.tx_json.hash
+    : undefined;
+
+    txSummary.result = {
      engine_result: this.result.engine_result,
      engine_result_message: this.result.engine_result_message,
      ledger_hash: this.result.ledger_hash,
      ledger_index: this.result.ledger_index,
-      transaction_hash: this.result.tx_json.hash
+      transaction_hash: transaction_hash
    };
  }

-  return result;
+  return txSummary;
 };

 exports.Transaction = Transaction;
--- a/test/fixtures/transactionmanager.json
+++ b/test/fixtures/transactionmanager.json
@@ -56,24 +56,24 @@
    "meta": {
      "AffectedNodes": [
        {
-          "ModifiedNode": {
-            "FinalFields": {
-              "Account": "rNP2Y5EZrVZdFKsow11NoKTE5FjXuBQd3d",
-              "Balance": "1000",
-              "Flags": 4849664,
-              "OwnerCount": 1,
-              "Sequence": 1
-            },
-            "LedgerEntryType": "AccountRoot",
-            "LedgerIndex": "A4B28FB972EF890DC39A8557DF8960D41DADA00D39B0F1EFCD4BBB85FCA13A30",
-            "PreviousFields": {
-              "Balance": "1000",
-              "Sequence": 3864
-            },
-            "PreviousTxnID": "F4910E55A39C42AB82071212D84119631DDE0B0F4F8F9040F252B0066898DBDF",
-            "PreviousTxnLgrSeq": 11693103
-          }
+        "ModifiedNode": {
+          "FinalFields": {
+            "Account": "rNP2Y5EZrVZdFKsow11NoKTE5FjXuBQd3d",
+            "Balance": "1000",
+            "Flags": 4849664,
+            "OwnerCount": 1,
+            "Sequence": 1
+          },
+          "LedgerEntryType": "AccountRoot",
+          "LedgerIndex": "A4B28FB972EF890DC39A8557DF8960D41DADA00D39B0F1EFCD4BBB85FCA13A30",
+          "PreviousFields": {
+            "Balance": "1000",
+            "Sequence": 3864
+          },
+          "PreviousTxnID": "F4910E55A39C42AB82071212D84119631DDE0B0F4F8F9040F252B0066898DBDF",
+          "PreviousTxnLgrSeq": 11693103
        }
+      }
      ],
      "TransactionIndex": 9,
      "TransactionResult": "tesSUCCESS"
@@ -101,24 +101,24 @@
      "TransactionIndex": 3,
      "AffectedNodes": [
        {
-          "ModifiedNode": {
-            "LedgerEntryType": "AccountRoot",
-            "PreviousTxnLgrSeq": 11693103,
-            "PreviousTxnID": "F4910E55A39C42AB82071212D84119631DDE0B0F4F8F9040F252B0066898DBDF",
-            "LedgerIndex": "A4B28FB972EF890DC39A8557DF8960D41DADA00D39B0F1EFCD4BBB85FCA13A30",
-            "PreviousFields": {
-              "Sequence": 3864,
-              "Balance": "1000"
-            },
-            "FinalFields": {
-              "Flags": 4849664,
-              "Sequence": 3865,
-              "OwnerCount": 1,
-              "Balance": "1000",
-              "Account": "rNP2Y5EZrVZdFKsow11NoKTE5FjXuBQd3d"
-            }
+        "ModifiedNode": {
+          "LedgerEntryType": "AccountRoot",
+          "PreviousTxnLgrSeq": 11693103,
+          "PreviousTxnID": "F4910E55A39C42AB82071212D84119631DDE0B0F4F8F9040F252B0066898DBDF",
+          "LedgerIndex": "A4B28FB972EF890DC39A8557DF8960D41DADA00D39B0F1EFCD4BBB85FCA13A30",
+          "PreviousFields": {
+            "Sequence": 3864,
+            "Balance": "1000"
+          },
+          "FinalFields": {
+            "Flags": 4849664,
+            "Sequence": 3865,
+            "OwnerCount": 1,
+            "Balance": "1000",
+            "Account": "rNP2Y5EZrVZdFKsow11NoKTE5FjXuBQd3d"
          }
        }
+      }
      ],
      "TransactionResult": "tesSUCCESS"
    },
@@ -290,5 +290,17 @@
    },
    "status": "success",
    "type": "response"
+  },
+  "SUBMIT_REMOTE_ERROR": {
+    "error": "invalidTransaction",
+    "error_exception": "fails local checks: The MemoType and MemoFormat fields may only contain characters that are allowed in URLs under RFC 3986.",
+    "id": 1,
+    "request": {
+      "command": "submit",
+      "id": 1,
+      "tx_blob": "12000022800000002400000001201B00CCEAD0614000000000000001684000000000002EE0732102999FB4BC17144F83CDC2F17EA642519FF115EE7B0CC8C78DE9061F1A473F7BAC7447304502210098DC7E9ED1CE860FB6B0904E6E8140D5463D288BA633F36E69A68ACB3D6FCA06022014E76E22F5173B37239F9F56F904839B462F5019169C56A324D3F074FBA39A2A811492DECA2DC92352BE97C1F6347F7E6CCB9A8241C883143108B9AC27BF036EFE5CBE787921F54D622B7A5BF9EA7C0B6D79206D656D6F747970657E0C6D79206D656D6F5F64617461E1F1"
+    },
+    "status": "error",
+    "type": "response"
  }
 }
--- a/test/transaction-manager-test.js
+++ b/test/transaction-manager-test.js
@@ -1,3 +1,6 @@
+/* eslint-disable max-len */
+/* eslint-disable comma-spacing */
+
 'use strict';

 var ws = require('ws');
@@ -34,6 +37,8 @@ var SUBMIT_TEF_RESPONSE = require('./fixtures/transactionmanager')
 .SUBMIT_TEF_RESPONSE;
 var SUBMIT_TEL_RESPONSE = require('./fixtures/transactionmanager')
 .SUBMIT_TEL_RESPONSE;
+var SUBMIT_REMOTE_ERROR = require('./fixtures/transactionmanager')
+.SUBMIT_REMOTE_ERROR;

 describe('TransactionManager', function() {
  var rippled;
@@ -491,7 +496,7 @@ describe('TransactionManager', function() {
      assert(false, 'Should not receive proposed event');
    });
    transaction.once('submitted', function(m) {
-      assert.strictEqual(m.engine_result, 'terNO_ACCOUNT');
+      assert.strictEqual(m.engine_result, SUBMIT_TER_RESPONSE.result.engine_result);
      receivedSubmitted = true;
    });

@@ -523,6 +528,22 @@ describe('TransactionManager', function() {
      assert.strictEqual(err.engine_result, 'tejMaxLedger');
      assert(receivedSubmitted);
      assert.strictEqual(transactionManager.getPending().length(), 0);
+      assert.strictEqual(transactionManager.getPending().length(), 0);
+
+      var summary = transaction.summary();
+      assert.strictEqual(summary.submissionAttempts, 1);
+      assert.strictEqual(summary.submitIndex, 2);
+      assert.strictEqual(summary.initialSubmitIndex, 2);
+      assert.strictEqual(summary.lastLedgerSequence, 5);
+      assert.strictEqual(summary.state, 'failed');
+      assert.strictEqual(summary.finalized, true);
+      assert.deepEqual(summary.result, {
+        engine_result: SUBMIT_TER_RESPONSE.result.engine_result,
+        engine_result_message: SUBMIT_TER_RESPONSE.result.engine_result_message,
+        ledger_hash: undefined,
+        ledger_index: undefined,
+        transaction_hash: SUBMIT_TER_RESPONSE.result.tx_json.hash
+      });
      transactionManager.once('sequence_filled', done);
    });
  });
@@ -569,6 +590,21 @@ describe('TransactionManager', function() {
      assert(receivedResubmitted);
      assert.strictEqual(err.engine_result, 'tejMaxLedger');
      assert.strictEqual(transactionManager.getPending().length(), 0);
+
+      var summary = transaction.summary();
+      assert.strictEqual(summary.submissionAttempts, 2);
+      assert.strictEqual(summary.submitIndex, 3);
+      assert.strictEqual(summary.initialSubmitIndex, 2);
+      assert.strictEqual(summary.lastLedgerSequence, 5);
+      assert.strictEqual(summary.state, 'failed');
+      assert.strictEqual(summary.finalized, true);
+      assert.deepEqual(summary.result, {
+        engine_result: SUBMIT_TEF_RESPONSE.result.engine_result,
+        engine_result_message: SUBMIT_TEF_RESPONSE.result.engine_result_message,
+        ledger_hash: undefined,
+        ledger_index: undefined,
+        transaction_hash: SUBMIT_TEF_RESPONSE.result.tx_json.hash
+      });
      done();
    });
  });
@@ -612,6 +648,21 @@ describe('TransactionManager', function() {
      assert(receivedResubmitted);
      assert.strictEqual(err.engine_result, 'tejMaxLedger');
      assert.strictEqual(transactionManager.getPending().length(), 0);
+
+      var summary = transaction.summary();
+      assert.strictEqual(summary.submissionAttempts, 2);
+      assert.strictEqual(summary.submitIndex, 3);
+      assert.strictEqual(summary.initialSubmitIndex, 2);
+      assert.strictEqual(summary.lastLedgerSequence, 5);
+      assert.strictEqual(summary.state, 'failed');
+      assert.strictEqual(summary.finalized, true);
+      assert.deepEqual(summary.result, {
+        engine_result: SUBMIT_TEL_RESPONSE.result.engine_result,
+        engine_result_message: SUBMIT_TEL_RESPONSE.result.engine_result_message,
+        ledger_hash: undefined,
+        ledger_index: undefined,
+        transaction_hash: SUBMIT_TEL_RESPONSE.result.tx_json.hash
+      });
      done();
    });
  });
@@ -630,6 +681,89 @@ describe('TransactionManager', function() {
    transaction.submit(function(err) {
      assert.strictEqual(err.engine_result, 'tejSecretInvalid');
      assert.strictEqual(transactionManager.getPending().length(), 0);
+
+      var summary = transaction.summary();
+      assert.deepEqual(summary.tx_json, transaction.tx_json);
+      assert.strictEqual(summary.submissionAttempts, 0);
+      assert.strictEqual(summary.submitIndex, undefined);
+      assert.strictEqual(summary.initialSubmitIndex, undefined);
+      assert.strictEqual(summary.lastLedgerSequence, undefined);
+      assert.strictEqual(summary.state, 'failed');
+      assert.strictEqual(summary.finalized, true);
+      assert.deepEqual(summary.result, {
+        engine_result: 'tejSecretInvalid',
+        engine_result_message: 'Invalid secret',
+        ledger_hash: undefined,
+        ledger_index: undefined,
+        transaction_hash: undefined
+      });
+      done();
+    });
+  });
+
+  it('Submit transaction -- remote error', function(done) {
+    var transaction = remote.createTransaction('Payment', {
+      account: ACCOUNT.address,
+      destination: ACCOUNT2.address,
+      amount: '1'
+    });
+
+    // MemoType must contain only valid URL characters (RFC 3986). This
+    // transaction is invalid
+    //transaction.addMemo('my memotype','my_memo_data');
+    transaction.tx_json.Memos = [{
+      Memo: {
+        MemoType: '6D79206D656D6F74797065',
+        MemoData: '6D795F6D656D6F5F64617461'
+      }
+    }];
+
+    var receivedSubmitted = false;
+    transaction.once('proposed', function() {
+      assert(false, 'Should not receive proposed event');
+    });
+    transaction.once('submitted', function(m) {
+      assert.strictEqual(m.error, 'remoteError');
+      receivedSubmitted = true;
+    });
+
+    rippled.on('request_submit', function(m, req) {
+      assert.strictEqual(transactionManager.getPending().length(), 1);
+      assert.strictEqual(m.tx_blob, SerializedObject.from_json(
+        transaction.tx_json).to_hex());
+
+        /* eslint-disable max-len */
+
+        // rippled returns an exception here rather than an engine result
+        // https://github.com/ripple/rippled/blob/c61d0c663e410c3d3622f20092535710243b55af/src/ripple/rpc/handlers/Submit.cpp#L66-L75
+
+        /* eslint-enable max-len */
+
+        req.sendResponse(SUBMIT_REMOTE_ERROR, {id: m.id});
+    });
+
+    transaction.submit(function(err) {
+      assert(err, 'Transaction submission should not succeed');
+      assert(receivedSubmitted);
+      assert.strictEqual(err.error, 'remoteError');
+      assert.strictEqual(err.remote.error, 'invalidTransaction');
+      assert.strictEqual(transactionManager.getPending().length(), 0);
+
+      var summary = transaction.summary();
+      assert.deepEqual(summary.tx_json, transaction.tx_json);
+      assert.strictEqual(summary.submissionAttempts, 1);
+      assert.strictEqual(summary.submitIndex, 2);
+      assert.strictEqual(summary.initialSubmitIndex, 2);
+      assert.strictEqual(summary.lastLedgerSequence, 5);
+      assert.strictEqual(summary.state, 'failed');
+      assert.strictEqual(summary.finalized, true);
+      assert.deepEqual(summary.result, {
+        engine_result: undefined,
+        engine_result_message: undefined,
+        ledger_hash: undefined,
+        ledger_index: undefined,
+        transaction_hash: undefined
+      });
      done();
    });
  });
--- a/test/transaction-test.js
+++ b/test/transaction-test.js
@@ -1,6 +1,9 @@
 /* eslint-disable max-len */
+
 'use strict';
+
 var assert = require('assert');
+var lodash = require('lodash');
 var Transaction = require('ripple-lib').Transaction;
 var TransactionQueue = require('ripple-lib').TransactionQueue;
 var Remote = require('ripple-lib').Remote;
@@ -37,6 +40,19 @@ var transactionResult = {
  }
 };

+// https://github.com/ripple/rippled/blob/c61d0c663e410c3d3622f20092535710243b55af/src/ripple/protocol/impl/STTx.cpp#L342-L370
+var allowed_memo_chars = ('0123456789-._~:/?#[]@!$&\'()*+,;=%ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz').split('');
+
+// Disallowed ASCII characters
+var disallowed_memo_chars = [];
+
+for (var i = 0; i <= 127; i++) {
+  var char = String.fromCharCode(i);
+  if (!lodash.contains(allowed_memo_chars, char)) {
+    disallowed_memo_chars.push(char);
+  }
+}
+
 describe('Transaction', function() {
  before(function() {
    sjcl.random.addEntropy(
@@ -1155,6 +1171,21 @@ describe('Transaction', function() {
    ];

    assert.deepEqual(transaction.tx_json.Memos, expected);
+
+    allowed_memo_chars.forEach(function(c) {
+      var hexStr = new Buffer(c).toString('hex').toUpperCase();
+      var tx = new Transaction();
+
+      tx.addMemo(c, c, c);
+
+      assert.deepEqual(tx.tx_json.Memos, [{
+        Memo: {
+          MemoType: hexStr,
+          MemoFormat: hexStr,
+          MemoData: hexStr
+        }
+      }]);
+    });
  });

  it('Add Memo - by object', function() {
@@ -1231,36 +1262,46 @@ describe('Transaction', function() {
    var transaction = new Transaction();
    transaction.tx_json.TransactionType = 'Payment';

+    var error_regex = /^Error: MemoType must be a string containing only valid URL characters$/;
+
    assert.throws(function() {
      transaction.addMemo(1);
-    }, /^Error: MemoType must be a string$/);
-  });
-
-  it('Add Memo - invalid ASCII MemoType', function() {
-    var transaction = new Transaction();
-    transaction.tx_json.TransactionType = 'Payment';
-
+    }, error_regex);
    assert.throws(function() {
      transaction.addMemo('한국어');
-    }, /^Error: MemoType must be valid ASCII$/);
+    }, error_regex);
+    assert.throws(function() {
+      transaction.addMemo('my memo');
+    }, error_regex);
+
+    disallowed_memo_chars.forEach(function(c) {
+      assert.throws(function() {
+        transaction.addMemo(c);
+      }, error_regex);
+    });
  });

  it('Add Memo - invalid MemoFormat', function() {
    var transaction = new Transaction();
    transaction.tx_json.TransactionType = 'Payment';

+    var error_regex = /^Error: MemoFormat must be a string containing only valid URL characters$/;
+
    assert.throws(function() {
      transaction.addMemo(undefined, 1);
-    }, /^Error: MemoFormat must be a string$/);
-  });
-
-  it('Add Memo - invalid ASCII MemoFormat', function() {
-    var transaction = new Transaction();
-    transaction.tx_json.TransactionType = 'Payment';
-
+    }, error_regex);
    assert.throws(function() {
      transaction.addMemo(undefined, 'России');
-    }, /^Error: MemoFormat must be valid ASCII$/);
+    }, error_regex);
+    assert.throws(function() {
+      transaction.addMemo(undefined, 'my memo');
+    }, error_regex);
+
+    disallowed_memo_chars.forEach(function(c) {
+      assert.throws(function() {
+        transaction.addMemo(undefined, c);
+      }, error_regex);
+    });
  });

  it('Add Memo - MemoData string', function() {