From b69edc44f2eef7dafd813be9582b1f52fb6c24a2 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 26 Jul 2016 11:00:09 -0300 Subject: [PATCH 1/5] test tx creation with foreign id --- test/integration/server.js | 78 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/test/integration/server.js b/test/integration/server.js index 4026505..387783d 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -2982,6 +2982,84 @@ describe('Wallet service', function() { }); }); }); + it('should create a tx with foreign ID', function(done) { + helpers.stubUtxos(server, wallet, 2, function() { + var txOpts = { + txProposalId: '123', + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 1e8, + }], + feePerKb: 100e2, + }; + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.id.should.equal('123'); + done(); + }); + }); + }); + it('should return already created tx if same foreign ID is specified and tx still unpublished', function(done) { + helpers.stubUtxos(server, wallet, 2, function() { + var txOpts = { + txProposalId: '123', + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 1e8, + }], + feePerKb: 100e2, + }; + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.id.should.equal('123'); + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.id.should.equal('123'); + server.storage.fetchTxs(wallet.id, {}, function(err, txs) { + should.not.exist(err); + should.exist(txs); + txs.length.should.equal(1); + done(); + }); + }); + }); + }); + }); + it('should fail to create tx if same foreign ID is specified and tx already published', function(done) { + helpers.stubUtxos(server, wallet, [2, 2, 2], function() { + var txOpts = { + txProposalId: '123', + outputs: [{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 1e8, + }], + feePerKb: 100e2, + }; + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.id.should.equal('123'); + var publishOpts = helpers.getProposalSignatureOpts(tx, TestData.copayers[0].privKey_1H_0); + server.publishTx(publishOpts, function(err) { + should.not.exist(err); + server.createTx(txOpts, function(err, tx) { + should.exist(err); + should.not.exist(tx); + err.code.should.equal('TX_ALREADY_EXISTS'); + server.storage.fetchTxs(wallet.id, {}, function(err, txs) { + should.not.exist(err); + should.exist(txs); + txs.length.should.equal(1); + done(); + }); + }); + }); + }); + }); + }); it('should be able to publish a temporary tx proposal', function(done) { helpers.stubUtxos(server, wallet, [1, 2], function() { var txOpts = { From 49929942e7bee2ae21d0139c97a4368b7507fc1c Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 26 Jul 2016 11:00:52 -0300 Subject: [PATCH 2/5] accept id as arg for tx creation --- lib/model/txproposal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/model/txproposal.js b/lib/model/txproposal.js index dd0b71d..4998ed9 100644 --- a/lib/model/txproposal.js +++ b/lib/model/txproposal.js @@ -27,7 +27,7 @@ TxProposal.create = function(opts) { var now = Date.now(); x.createdOn = Math.floor(now / 1000); - x.id = _.padLeft(now, 14, '0') + Uuid.v4(); + x.id = opts.id || Uuid.v4(); x.walletId = opts.walletId; x.creatorId = opts.creatorId; x.message = opts.message; From 04fdc090bd8f1d86b5e5e4316d54ffaac97bc70d Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 26 Jul 2016 11:01:14 -0300 Subject: [PATCH 3/5] check tx already exists based on foreign id --- lib/errors/errordefinitions.js | 1 + lib/server.js | 148 ++++++++++++++++++--------------- 2 files changed, 84 insertions(+), 65 deletions(-) diff --git a/lib/errors/errordefinitions.js b/lib/errors/errordefinitions.js index f5c8c2f..3a09d69 100644 --- a/lib/errors/errordefinitions.js +++ b/lib/errors/errordefinitions.js @@ -23,6 +23,7 @@ var errors = { NOT_AUTHORIZED: 'Not authorized', TOO_MANY_KEYS: 'Too many keys registered', TX_ALREADY_BROADCASTED: 'The transaction proposal is already broadcasted', + TX_ALREADY_EXISTS: 'A transaction proposal with the same id already exists', TX_CANNOT_CREATE: 'Cannot create TX proposal during backoff time', TX_CANNOT_REMOVE: 'Cannot remove this tx proposal during locktime', TX_MAX_SIZE_EXCEEDED: 'TX exceeds maximum allowed size', diff --git a/lib/server.js b/lib/server.js index 005e24b..93fb79d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1934,6 +1934,7 @@ WalletService.prototype._validateAndSanitizeTxOpts = function(wallet, opts, cb) /** * Creates a new transaction proposal. * @param {Object} opts + * @param {string} opts.txProposalId - Optional. If provided it will be used as this TX proposal ID. Should be unique in the scope of the wallet. * @param {Array} opts.outputs - List of outputs. * @param {string} opts.outputs[].toAddress - Destination address. * @param {number} opts.outputs[].amount - Amount to transfer in satoshi. @@ -1973,73 +1974,90 @@ WalletService.prototype.createTx = function(opts, cb) { } }; + function checkTxpAlreadyExists(txProposalId, cb) { + if (!txProposalId) return cb(); + + self.storage.fetchTx(self.walletId, txProposalId, function(err, txp) { + if (err || !txp) return cb(err); + if (txp.status == 'temporary') { + return cb(null, txp); + } else { + return cb(Errors.TX_ALREADY_EXISTS); + } + }); + }; + self._runLocked(cb, function(cb) { - var wallet, txp, changeAddress; - async.series([ - - function(next) { - self.getWallet({}, function(err, w) { - if (err) return next(err); - if (!w.isComplete()) return next(Errors.WALLET_NOT_COMPLETE); - wallet = w; - next(); - }); - }, - function(next) { - self._validateAndSanitizeTxOpts(wallet, opts, next); - }, - function(next) { - self._canCreateTx(function(err, canCreate) { - if (err) return next(err); - if (!canCreate) return next(Errors.TX_CANNOT_CREATE); - next(); - }); - }, - function(next) { - if (opts.sendMax) return next(); - getChangeAddress(wallet, function(err, address) { - if (err) return next(err); - changeAddress = address; - next(); - }); - }, - function(next) { - var txOpts = { - walletId: self.walletId, - creatorId: self.copayerId, - outputs: opts.outputs, - message: opts.message, - changeAddress: changeAddress, - feePerKb: opts.feePerKb, - payProUrl: opts.payProUrl, - walletM: wallet.m, - walletN: wallet.n, - excludeUnconfirmedUtxos: !!opts.excludeUnconfirmedUtxos, - validateOutputs: !opts.validateOutputs, - addressType: wallet.addressType, - customData: opts.customData, - inputs: opts.inputs, - fee: opts.inputs && !_.isNumber(opts.feePerKb) ? opts.fee : null, - noShuffleOutputs: opts.noShuffleOutputs - }; - - txp = Model.TxProposal.create(txOpts); - next(); - }, - function(next) { - self._selectTxInputs(txp, opts.utxosToExclude, next); - }, - function(next) { - if (!changeAddress || opts.dryRun) return next(); - self.storage.storeAddressAndWallet(wallet, txp.changeAddress, next); - }, - function(next) { - if (opts.dryRun) return next(); - self.storage.storeTx(wallet.id, txp, next); - }, - ], function(err) { + var txp, changeAddress; + self.getWallet({}, function(err, wallet) { if (err) return cb(err); - return cb(null, txp); + if (!wallet.isComplete()) return cb(Errors.WALLET_NOT_COMPLETE); + + checkTxpAlreadyExists(opts.txProposalId, function(err, txp) { + if (err) return cb(err); + if (txp) return cb(null, txp); + + async.series([ + + function(next) { + self._validateAndSanitizeTxOpts(wallet, opts, next); + }, + function(next) { + self._canCreateTx(function(err, canCreate) { + if (err) return next(err); + if (!canCreate) return next(Errors.TX_CANNOT_CREATE); + next(); + }); + }, + function(next) { + if (opts.sendMax) return next(); + getChangeAddress(wallet, function(err, address) { + if (err) return next(err); + changeAddress = address; + next(); + }); + }, + function(next) { + var txOpts = { + id: opts.txProposalId, + walletId: self.walletId, + creatorId: self.copayerId, + outputs: opts.outputs, + message: opts.message, + changeAddress: changeAddress, + feePerKb: opts.feePerKb, + payProUrl: opts.payProUrl, + walletM: wallet.m, + walletN: wallet.n, + excludeUnconfirmedUtxos: !!opts.excludeUnconfirmedUtxos, + validateOutputs: !opts.validateOutputs, + addressType: wallet.addressType, + customData: opts.customData, + inputs: opts.inputs, + fee: opts.inputs && !_.isNumber(opts.feePerKb) ? opts.fee : null, + noShuffleOutputs: opts.noShuffleOutputs + }; + + txp = Model.TxProposal.create(txOpts); + next(); + }, + function(next) { + self._selectTxInputs(txp, opts.utxosToExclude, next); + }, + function(next) { + if (!changeAddress || opts.dryRun) return next(); + self.storage.storeAddressAndWallet(wallet, txp.changeAddress, next); + }, + function(next) { + if (opts.dryRun) return next(); + self.storage.storeTx(wallet.id, txp, next); + }, + ], function(err) { + if (err && err !== 'break') return cb(err); + return cb(null, txp); + }); + + }); }); }); }; From d47bf73dd805ffded1ec285900e87f527c28ce85 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 26 Jul 2016 11:03:29 -0300 Subject: [PATCH 4/5] test allow duplicate tx if no id is specified --- test/integration/server.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/integration/server.js b/test/integration/server.js index 387783d..6a568eb 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -3053,7 +3053,17 @@ describe('Wallet service', function() { should.not.exist(err); should.exist(txs); txs.length.should.equal(1); - done(); + txOpts.txProposalId = null; + server.createTx(txOpts, function(err, tx) { + should.not.exist(err); + should.exist(tx); + tx.id.should.not.equal('123'); + server.storage.fetchTxs(wallet.id, {}, function(err, txs) { + should.not.exist(err); + txs.length.should.equal(2); + done(); + }); + }); }); }); }); From b3578ccfb571108c5e94d851046ba313efefacc3 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Tue, 26 Jul 2016 11:09:17 -0300 Subject: [PATCH 5/5] rm dead code --- lib/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index 93fb79d..7f674fa 100644 --- a/lib/server.js +++ b/lib/server.js @@ -2053,7 +2053,7 @@ WalletService.prototype.createTx = function(opts, cb) { self.storage.storeTx(wallet.id, txp, next); }, ], function(err) { - if (err && err !== 'break') return cb(err); + if (err) return cb(err); return cb(null, txp); });