From 7975bb24996d34a66aac7dcf5aa393b69609ffdf Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Tue, 10 Feb 2015 16:30:58 -0300 Subject: [PATCH 1/4] remove pending tx --- lib/model/txproposal.js | 4 ++++ lib/server.js | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index b822999..cc81f26 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -93,6 +93,10 @@ TxProposal.prototype.getRawTx = function() { }; +TxProposal.prototype.getActors = function() { + return _.keys(this.actions); +}; + TxProposal.prototype.addAction = function(copayerId, type, signatures) { var action = new TxProposalAction({ copayerId: copayerId, diff --git a/lib/server.js b/lib/server.js index ef4e0c1..6284fb9 100644 --- a/lib/server.js +++ b/lib/server.js @@ -482,6 +482,39 @@ CopayServer.prototype.removeWallet = function(opts, cb) { }); }; +/** + * removePendingTx + * + * @param opts + * @param {string} opts.id - The tx id. + * @return {undefined} + */ +CopayServer.prototype.removePendingTx = function(opts, cb) { + var self = this; + + if (!Utils.checkRequired(opts, ['id'])) + return cb(new ClientError('Required argument missing')); + + Utils.runLocked(self.id, cb, function(cb) { + + self.storage.fetchTx(self.walletId, opts.id, function(err, txp) { + if (err) return cb(err); + if (!txp) + return cb(new ClientError('Transaction proposal not found')); + + if (!txp.isPending()) + return cb(new ClientError('Transaction proposal not pending')); + + var actors = txp.getActors(); + + if (actors.length > 1 || actors[0] !== self.copayerId) + return cb(new ClientError('No allowed to erase this TX')); + + self.storage.removeTx(opts.id, cb); + }); + }); +}; + CopayServer.prototype._broadcastTx = function(txp, cb) { var raw = txp.getRawTx(); @@ -510,8 +543,9 @@ CopayServer.prototype.signTx = function(opts, cb) { }, function(err, txp) { if (err) return cb(err); if (!txp) return cb(new ClientError('Transaction proposal not found')); + var action = _.find(txp.actions, { - copayerId: opts.copayerId + copayerId: self.copayerId }); if (action) return cb(new ClientError('CVOTED', 'Copayer already voted on this transaction proposal')); From 324c058303419acc7c3a49c8c09ab541819f5e99 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Tue, 10 Feb 2015 16:55:00 -0300 Subject: [PATCH 2/4] add test to reject --- lib/model/txproposal.js | 22 ++++++++ test/integration.js | 114 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index cc81f26..171850e 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -93,10 +93,32 @@ TxProposal.prototype.getRawTx = function() { }; +/** + * getActors + * + * @return {String[]} copayerIds that performed actions in this proposal (accept / reject) + */ TxProposal.prototype.getActors = function() { return _.keys(this.actions); }; + +/** + * getActionBy + * + * @param {String} copayerId + * @return {Object} type / createdOn + */ +TxProposal.prototype.getActionBy = function(copayerId) { + var a = this.actions[copayerId]; + if (!a) return null; + + return { + type: a.type, + createdOn: a.createdOn, + }; +}; + TxProposal.prototype.addAction = function(copayerId, type, signatures) { var action = new TxProposalAction({ copayerId: copayerId, diff --git a/test/integration.js b/test/integration.js index b548f17..ba5e772 100644 --- a/test/integration.js +++ b/test/integration.js @@ -66,7 +66,7 @@ helpers.createAndJoinWallet = function(m, n, cb) { helpers.getAuthServer(copayerIds[0], function(s) { s.getWallet({}, function(err, w) { - cb(s, w, _.take(TestData.copayers, w.n)); + cb(s, w, _.take(TestData.copayers, w.n), copayerIds); }); }); }); @@ -765,14 +765,66 @@ describe('Copay server', function() { }); }); - describe('#signTx', function() { - var server, wallet, copayerPriv, txid; + + describe('#rejectTx', function() { + var server, wallet, copayerPriv, txid, copayerIds; beforeEach(function(done) { - helpers.createAndJoinWallet(2, 2, function(s, w, c) { + helpers.createAndJoinWallet(2, 2, function(s, w, c, ids) { server = s; wallet = w; copayerPriv = c; + copayerIds = ids; + server.createAddress({}, function(err, address) { + helpers.createUtxos(server, wallet, helpers.toSatoshi([1, 2, 3, 4, 5, 6, 7, 8]), function(utxos) { + helpers.stubBlockExplorer(server, utxos); + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 10, null, copayerPriv[0].privKey); + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + tx.should.exist; + txid = tx.id; + done(); + }); + }); + }); + }); + }); + + it('should reject a TX', function(done) { + server.getPendingTxs({}, function(err, txs) { + var tx = txs[0]; + tx.id.should.equal(txid); + + server.rejectTx({ + txProposalId: txid, + }, function(err) { + should.not.exist(err); + server.getPendingTxs({}, function(err, txs) { + should.not.exist(err); + var tx = txs[0]; + tx.id.should.equal(txid); + + var actors = tx.getActors(); + actors.length.should.equal(1); + actors[0].should.equal(copayerIds[0]); + tx.getActionBy(copayerIds[0]).type.should.equal('reject'); + + done(); + }); + }); + }); + }); + }); + + describe('#signTx', function() { + var server, wallet, copayerPriv, txid, copayerIds; + + beforeEach(function(done) { + helpers.createAndJoinWallet(2, 2, function(s, w, c, ids) { + server = s; + wallet = w; + copayerPriv = c; + copayerIds = ids; server.createAddress({}, function(err, address) { helpers.createUtxos(server, wallet, helpers.toSatoshi([1, 2, 3, 4, 5, 6, 7, 8]), function(utxos) { helpers.stubBlockExplorer(server, utxos); @@ -799,7 +851,18 @@ describe('Copay server', function() { signatures: signatures, }, function(err) { should.not.exist(err); - done(); + server.getPendingTxs({}, function(err, txs) { + should.not.exist(err); + var tx = txs[0]; + tx.id.should.equal(txid); + + var actors = tx.getActors(); + actors.length.should.equal(1); + actors[0].should.equal(copayerIds[0]); + tx.getActionBy(copayerIds[0]).type.should.equal('accept'); + + done(); + }); }); }); }); @@ -836,6 +899,47 @@ describe('Copay server', function() { }); }); }); + + it('should fail when signing a TX previously rejected', function(done) { + server.getPendingTxs({}, function(err, txs) { + var tx = txs[0]; + tx.id.should.equal(txid); + + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey, wallet.n); + server.signTx({ + txProposalId: txid, + signatures: signatures, + }, function(err) { + server.rejectTx({ + txProposalId: txid, + }, function(err) { + err.code.should.contain('CVOTED'); + done(); + }); + }); + }); + }); + + it('should fail when rejected a previously signed TX', function(done) { + server.getPendingTxs({}, function(err, txs) { + var tx = txs[0]; + tx.id.should.equal(txid); + + server.rejectTx({ + txProposalId: txid, + }, function(err) { + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey, wallet.n); + server.signTx({ + txProposalId: txid, + signatures: signatures, + }, function(err) { + err.code.should.contain('CVOTED'); + done(); + }); + }); + }); + }); + }); From f8702ebcce3750c749469163b0730ac85d4b8762 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Tue, 10 Feb 2015 18:04:50 -0300 Subject: [PATCH 3/4] rm pending Tx --- lib/server.js | 14 +++-- lib/storage.js | 5 +- test/integration.js | 129 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 128 insertions(+), 20 deletions(-) diff --git a/lib/server.js b/lib/server.js index 6284fb9..e5777df 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1,5 +1,4 @@ 'use strict'; - var _ = require('lodash'); var $ = require('preconditions').singleton(); var async = require('async'); @@ -505,12 +504,19 @@ CopayServer.prototype.removePendingTx = function(opts, cb) { if (!txp.isPending()) return cb(new ClientError('Transaction proposal not pending')); + + if (txp.creatorId !== self.copayerId) + return cb(new ClientError('Not allowed to erase this TX')); + var actors = txp.getActors(); - if (actors.length > 1 || actors[0] !== self.copayerId) - return cb(new ClientError('No allowed to erase this TX')); + if (actors.length > 1) + return cb(new ClientError('Not allowed to erase this TX')); - self.storage.removeTx(opts.id, cb); + if (actors.length == 1 && actors[0] !== self.copayerId) + return cb(new ClientError('Not allowed to erase this TX')); + + self.storage.removeTx(self.walletId, opts.id, cb); }); }); }; diff --git a/lib/storage.js b/lib/storage.js index e637516..fcba017 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -190,11 +190,10 @@ Storage.prototype.storeTx = function(walletId, txp, cb) { Storage.prototype.removeTx = function(walletId, txProposalId, cb) { var ops = [{ type: 'del', - key: KEY.TXP(walletId, txp.id), + key: KEY.TXP(walletId, txProposalId), }, { type: 'del', - key: KEY.PENDING_TXP(walletId, txp.id), - value: txp, + key: KEY.PENDING_TXP(walletId, txProposalId), }]; this.db.batch(ops, cb); diff --git a/test/integration.js b/test/integration.js index ba5e772..1f632a3 100644 --- a/test/integration.js +++ b/test/integration.js @@ -130,11 +130,11 @@ helpers.stubBlockExplorer = function(server, utxos, txid) { -helpers.clientSign = function(tx, xpriv, n) { +helpers.clientSign = function(tx, xprivHex) { //Derive proper key to sign, for each input var privs = [], derived = {}; - var xpriv = new Bitcore.HDPrivateKey(TestData.copayers[0].xPrivKey); + var xpriv = new Bitcore.HDPrivateKey(xprivHex); _.each(tx.inputs, function(i) { if (!derived[i.path]) { @@ -146,7 +146,7 @@ helpers.clientSign = function(tx, xpriv, n) { var t = new Bitcore.Transaction(); _.each(tx.inputs, function(i) { - t.from(i, i.publicKeys, n); + t.from(i, i.publicKeys, tx.requiredSignatures); }); t.to(tx.toAddress, tx.amount) @@ -435,7 +435,7 @@ describe('Copay server', function() { describe('#verifyMessageSignature', function() { var server, wallet; beforeEach(function(done) { - helpers.createAndJoinWallet(2, 2, function(s, w) { + helpers.createAndJoinWallet(2, 3, function(s, w) { server = s; wallet = w; done(); @@ -559,7 +559,7 @@ describe('Copay server', function() { describe('#createTx', function() { var server, wallet, copayerPriv; beforeEach(function(done) { - helpers.createAndJoinWallet(2, 2, function(s, w, c) { + helpers.createAndJoinWallet(2, 3, function(s, w, c) { server = s; wallet = w; copayerPriv = c; @@ -770,7 +770,7 @@ describe('Copay server', function() { var server, wallet, copayerPriv, txid, copayerIds; beforeEach(function(done) { - helpers.createAndJoinWallet(2, 2, function(s, w, c, ids) { + helpers.createAndJoinWallet(2, 3, function(s, w, c, ids) { server = s; wallet = w; copayerPriv = c; @@ -820,7 +820,7 @@ describe('Copay server', function() { var server, wallet, copayerPriv, txid, copayerIds; beforeEach(function(done) { - helpers.createAndJoinWallet(2, 2, function(s, w, c, ids) { + helpers.createAndJoinWallet(2, 3, function(s, w, c, ids) { server = s; wallet = w; copayerPriv = c; @@ -845,7 +845,7 @@ describe('Copay server', function() { var tx = txs[0]; tx.id.should.equal(txid); - var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey, wallet.n); + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: txid, signatures: signatures, @@ -867,12 +867,28 @@ describe('Copay server', function() { }); }); + + it('should fail to sign with a xpriv from other copayer', function(done) { + server.getPendingTxs({}, function(err, txs) { + var tx = txs[0]; + tx.id.should.equal(txid); + var signatures = helpers.clientSign(tx, TestData.copayers[1].xPrivKey); + server.signTx({ + txProposalId: txid, + signatures: signatures, + }, function(err) { + err.code.should.contain('BADSIG'); + done(); + }); + }); + }); + it('should fail if one signature is broken', function(done) { server.getPendingTxs({}, function(err, txs) { var tx = txs[0]; tx.id.should.equal(txid); - var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey, wallet.n); + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey); signatures[0] = 1; server.signTx({ @@ -884,6 +900,7 @@ describe('Copay server', function() { }); }); }); + it('should fail on invalid signature', function(done) { server.getPendingTxs({}, function(err, txs) { var tx = txs[0]; @@ -905,7 +922,7 @@ describe('Copay server', function() { var tx = txs[0]; tx.id.should.equal(txid); - var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey, wallet.n); + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: txid, signatures: signatures, @@ -928,7 +945,7 @@ describe('Copay server', function() { server.rejectTx({ txProposalId: txid, }, function(err) { - var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey, wallet.n); + var signatures = helpers.clientSign(tx, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: txid, signatures: signatures, @@ -970,7 +987,7 @@ describe('Copay server', function() { server.getPendingTxs({}, function(err, txps) { var txp = txps[0]; txp.id.should.equal(txpid); - var signatures = helpers.clientSign(txp, TestData.copayers[0].xPrivKey, wallet.n); + var signatures = helpers.clientSign(txp, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: txpid, signatures: signatures, @@ -996,7 +1013,7 @@ describe('Copay server', function() { server.getPendingTxs({}, function(err, txps) { var txp = txps[0]; txp.id.should.equal(txpid); - var signatures = helpers.clientSign(txp, TestData.copayers[0].xPrivKey, wallet.n); + var signatures = helpers.clientSign(txp, TestData.copayers[0].xPrivKey); server.signTx({ txProposalId: txpid, signatures: signatures, @@ -1234,4 +1251,90 @@ describe('Copay server', function() { }, cat); }); }); + + + describe('#removePendingTx', function() { + var server, wallet, copayerPriv, txp; + beforeEach(function(done) { + helpers.createAndJoinWallet(2, 3, function(s, w, c) { + server = s; + wallet = w; + copayerPriv = c; + server.createAddress({}, function(err, address) { + helpers.createUtxos(server, wallet, helpers.toSatoshi([100, 200]), function(utxos) { + helpers.stubBlockExplorer(server, utxos); + var txOpts = helpers.createProposalOpts('18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', 80, 'some message', copayerPriv[0].privKey); + server.createTx(txOpts, function(err, tx) { + server.getPendingTxs({}, function(err, txs) { + txp = txs[0]; + done(); + }); + }); + }); + }); + }); + }); + + it('should allow creator to remove an unsigned TX', function(done) { + server.removePendingTx({ + id: txp.id + }, function(err) { + should.not.exist(err); + server.getPendingTxs({}, function(err, txs) { + txs.length.should.equal(0); + done(); + }); + }); + }); + + it('should allow creator to remove an signed TX by himself', function(done) { + var signatures = helpers.clientSign(txp, TestData.copayers[0].xPrivKey); + server.signTx({ + txProposalId: txp[0], + signatures: signatures, + }, function(err) { + server.removePendingTx({ + id: txp.id + }, function(err) { + should.not.exist(err); + server.getPendingTxs({}, function(err, txs) { + txs.length.should.equal(0); + done(); + }); + }); + }); + }); + + it('should not allow non-creator copayer to remove an unsigned TX ', function(done) { + helpers.getAuthServer(wallet.copayers[1].id, function(server2) { + server2.removePendingTx({ + id: txp.id + }, function(err) { + err.message.should.contain('Not allowed'); + server2.getPendingTxs({}, function(err, txs) { + txs.length.should.equal(1); + done(); + }); + }); + }); + }); + + it('should not allow creator copayer to remove an TX signed by other copayer', 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); + server.removePendingTx({ + id: txp.id + }, function(err) { + err.message.should.contain('Not allowed'); + done(); + }); + }); + }); + }); + }); }); From 7ba4d4814c43712fca34126912d4922becaa7b14 Mon Sep 17 00:00:00 2001 From: Matias Alejo Garcia Date: Wed, 11 Feb 2015 11:41:05 -0300 Subject: [PATCH 4/4] fix var name --- lib/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index e5777df..6ac2dd3 100644 --- a/lib/server.js +++ b/lib/server.js @@ -494,7 +494,7 @@ CopayServer.prototype.removePendingTx = function(opts, cb) { if (!Utils.checkRequired(opts, ['id'])) return cb(new ClientError('Required argument missing')); - Utils.runLocked(self.id, cb, function(cb) { + Utils.runLocked(self.walletId, cb, function(cb) { self.storage.fetchTx(self.walletId, opts.id, function(err, txp) { if (err) return cb(err);