From 74c8b341884a846fd97b5948eeebcc3c4dd6a68e Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Jun 2015 16:39:21 -0300 Subject: [PATCH 1/5] allow delete proposals after 24hrs --- lib/server.js | 17 +++++++------ test/integration/server.js | 51 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/lib/server.js b/lib/server.js index a0a3a0d..13f7831 100644 --- a/lib/server.js +++ b/lib/server.js @@ -32,6 +32,7 @@ var blockchainExplorerOpts; var messageBroker; + /** * Creates an instance of the Bitcore Wallet Service. * @constructor @@ -865,14 +866,15 @@ WalletService.prototype.removePendingTx = function(opts, cb) { if (!txp.isPending()) return cb(new ClientError('TXNOTPENDING', 'Transaction proposal not pending')); + var now = Math.floor(Date.now() / 1000); + if (now - txp.createdOn < WalletService.lockTimeoutHours * 3600) { + var actors = txp.getActors(); + if (txp.creatorId !== self.copayerId) + return cb(new ClientError('Only creators can remove pending proposals during locktime')); - if (txp.creatorId !== self.copayerId) - return cb(new ClientError('Only creators can remove pending proposals')); - - var actors = txp.getActors(); - - if (actors.length > 1 || (actors.length == 1 && actors[0] !== self.copayerId)) - return cb(new ClientError('TXACTIONED', 'Cannot remove a proposal signed/rejected by other copayers')); + if (actors.length > 1 || (actors.length == 1 && actors[0] !== self.copayerId)) + return cb(new ClientError('TXACTIONED', 'Cannot remove a proposal signed/rejected by other copayers during locktime')); + } self._notify('TxProposalRemoved', {}, function() { self.storage.removeTx(self.walletId, txp.id, cb); @@ -1318,6 +1320,7 @@ WalletService.prototype.getTxHistory = function(opts, cb) { }); }; +WalletService.lockTimeoutHours = 24; WalletService.scanConfig = { SCAN_WINDOW: 20, diff --git a/test/integration/server.js b/test/integration/server.js index 78b759e..2d82fe8 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2656,6 +2656,7 @@ describe('Wallet service', function() { }); }); }); + it('should allow creator to remove an unsigned TX', function(done) { server.removePendingTx({ @@ -2669,7 +2670,7 @@ describe('Wallet service', function() { }); }); - it('should allow creator to remove an signed TX by himself', function(done) { + it('should allow creator to remove a signed TX by himself', function(done) { var signatures = helpers.clientSign(txp, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: txp.id, @@ -2754,7 +2755,7 @@ describe('Wallet service', function() { }); }); - it('should not allow creator copayer to remove an TX signed by other copayer', function(done) { + it('should not allow creator copayer to remove a TX signed by other copayer, in less than 24hrs', function(done) { helpers.getAuthServer(wallet.copayers[1].id, function(server2) { var signatures = helpers.clientSign(txp, TestData.copayers[1].xPrivKey); server2.signTx({ @@ -2772,6 +2773,52 @@ describe('Wallet service', function() { }); }); }); + + + it('should allow creator copayer to remove a TX signed by other copayer, after 24hrs', function(done) { + helpers.getAuthServer(wallet.copayers[1].id, function(server2) { + var signatures = helpers.clientSign(txp, TestData.copayers[1].xPrivKey); + server2.signTx({ + txProposalId: txp.id, + signatures: signatures, + }, function(err) { + should.not.exist(err); + + var clock = sinon.useFakeTimers(Date.now()+1+24*3600*1000); + server.removePendingTx({ + txProposalId: txp.id + }, function(err) { + should.not.exist(err); + clock.restore(); + done(); + }); + }); + }); + }); + + + it('should allow other copayer to remove a TX signed, after 24hrs', function(done) { + helpers.getAuthServer(wallet.copayers[1].id, function(server2) { + var signatures = helpers.clientSign(txp, TestData.copayers[1].xPrivKey); + server2.signTx({ + txProposalId: txp.id, + signatures: signatures, + }, function(err) { + should.not.exist(err); + + var clock = sinon.useFakeTimers(Date.now()+1+24*3600*1000); + server2.removePendingTx({ + txProposalId: txp.id + }, function(err) { + should.not.exist(err); + clock.restore(); + done(); + }); + }); + }); + }); + + }); describe('#getTxHistory', function() { From 5d943a4b271a85a6dc0e8d6a1ea808460b90795c Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Jun 2015 18:00:52 -0300 Subject: [PATCH 2/5] allow to remove creator, in tx without other signatures --- lib/model/txproposal.js | 12 ++++++++++++ lib/server.js | 4 ++-- test/integration/server.js | 19 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index f2b0f12..c102339 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -131,6 +131,18 @@ TxProposal.prototype.getActors = function() { }; +/** + * getApprovers + * + * @return {String[]} copayerIds that approved the tx proposal (accept) + */ +TxProposal.prototype.getApprovers = function() { + return _.pluck( + _.filter(this.actions, { + type: 'accept' + }), 'copayerId'); +}; + /** * getActionBy * diff --git a/lib/server.js b/lib/server.js index 13f7831..b4bfc46 100644 --- a/lib/server.js +++ b/lib/server.js @@ -868,11 +868,11 @@ WalletService.prototype.removePendingTx = function(opts, cb) { var now = Math.floor(Date.now() / 1000); if (now - txp.createdOn < WalletService.lockTimeoutHours * 3600) { - var actors = txp.getActors(); if (txp.creatorId !== self.copayerId) return cb(new ClientError('Only creators can remove pending proposals during locktime')); - if (actors.length > 1 || (actors.length == 1 && actors[0] !== self.copayerId)) + var approvers = txp.getApprovers(); + if (approvers.length > 1 || (approvers.length == 1 && approvers[0] !== self.copayerId)) return cb(new ClientError('TXACTIONED', 'Cannot remove a proposal signed/rejected by other copayers during locktime')); } diff --git a/test/integration/server.js b/test/integration/server.js index 2d82fe8..558907d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2774,6 +2774,25 @@ describe('Wallet service', function() { }); }); + it('should allow creator copayer to remove a TX rejected by other copayer, in less than 24hrs', function(done) { + helpers.getAuthServer(wallet.copayers[1].id, function(server2) { + var signatures = helpers.clientSign(txp, TestData.copayers[1].xPrivKey); + server2.rejectTx({ + txProposalId: txp.id, + signatures: signatures, + }, function(err) { + should.not.exist(err); + server.removePendingTx({ + txProposalId: txp.id + }, function(err) { + should.not.exist(err); + done(); + }); + }); + }); + }); + + it('should allow creator copayer to remove a TX signed by other copayer, after 24hrs', function(done) { helpers.getAuthServer(wallet.copayers[1].id, function(server2) { From 139deac770e620e8e6d3cd40186a7c4b2fa615db Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Thu, 11 Jun 2015 18:38:42 -0300 Subject: [PATCH 3/5] adds .deleteLockTime to getTxproposals --- lib/server.js | 37 +++++++++++++++++++++++++++++-------- test/integration/server.js | 28 ++++++++++++++++++---------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/lib/server.js b/lib/server.js index b4bfc46..c1277ee 100644 --- a/lib/server.js +++ b/lib/server.js @@ -843,6 +843,26 @@ WalletService.prototype.removeWallet = function(opts, cb) { }); }; +WalletService.prototype.getRemainingDeleteLockTime = function(txp) { + var now = Math.floor(Date.now() / 1000); + + var lockTimeRemaining = txp.createdOn + WalletService.deleteLockTime - now; + if (lockTimeRemaining < 0) + return 0; + + // not the creator? need to wait + if (txp.creatorId !== this.copayerId) + return lockTimeRemaining; + + // has other approvers? need to wait + var approvers = txp.getApprovers(); + if (approvers.length > 1 || (approvers.length == 1 && approvers[0] !== this.copayerId)) + return lockTimeRemaining; + + return 0; +}; + + /** * removePendingTx * @@ -866,14 +886,10 @@ WalletService.prototype.removePendingTx = function(opts, cb) { if (!txp.isPending()) return cb(new ClientError('TXNOTPENDING', 'Transaction proposal not pending')); - var now = Math.floor(Date.now() / 1000); - if (now - txp.createdOn < WalletService.lockTimeoutHours * 3600) { - if (txp.creatorId !== self.copayerId) - return cb(new ClientError('Only creators can remove pending proposals during locktime')); - var approvers = txp.getApprovers(); - if (approvers.length > 1 || (approvers.length == 1 && approvers[0] !== self.copayerId)) - return cb(new ClientError('TXACTIONED', 'Cannot remove a proposal signed/rejected by other copayers during locktime')); + var deleteLockTime = self.getRemainingDeleteLockTime(txp); + if (deleteLockTime > 0) { + return cb(new ClientError('TXCANNOTREMOVE', 'Cannot remove this tx proposal during locktime. Time remaining:' + deleteLockTime)); } self._notify('TxProposalRemoved', {}, function() { @@ -1104,6 +1120,10 @@ WalletService.prototype.getPendingTxs = function(opts, cb) { self.storage.fetchPendingTxs(self.walletId, function(err, txps) { if (err) return cb(err); + _.each(txps,function(txp){ + txp.deleteLockTime = self.getRemainingDeleteLockTime(txp); + }); + return cb(null, txps); }); }; @@ -1320,7 +1340,8 @@ WalletService.prototype.getTxHistory = function(opts, cb) { }); }; -WalletService.lockTimeoutHours = 24; +// in seconds +WalletService.deleteLockTime = 24 * 3600; WalletService.scanConfig = { SCAN_WINDOW: 20, diff --git a/test/integration/server.js b/test/integration/server.js index 558907d..1da0b45 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1269,6 +1269,8 @@ describe('Wallet service', function() { server.getPendingTxs({}, function(err, txs) { should.not.exist(err); txs.length.should.equal(1); + // creator + txs[0].deleteLockTime.should.equal(0); server.getBalance({}, function(err, balance) { should.not.exist(err); balance.totalAmount.should.equal(helpers.toSatoshi(300)); @@ -2656,7 +2658,7 @@ describe('Wallet service', function() { }); }); }); - + it('should allow creator to remove an unsigned TX', function(done) { server.removePendingTx({ @@ -2746,6 +2748,7 @@ describe('Wallet service', function() { server2.removePendingTx({ txProposalId: txp.id }, function(err) { + should.exist(err); err.message.should.contain('creators'); server2.getPendingTxs({}, function(err, txs) { txs.length.should.equal(1); @@ -2766,8 +2769,8 @@ describe('Wallet service', function() { server.removePendingTx({ txProposalId: txp.id }, function(err) { - err.code.should.equal('TXACTIONED'); - err.message.should.contain('other copayers'); + err.code.should.equal('TXCANNOTREMOVE'); + err.message.should.contain('Cannot remove'); done(); }); }); @@ -2803,13 +2806,18 @@ describe('Wallet service', function() { }, function(err) { should.not.exist(err); - var clock = sinon.useFakeTimers(Date.now()+1+24*3600*1000); - server.removePendingTx({ - txProposalId: txp.id - }, function(err) { + server.getPendingTxs({}, function(err, txs) { should.not.exist(err); - clock.restore(); - done(); + txs[0].deleteLockTime.should.be.above(WalletService.deleteLockTime-10); + + var clock = sinon.useFakeTimers(Date.now() + 1 + 24 * 3600 * 1000); + server.removePendingTx({ + txProposalId: txp.id + }, function(err) { + should.not.exist(err); + clock.restore(); + done(); + }); }); }); }); @@ -2825,7 +2833,7 @@ describe('Wallet service', function() { }, function(err) { should.not.exist(err); - var clock = sinon.useFakeTimers(Date.now()+1+24*3600*1000); + var clock = sinon.useFakeTimers(Date.now() + 1 + 24 * 3600 * 1000); server2.removePendingTx({ txProposalId: txp.id }, function(err) { From 94becc89609cf9e55e548de665e3325d8476c3a1 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Fri, 12 Jun 2015 10:06:15 -0300 Subject: [PATCH 4/5] fix notify order --- lib/server.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/server.js b/lib/server.js index c1277ee..9f0da68 100644 --- a/lib/server.js +++ b/lib/server.js @@ -858,7 +858,7 @@ WalletService.prototype.getRemainingDeleteLockTime = function(txp) { var approvers = txp.getApprovers(); if (approvers.length > 1 || (approvers.length == 1 && approvers[0] !== this.copayerId)) return lockTimeRemaining; - + return 0; }; @@ -891,9 +891,8 @@ WalletService.prototype.removePendingTx = function(opts, cb) { if (deleteLockTime > 0) { return cb(new ClientError('TXCANNOTREMOVE', 'Cannot remove this tx proposal during locktime. Time remaining:' + deleteLockTime)); } - - self._notify('TxProposalRemoved', {}, function() { - self.storage.removeTx(self.walletId, txp.id, cb); + self.storage.removeTx(self.walletId, txp.id, function() { + self._notify('TxProposalRemoved', {}, cb); }); }); }); @@ -1120,7 +1119,7 @@ WalletService.prototype.getPendingTxs = function(opts, cb) { self.storage.fetchPendingTxs(self.walletId, function(err, txps) { if (err) return cb(err); - _.each(txps,function(txp){ + _.each(txps, function(txp) { txp.deleteLockTime = self.getRemainingDeleteLockTime(txp); }); From dcfff424d78882cbefeb51c8f0bca9164f329aa9 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Fri, 12 Jun 2015 10:11:54 -0300 Subject: [PATCH 5/5] fix err msg --- lib/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index 9f0da68..a590b15 100644 --- a/lib/server.js +++ b/lib/server.js @@ -889,7 +889,7 @@ WalletService.prototype.removePendingTx = function(opts, cb) { var deleteLockTime = self.getRemainingDeleteLockTime(txp); if (deleteLockTime > 0) { - return cb(new ClientError('TXCANNOTREMOVE', 'Cannot remove this tx proposal during locktime. Time remaining:' + deleteLockTime)); + return cb(new ClientError('TXCANNOTREMOVE', 'Cannot remove this tx proposal during locktime')); } self.storage.removeTx(self.walletId, txp.id, function() { self._notify('TxProposalRemoved', {}, cb);