Merge pull request #589 from wltsmrz/multisign-update

Fix Transaction.complete() for multisigned transactions

src/core/transaction.js

@@ -384,24 +384,32 @@ Transaction.prototype.err = function(error, errorMessage) {
 };
 
 Transaction.prototype.complete = function() {
-  // Auto-fill the secret
-  this._secret = this._secret || this.getSecret();
+  const hasMultiSigners = this.hasMultiSigners();
 
-  if (_.isUndefined(this._secret)) {
-    return this.err('tejSecretUnknown', 'Missing secret');
-  }
+  if (!hasMultiSigners) {
+    // Auto-fill the secret
+    this._secret = this._secret || this.getSecret();
 
-  if (this.remote && !(this.remote.local_signing || this.remote.trusted)) {
-    return this.err(
-      'tejServerUntrusted',
-      'Attempt to give secret to untrusted server');
+    if (_.isUndefined(this._secret)) {
+      return this.err('tejSecretUnknown', 'Missing secret');
+    }
+
+    if (this.remote && !(this.remote.local_signing || this.remote.trusted)) {
+      return this.err(
+        'tejServerUntrusted',
+        'Attempt to give secret to untrusted server');
+    }
   }
 
   if (_.isUndefined(this.tx_json.SigningPubKey)) {
-    try {
-      this.setSigningPubKey(this.getSigningPubKey());
-    } catch (e) {
-      return this.err('tejSecretInvalid', 'Invalid secret');
+    if (hasMultiSigners) {
+      this.setSigningPubKey('');
+    } else {
+      try {
+        this.setSigningPubKey(this.getSigningPubKey());
+      } catch (e) {
+        return this.err('tejSecretInvalid', 'Invalid secret');
+      }
     }
   }
 

src/core/transactionmanager.js

@@ -724,6 +724,10 @@ TransactionManager.prototype.submit = function(tx) {
     return;
   }
 
+  tx.once('cleanup', function() {
+    self.getPending().remove(tx);
+  });
+
   if (!_.isNumber(tx.tx_json.Sequence)) {
     // Honor manually-set sequences
     tx.setSequence(this._nextSequence++);
@@ -735,13 +739,8 @@ TransactionManager.prototype.submit = function(tx) {
 
   if (tx.hasMultiSigners()) {
     tx.setResubmittable(false);
-    tx.setSigningPubKey('');
   }
 
-  tx.once('cleanup', function() {
-    self.getPending().remove(tx);
-  });
-
   if (!tx.complete()) {
     this._nextSequence -= 1;
     return;

test/transaction-test.js

@@ -2302,6 +2302,10 @@ describe('Transaction', function() {
     {Signer: s2},
     {Signer: s1}
     ]);
+
+    transaction.remote = new Remote();
+    assert(transaction.complete());
+    assert.strictEqual(transaction.tx_json.SigningPubKey, '');
   });
 
   it('Multisign -- missing LastLedgerSequence', function() {
@@ -2317,4 +2321,27 @@ describe('Transaction', function() {
       transaction.getMultiSigningJson();
     });
   });
+
+  it('Multisign -- LastLedgerSequence autofill', function() {
+    const transaction = Transaction.from_json({
+      Account: 'rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn',
+      Sequence: 1,
+      Fee: '100',
+      TransactionType: 'AccountSet',
+      Flags: 0
+    });
+
+    const sequence = 1;
+    transaction.remote = {
+      getLedgerSequenceSync: () => {
+        return sequence;
+      }
+    };
+
+    const mJson = transaction.getMultiSigningJson();
+    assert.strictEqual(mJson.LastLedgerSequence,
+                       sequence + 1 + transaction._lastLedgerOffset);
+    assert.strictEqual(transaction.tx_json.LastLedgerSequence,
+                       sequence + 1 + transaction._lastLedgerOffset);
+  });
 });