Merge pull request #225 from isocolsky/safe_broadcast

Safe broadcast

lib/blockchainexplorer.js

@@ -22,6 +22,7 @@ function BlockChainExplorer(opts) {
   switch (provider) {
     case 'insight':
       var explorer = new Explorers.Insight(url, network);
+      explorer.getTransaction = _.bind(getTransactionInsight, explorer, url);
       explorer.getTransactions = _.bind(getTransactionsInsight, explorer, url);
       explorer.getAddressActivity = _.bind(getAddressActivityInsight, explorer, url);
       explorer.initSocket = _.bind(initSocketInsight, explorer, url);
@@ -31,6 +32,19 @@ function BlockChainExplorer(opts) {
   };
 };
 
+function getTransactionInsight(url, txid, cb) {
+  var url = url + '/api/tx/' + txid;
+  var args = {
+    method: "GET",
+    url: url,
+  };
+
+  request(args, function(err, res, tx) {
+    if (err || res.statusCode != 200) return cb(err || res);
+    return cb(null, tx);
+  });
+};
+
 function getTransactionsInsight(url, addresses, from, to, cb) {
   var qs = [];
   if (_.isNumber(from)) qs.push('from=' + from);

lib/server.js

@@ -920,6 +920,18 @@ WalletService.prototype._broadcastTx = function(txp, cb) {
   })
 };
 
+WalletService.prototype._checkTxInBlockchain = function(txp, cb) {
+  var tx = txp.getBitcoreTx();
+  var bc = this._getBlockchainExplorer('insight', txp.getNetworkName());
+  bc.getTransaction(tx.id, function(err, tx) {
+    if (err) {
+      log.error('Could not get transaction info', err);
+      return cb(new ClientError('BLOCKCHAINERROR', 'Could not get transaction info'));
+    }
+    return cb(null, tx);
+  })
+};
+
 /**
  * Sign a transaction proposal.
  * @param {Object} opts
@@ -993,6 +1005,20 @@ WalletService.prototype.broadcastTx = function(opts, cb) {
   if (!Utils.checkRequired(opts, ['txProposalId']))
     return cb(new ClientError('Required argument missing'));
 
+  function setBroadcasted(txp, txid, cb) {
+    txp.setBroadcasted(txid);
+    self.storage.storeTx(self.walletId, txp, function(err) {
+      if (err) return cb(err);
+
+      self._notify('NewOutgoingTx', {
+        txProposalId: opts.txProposalId,
+        txid: txid
+      }, function() {
+        return cb(null, txp);
+      });
+    });
+  };
+
   self.getWallet({}, function(err, wallet) {
     if (err) return cb(err);
 
@@ -1008,19 +1034,18 @@ WalletService.prototype.broadcastTx = function(opts, cb) {
         return cb(new ClientError('TXNOTACCEPTED', 'The transaction proposal is not accepted'));
 
       self._broadcastTx(txp, function(err, txid) {
-        if (err) return cb(err);
+        if (err) {
+          var broadcastErr = err;
+          // Check if tx already in blockchain
+          self._checkTxInBlockchain(txp, function(err, tx) {
+            if (err) return cb(err);
+            if (!tx) return cb(broadcastErr);
 
-        txp.setBroadcasted(txid);
-        self.storage.storeTx(self.walletId, txp, function(err) {
-          if (err) return cb(err);
-
-          self._notify('NewOutgoingTx', {
-            txProposalId: opts.txProposalId,
-            txid: txid
-          }, function() {
-            return cb(null, txp);
+            setBroadcasted(txp, tx.txid, cb);
           });
-        });
+        } else {
+          setBroadcasted(txp, txid, cb);
+        }
       });
     });
   });

test/integration/server.js

@@ -1662,7 +1662,7 @@ describe('Wallet service', function() {
       });
     });
 
-    it('should fail to brodcast an already broadcasted tx', function(done) {
+    it('should fail to brodcast a tx already marked as broadcasted', function(done) {
       helpers.stubBroadcast('999');
       server.broadcastTx({
         txProposalId: txpid
@@ -1696,6 +1696,50 @@ describe('Wallet service', function() {
 
     it('should keep tx as accepted if unable to broadcast it', function(done) {
       helpers.stubBroadcastFail();
+      blockchainExplorer.getTransaction = sinon.stub().callsArgWith(1, null, null);
+      server.broadcastTx({
+        txProposalId: txpid
+      }, function(err) {
+        should.exist(err);
+        err.code.should.equal('BLOCKCHAINERROR');
+        server.getTx({
+          txProposalId: txpid
+        }, function(err, txp) {
+          should.not.exist(err);
+          should.not.exist(txp.txid);
+          txp.isBroadcasted().should.be.false;
+          should.not.exist(txp.broadcastedOn);
+          txp.isAccepted().should.be.true;
+          done();
+        });
+      });
+    });
+
+    it('should mark tx as broadcasted if accepted but already in blockchain', function(done) {
+      helpers.stubBroadcastFail();
+      blockchainExplorer.getTransaction = sinon.stub().callsArgWith(1, null, {
+        txid: '999'
+      });
+      server.broadcastTx({
+        txProposalId: txpid
+      }, function(err) {
+        should.not.exist(err);
+        server.getTx({
+          txProposalId: txpid
+        }, function(err, txp) {
+          should.not.exist(err);
+          should.exist(txp.txid);
+          txp.txid.should.equal('999');
+          txp.isBroadcasted().should.be.true;
+          should.exist(txp.broadcastedOn);
+          done();
+        });
+      });
+    });
+
+    it('should keep tx as accepted if broadcast fails and cannot check tx in blockchain', function(done) {
+      helpers.stubBroadcastFail();
+      blockchainExplorer.getTransaction = sinon.stub().callsArgWith(1, 'bc check error');
       server.broadcastTx({
         txProposalId: txpid
       }, function(err) {