Keep track of all transactionIDs for a given Transaction that go on the network

src/js/ripple/transaction.js

@@ -81,6 +81,12 @@ function Transaction(remote) {
 
   this.finalized              = false;
   this._previous_signing_hash = void(0);
+
+  // We aren't clever enough to eschew preventative measures so we keep an array
+  // of all submitted transactionIDs (which can change due to load_factor
+  // effecting the Fee amount). This should be populated with a transactionID
+  // any time it goes on the network
+  this.submittedTxnIDs = []
 };
 
 util.inherits(Transaction, EventEmitter);
@@ -210,6 +216,12 @@ Transaction.prototype.signingHash = function() {
   return this.hash(config.testnet ? 'HASH_TX_SIGN_TESTNET' : 'HASH_TX_SIGN');
 };
 
+Transaction.prototype.addSubmittedTxnID = function(hash) {
+  if (-1 == this.submittedTxnIDs.indexOf(hash)) {
+    this.submittedTxnIDs.push(hash);
+  }
+};
+
 Transaction.prototype.hash = function(prefix, as_uint256) {
   if (typeof prefix === 'string') {
     if (typeof hashprefixes[prefix] === 'undefined') {

src/js/ripple/transactionmanager.js

@@ -34,11 +34,14 @@ function TransactionManager(account) {
 
     self._sequenceCache[sequence] = transaction;
 
-    var pending = self._pending.get('_hash', hash);
+    // ND: we need to check against all submissions ids
+    var pending = self._pending.getBySubmissions(hash);
+    // var pending = self._pending.get('_hash', hash);
 
     self._remote._trace('transactionmanager: transaction_received: %s', transaction.transaction);
 
     if (pending) {
+      // ND: A `success` handler will `finalize` this later
       pending.emit('success', transaction);
     } else {
       self._cache[hash] = transaction;
@@ -48,6 +51,7 @@ function TransactionManager(account) {
   this._account.on('transaction-outbound', transactionReceived);
 
   function adjustFees() {
+    // ND: note, that `Fee` is a component of a transactionID
     self._pending.forEach(function(pending) {
       if (self._remote.local_fee && pending.tx_json.Fee) {
         var oldFee = pending.tx_json.Fee;
@@ -248,7 +252,11 @@ TransactionManager.prototype._request = function(tx) {
     submitRequest.tx_json(tx.tx_json);
   }
 
-  tx._hash = tx.hash();
+  // ND: We could consider sharing the work with tx_blob when doing
+  // local_signing
+
+  tx.addSubmittedTxnID(tx.hash());
+  // tx._hash = tx.hash();
 
   remote._trace('transactionmanager: submit: %s', tx.tx_json);
 
@@ -302,8 +310,11 @@ TransactionManager.prototype._request = function(tx) {
     // Finalized (e.g. aborted) transactions must stop all activity
     if (tx.finalized) return;
 
+
+    // ND: If for some unknown reason our hash wasn't computed correctly this is
+    // an extra measure.
     if (message.tx_json && message.tx_json.hash) {
-      tx._hash = message.tx_json.hash;
+      tx.addSubmittedTxnID(message.tx_json.hash);
     }
 
     message.result = message.engine_result || '';
@@ -331,6 +342,16 @@ TransactionManager.prototype._request = function(tx) {
   };
 
   submitRequest.timeout(this._submissionTimeout, function() {
+    // ND: What if the response is just slow and we get a response that
+    // `submitted` above will cause to have concurrent resubmit logic streams?
+    // It's simpler to just mute handlers and look out for finalized
+    // `transaction` messages.
+
+    // ND: We should audit the code for other potential multiple resubmit
+    // streams. Connection/reconnection could be one? That's why it's imperative
+    // that ALL transactionIDs sent over network are tracked.
+    submitRequest.removeAllListeners();
+
     // Finalized (e.g. aborted) transactions must stop all activity
     if (tx.finalized) return;
 
@@ -397,7 +418,9 @@ TransactionManager.prototype.submit = function(tx) {
 
   function finalize(message) {
     if (!tx.finalized) {
-      self._pending.removeHash(tx._hash);
+      // ND: We can just remove this `tx` by identity
+      self._pending.remove(tx);
+      // self._pending.removeHash(tx._hash);
       remote._trace('transactionmanager: finalize_transaction: %s', tx.tx_json);
       tx.finalized = true;
       tx.emit('final', message);
@@ -421,6 +444,10 @@ TransactionManager.prototype.submit = function(tx) {
   } else if (fee && fee > this._maxFee) {
     tx.emit('error', new RippleError('tejMaxFeeExceeded', 'Max fee exceeded'));
   } else {
+    // ND: this is the ONLY place we put the tx into the queue. The
+    // TransactionQueue queue is merely a list, so any mutations to tx._hash
+    // will cause subsequent look ups (eg. inside 'transaction-outbound'
+    // validated transaction clearing) to fail.
     this._pending.push(tx);
     this._request(tx);
   }

src/js/ripple/transactionqueue.js

@@ -13,7 +13,18 @@ TransactionQueue.prototype.length = function() {
   return this._queue.length;
 };
 
-TransactionQueue.prototype.indexOf = function(prop, val) {
+TransactionQueue.prototype.getBySubmissions = function(hash) {
+  for (var i=0, tx; tx=this._queue[i]; i++) {
+    for (var j=0, id; id=tx.submittedTxnIDs[j]; j++) {
+      if (hash === id) {
+        return tx;
+      };
+    }
+  }
+  return false;
+};
+
+/*TransactionQueue.prototype.indexOf = function(prop, val) {
   var index = -1;
   for (var i=0, tx; tx=this._queue[i]; i++) {
     if (tx[prop] === val) {
@@ -23,17 +34,22 @@ TransactionQueue.prototype.indexOf = function(prop, val) {
   }
   return index;
 };
-
-TransactionQueue.prototype.hasHash = function(hash) {
-  return this.indexOf('hash', hash) !== -1;
+*/
+  // ND: fixed `hash` vs _`hash`
+/*TransactionQueue.prototype.hasHash = function(hash) {
+  return this.indexOf('_hash', hash) !== -1;
 };
+*/
 
-TransactionQueue.prototype.get = function(prop, val) {
+
+/*TransactionQueue.prototype.get = function(prop, val) {
   var index = this.indexOf(prop, val);
   return index > -1 ? this._queue[index] : false;
 };
+*/
 
-TransactionQueue.prototype.removeSequence = function(sequence) {
+// ND: These are unused
+/*TransactionQueue.prototype.removeSequence = function(sequence) {
   var result = [ ];
   for (var i=0, tx; tx=this._queue[i]; i++) {
     if (!tx.tx_json) continue;
@@ -41,15 +57,26 @@ TransactionQueue.prototype.removeSequence = function(sequence) {
   }
   this._queue = result;
 };
-
-TransactionQueue.prototype.removeHash = function(hash) {
+*/
+/*TransactionQueue.prototype.removeHash = function(hash) {
   var result = [ ];
   for (var i=0, tx; tx=this._queue[i]; i++) {
     if (!tx.tx_json) continue;
-    if (tx._hash !== hash) result.push(tx);
+    if (tx.hash !== hash) result.push(tx);
   }
   this._queue = result;
 };
+*/
+
+// ND: We are just removing the Transaction by identity
+TransactionQueue.prototype.remove = function(removedTx) {
+  for (var i = this._queue.length - 1; i >= 0; i--) {
+    if (this._queue[i] === removedTx) {
+      this._queue.splice(i, 1);
+      break;
+    };
+  };
+};
 
 TransactionQueue.prototype.forEach = function() {
   Array.prototype.forEach.apply(this._queue, arguments);