From 619db55e6c0e9d5e19f704f1677905d1db4ebe46 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 20 Feb 2015 12:33:46 -0300 Subject: [PATCH 1/4] refactor actions as array --- lib/model/txproposal.js | 23 ++++++++++++----------- lib/storage.js | 2 +- test/integration/server.js | 23 +++++++++++------------ test/txproposal.js | 2 +- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index 426a826..3742b6a 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -30,7 +30,7 @@ TxProposal.create = function(opts) { x.requiredSignatures = opts.requiredSignatures; x.requiredRejections = opts.requiredRejections; x.status = 'pending'; - x.actions = {}; + x.actions = []; return x; }; @@ -53,9 +53,8 @@ TxProposal.fromObj = function(obj) { x.status = obj.status; x.txid = obj.txid; x.inputPaths = obj.inputPaths; - x.actions = obj.actions; - _.each(x.actions, function(action, copayerId) { - x.actions[copayerId] = TxProposalAction.fromObj(action); + x.actions = _.map(obj.actions, function(action) { + return TxProposalAction.fromObj(action); }); return x; @@ -74,8 +73,8 @@ TxProposal.prototype._updateStatus = function() { TxProposal.prototype._getCurrentSignatures = function() { - var acceptedActions = _.filter(this.actions, function(x) { - return x && x.type == 'accept'; + var acceptedActions = _.filter(this.actions, { + type: 'accept' }); return _.map(acceptedActions, function(x) { @@ -125,7 +124,7 @@ TxProposal.prototype.getRawTx = function() { * @return {String[]} copayerIds that performed actions in this proposal (accept / reject) */ TxProposal.prototype.getActors = function() { - return _.keys(this.actions); + return _.pluck(this.actions, 'copayerId'); }; @@ -136,7 +135,9 @@ TxProposal.prototype.getActors = function() { * @return {Object} type / createdOn */ TxProposal.prototype.getActionBy = function(copayerId) { - return this.actions[copayerId]; + return _.find(this.actions, { + copayerId: copayerId + }); }; TxProposal.prototype.addAction = function(copayerId, type, comment, signatures, xpub) { @@ -147,7 +148,7 @@ TxProposal.prototype.addAction = function(copayerId, type, comment, signatures, xpub: xpub, comment: comment, }); - this.actions[copayerId] = action; + this.actions.push(action); this._updateStatus(); }; @@ -206,12 +207,12 @@ TxProposal.prototype.isPending = function() { }; TxProposal.prototype.isAccepted = function() { - var votes = _.countBy(_.values(this.actions), 'type'); + var votes = _.countBy(this.actions, 'type'); return votes['accept'] >= this.requiredSignatures; }; TxProposal.prototype.isRejected = function() { - var votes = _.countBy(_.values(this.actions), 'type'); + var votes = _.countBy(this.actions, 'type'); return votes['reject'] >= this.requiredRejections; }; diff --git a/lib/storage.js b/lib/storage.js index f74bd7c..78ae1ce 100644 --- a/lib/storage.js +++ b/lib/storage.js @@ -121,7 +121,7 @@ Storage.prototype._completeTxData = function(walletId, txs, cb) { if (err) return cb(err); _.each(txList, function(tx) { tx.creatorName = wallet.getCopayer(tx.creatorId).name; - _.each(_.values(tx.actions), function(action) { + _.each(tx.actions, function(action) { action.copayerName = wallet.getCopayer(action.copayerId).name; }); }); diff --git a/test/integration/server.js b/test/integration/server.js index cf5e799..fe4097d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1126,10 +1126,9 @@ describe('Copay server', function() { server.getTx({ id: txp.id }, function(err, txp) { - var actions = _.values(txp.actions); - actions.length.should.equal(1); - actions[0].copayerId.should.equal(wallet.copayers[0].id); - actions[0].copayerName.should.equal(wallet.copayers[0].name); + txp.actions.length.should.equal(1); + txp.actions[0].copayerId.should.equal(wallet.copayers[0].id); + txp.actions[0].copayerName.should.equal(wallet.copayers[0].name); done(); }); }); @@ -1221,7 +1220,7 @@ describe('Copay server', function() { should.not.exist(err); txps.length.should.equal(1); var txp = txps[0]; - _.keys(txp.actions).should.be.empty; + txp.actions.should.be.empty; next(null, txp); }); }, @@ -1243,8 +1242,8 @@ describe('Copay server', function() { txp.isPending().should.be.true; txp.isRejected().should.be.false; txp.isAccepted().should.be.false; - _.keys(txp.actions).length.should.equal(1); - var action = txp.actions[wallet.copayers[0].id]; + txp.actions.length.should.equal(1); + var action = txp.getActionBy(wallet.copayers[0].id); action.type.should.equal('accept'); next(null, txp); }); @@ -1279,7 +1278,7 @@ describe('Copay server', function() { txp.isAccepted().should.be.true; txp.isBroadcasted().should.be.true; txp.txid.should.equal('999'); - _.keys(txp.actions).length.should.equal(2); + txp.actions.length.should.equal(2); done(); }); }, @@ -1304,7 +1303,7 @@ describe('Copay server', function() { should.not.exist(err); txps.length.should.equal(1); var txp = txps[0]; - _.keys(txp.actions).should.be.empty; + txp.actions.should.be.empty; next(); }); }, @@ -1325,8 +1324,8 @@ describe('Copay server', function() { txp.isPending().should.be.true; txp.isRejected().should.be.false; txp.isAccepted().should.be.false; - _.keys(txp.actions).length.should.equal(1); - var action = txp.actions[wallet.copayers[0].id]; + txp.actions.length.should.equal(1); + var action = txp.getActionBy(wallet.copayers[0].id); action.type.should.equal('reject'); action.comment.should.equal('just because'); next(); @@ -1359,7 +1358,7 @@ describe('Copay server', function() { txp.isPending().should.be.false; txp.isRejected().should.be.true; txp.isAccepted().should.be.false; - _.keys(txp.actions).length.should.equal(2); + txp.actions.length.should.equal(2); done(); }); }, diff --git a/test/txproposal.js b/test/txproposal.js index abeffb3..08e29f9 100644 --- a/test/txproposal.js +++ b/test/txproposal.js @@ -37,7 +37,7 @@ describe('TXProposal', function() { }); }); - describe.skip('#getRawTx', function() { + describe('#getRawTx', function() { it('should generate correct raw transaction for signed 2-2', function() { var txp = TXP.fromObj(aTXP()); txp.sign('1', theSignatures, theXPub); From c8c185ec2ece1e7774db730b02b5a968f631b41e Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 20 Feb 2015 14:58:34 -0300 Subject: [PATCH 2/4] test rejection comment is not sent in clear text to the server --- test/integration/clientApi.js | 51 +++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/test/integration/clientApi.js b/test/integration/clientApi.js index 411211b..e234a9c 100644 --- a/test/integration/clientApi.js +++ b/test/integration/clientApi.js @@ -447,7 +447,6 @@ describe('client API ', function() { }); }); it('Should keep message and refusal texts', function(done) { - var msg = 'abcdefg'; helpers.createAndJoinWallet(clients, 2, 3, function(err, w) { clients[0].createAddress(function(err, x0) { should.not.exist(err); @@ -455,16 +454,16 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: msg, + message: 'some message', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); - clients[1].rejectTxProposal(x, 'xx', function(err, tx1) { + clients[1].rejectTxProposal(x, 'rejection comment', function(err, tx1) { should.not.exist(err); clients[2].getTxProposals({}, function(err, txs) { should.not.exist(err); - txs[0].decryptedMessage.should.equal(msg); - _.values(txs[0].actions)[0].comment.should.equal('xx'); + txs[0].decryptedMessage.should.equal('some message'); + txs[0].actions[0].comment.should.equal('rejection comment'); done(); }); }); @@ -472,6 +471,48 @@ describe('client API ', function() { }); }); }); + it('Should encrypt proposal message', function(done) { + helpers.createAndJoinWallet(clients, 2, 3, function(err, w) { + clients[0].createAddress(function(err, x0) { + should.not.exist(err); + blockExplorerMock.setUtxo(x0, 10, 2); + var opts = { + amount: 10000, + toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', + message: 'some message', + }; + var spy = sinon.spy(clients[0], '_doPostRequest'); + clients[0].sendTxProposal(opts, function(err, x) { + should.not.exist(err); + spy.calledOnce.should.be.true; + JSON.stringify(spy.getCall(0).args).should.not.contain('some message'); + done(); + }); + }); + }); + }); + it.only('Should encrypt proposal refusal comment', function(done) { + helpers.createAndJoinWallet(clients, 2, 3, function(err, w) { + clients[0].createAddress(function(err, x0) { + should.not.exist(err); + blockExplorerMock.setUtxo(x0, 10, 2); + var opts = { + amount: 10000, + toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', + }; + clients[0].sendTxProposal(opts, function(err, x) { + should.not.exist(err); + var spy = sinon.spy(clients[1], '_doPostRequest'); + clients[1].rejectTxProposal(x, 'rejection comment', function(err, tx1) { + should.not.exist(err); + spy.calledOnce.should.be.true; + JSON.stringify(spy.getCall(0).args).should.not.contain('rejection comment'); + done(); + }); + }); + }); + }); + }); it('should detect fake tx proposals (wrong signature)', function(done) { helpers.createAndJoinWallet(clients, 2, 2, function(err) { should.not.exist(err); From 5a3b754ea15f7f530890b38079beca1e3933a48e Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 20 Feb 2015 15:11:30 -0300 Subject: [PATCH 3/4] encrypt/decrypt rejection comment --- lib/client/api.js | 37 +++++++++++++++++++---------------- test/integration/clientApi.js | 2 +- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/lib/client/api.js b/lib/client/api.js index 1e456e5..fc1ee62 100644 --- a/lib/client/api.js +++ b/lib/client/api.js @@ -17,12 +17,12 @@ var BASE_URL = 'http://localhost:3001/copay/api'; var WALLET_CRITICAL_DATA = ['xPrivKey', 'm', 'publicKeyRing', 'sharedEncryptingKey']; -function _encryptProposalMessage(message, encryptingKey) { +function _encryptMessage(message, encryptingKey) { if (!message) return null; return WalletUtils.encryptMessage(message, encryptingKey); }; -function _decryptProposalMessage(message, encryptingKey) { +function _decryptMessage(message, encryptingKey) { if (!message) return ''; try { return WalletUtils.decryptMessage(message, encryptingKey); @@ -31,6 +31,15 @@ function _decryptProposalMessage(message, encryptingKey) { } }; +function _processTxps(txps, encryptingKey) { + _.each([].concat(txps), function(txp) { + txp.decryptedMessage = _decryptMessage(txp.message, encryptingKey); + _.each(txp.actions, function(action) { + action.comment = _decryptMessage(action.comment, encryptingKey); + }); + }); +}; + function _parseError(body) { if (_.isString(body)) { try { @@ -305,12 +314,9 @@ API.prototype.getStatus = function(cb) { if (err) return cb(err); var url = '/v1/wallets/'; - self._doGetRequest(url, data, function(err, body) { - _.each(body.pendingTxps, function(txp) { - txp.decryptedMessage = _decryptProposalMessage(txp.message, data.sharedEncryptingKey); - }); - - return cb(err, body, data.copayerId); + self._doGetRequest(url, data, function(err, result) { + _processTxps(result.pendingTxps, data.sharedEncryptingKey); + return cb(err, result, data.copayerId); }); }); }; @@ -335,7 +341,7 @@ API.prototype.sendTxProposal = function(opts, cb) { var args = { toAddress: opts.toAddress, amount: opts.amount, - message: _encryptProposalMessage(opts.message, data.sharedEncryptingKey), + message: _encryptMessage(opts.message, data.sharedEncryptingKey), }; var hash = WalletUtils.getProposalHash(args.toAddress, args.amount, args.message); args.proposalSignature = WalletUtils.signMessage(hash, data.signingPrivKey); @@ -431,7 +437,7 @@ API.prototype.import = function(str, cb) { }; /** - * + * * opts.doNotVerify * @return {undefined} */ @@ -444,14 +450,11 @@ API.prototype.getTxProposals = function(opts, cb) { var url = '/v1/txproposals/'; self._doGetRequest(url, data, function(err, txps) { if (err) return cb(err); - var fake = false; - _.each(txps, function(txp) { - txp.decryptedMessage = _decryptProposalMessage(txp.message, data.sharedEncryptingKey); + _processTxps(txps, data.sharedEncryptingKey); - if (!opts.doNotVerify - && !Verifier.checkTxProposal(data, txp)) - fake = true; + var fake = _.any(txps, function(txp) { + return (!opts.doNotVerify && !Verifier.checkTxProposal(data, txp)); }); if (fake) @@ -523,7 +526,7 @@ API.prototype.rejectTxProposal = function(txp, reason, cb) { var url = '/v1/txproposals/' + txp.id + '/rejections/'; var args = { - reason: reason || '', + reason: _encryptMessage(reason, data.sharedEncryptingKey) || '', }; self._doPostRequest(url, args, data, cb); }); diff --git a/test/integration/clientApi.js b/test/integration/clientApi.js index e234a9c..727429e 100644 --- a/test/integration/clientApi.js +++ b/test/integration/clientApi.js @@ -491,7 +491,7 @@ describe('client API ', function() { }); }); }); - it.only('Should encrypt proposal refusal comment', function(done) { + it('Should encrypt proposal refusal comment', function(done) { helpers.createAndJoinWallet(clients, 2, 3, function(err, w) { clients[0].createAddress(function(err, x0) { should.not.exist(err); From 266db38fa4df77f545394345fdd382633deb9565 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Fri, 20 Feb 2015 15:21:19 -0300 Subject: [PATCH 4/4] if decrypted, txp.message contains cleartext and txp.encryptedMessage contains cyphertext --- bit-wallet/bit-status | 2 +- lib/client/api.js | 3 ++- lib/client/verifier.js | 8 ++++---- test/integration/clientApi.js | 26 +++++++++++++------------- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/bit-wallet/bit-status b/bit-wallet/bit-status index 0e0b642..5d15a1f 100755 --- a/bit-wallet/bit-status +++ b/bit-wallet/bit-status @@ -30,7 +30,7 @@ client.getStatus(function(err, res) { if (!_.isEmpty(res.pendingTxps)) { console.log("* TX Proposals:") _.each(res.pendingTxps, function(x) { - console.log("\t%s [%s by %s] %dSAT => %s", utils.shortID(x.id), x.decryptedMessage, x.creatorName, x.amount, x.toAddress); + console.log("\t%s [%s by %s] %dSAT => %s", utils.shortID(x.id), x.message, x.creatorName, x.amount, x.toAddress); if (!_.isEmpty(x.actions)) { console.log('\t\t * Actions'); diff --git a/lib/client/api.js b/lib/client/api.js index fc1ee62..8d73cfa 100644 --- a/lib/client/api.js +++ b/lib/client/api.js @@ -33,7 +33,8 @@ function _decryptMessage(message, encryptingKey) { function _processTxps(txps, encryptingKey) { _.each([].concat(txps), function(txp) { - txp.decryptedMessage = _decryptMessage(txp.message, encryptingKey); + txp.encryptedMessage = txp.message; + txp.message = _decryptMessage(txp.message, encryptingKey); _.each(txp.actions, function(action) { action.comment = _decryptMessage(action.comment, encryptingKey); }); diff --git a/lib/client/verifier.js b/lib/client/verifier.js index cb891b6..2b3bb0b 100644 --- a/lib/client/verifier.js +++ b/lib/client/verifier.js @@ -35,8 +35,8 @@ Verifier.checkCopayers = function(copayers, walletPrivKey, myXPrivKey, n) { } // Not signed pub keys - if (!copayer.xPubKey || !copayer.xPubKeySignature || - !WalletUtils.verifyMessage(copayer.xPubKey, copayer.xPubKeySignature, walletPubKey)) { + if (!copayer.xPubKey || !copayer.xPubKeySignature || + !WalletUtils.verifyMessage(copayer.xPubKey, copayer.xPubKeySignature, walletPubKey)) { log.error('Invalid signatures in server response'); error = true; } @@ -62,9 +62,9 @@ Verifier.checkTxProposal = function(data, txp) { if (!creatorXPubKey) return false; var creatorSigningPubKey = (new Bitcore.HDPublicKey(creatorXPubKey)).derive('m/1/0').publicKey.toString(); - var hash = WalletUtils.getProposalHash(txp.toAddress, txp.amount, txp.message); + var hash = WalletUtils.getProposalHash(txp.toAddress, txp.amount, txp.encryptedMessage || txp.message); log.debug('Regenerating & verifying tx proposal hash -> Hash: ', hash, ' Signature: ', txp.proposalSignature); - if (!WalletUtils.verifyMessage(hash, txp.proposalSignature, creatorSigningPubKey)) + if (!WalletUtils.verifyMessage(hash, txp.proposalSignature, creatorSigningPubKey)) return false; return Verifier.checkAddress(data, txp.changeAddress); diff --git a/test/integration/clientApi.js b/test/integration/clientApi.js index 727429e..7cdd9b7 100644 --- a/test/integration/clientApi.js +++ b/test/integration/clientApi.js @@ -425,7 +425,7 @@ describe('client API ', function() { var opts = { amount: 120000000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola 1-1', + message: 'hello 1-1', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -462,7 +462,7 @@ describe('client API ', function() { should.not.exist(err); clients[2].getTxProposals({}, function(err, txs) { should.not.exist(err); - txs[0].decryptedMessage.should.equal('some message'); + txs[0].message.should.equal('some message'); txs[0].actions[0].comment.should.equal('rejection comment'); done(); }); @@ -523,7 +523,7 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola', + message: 'hello', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -562,7 +562,7 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola', + message: 'hello', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -601,7 +601,7 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola', + message: 'hello', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -643,7 +643,7 @@ describe('client API ', function() { var opts = { amount: 10000000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola 1-1', + message: 'hello 1-1', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -670,7 +670,7 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola 1-1', + message: 'hello 1-1', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -701,14 +701,14 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola 1-1', + message: 'hello 1-1', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); x.status.should.equal('pending'); x.requiredRejections.should.equal(2); x.requiredSignatures.should.equal(2); - clients[0].rejectTxProposal(x, 'no me gusto', function(err, tx) { + clients[0].rejectTxProposal(x, 'wont sign', function(err, tx) { should.not.exist(err, err); tx.status.should.equal('pending'); clients[1].signTxProposal(x, function(err, tx) { @@ -735,7 +735,7 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola 1-1', + message: 'hello 1-1', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err); @@ -743,13 +743,13 @@ describe('client API ', function() { x.requiredRejections.should.equal(2); x.requiredSignatures.should.equal(3); - clients[0].rejectTxProposal(x, 'no me gusto', function(err, tx) { + clients[0].rejectTxProposal(x, 'wont sign', function(err, tx) { should.not.exist(err, err); tx.status.should.equal('pending'); clients[1].signTxProposal(x, function(err, tx) { should.not.exist(err); tx.status.should.equal('pending'); - clients[2].rejectTxProposal(x, 'tampoco me gusto', function(err, tx) { + clients[2].rejectTxProposal(x, 'me neither', function(err, tx) { should.not.exist(err); tx.status.should.equal('rejected'); done(); @@ -770,7 +770,7 @@ describe('client API ', function() { var opts = { amount: 10000, toAddress: 'n2TBMPzPECGUfcT2EByiTJ12TPZkhN2mN5', - message: 'hola 1-1', + message: 'hello 1-1', }; clients[0].sendTxProposal(opts, function(err, x) { should.not.exist(err);