Skip to content

Commit

Permalink
Merge pull request #1248 from braydonf/bug/misleading-error
Browse files Browse the repository at this point in the history 
Fixes bug with misleading error with getSerializationError. Closes #1236
  • Loading branch information
@pnagurny
pnagurny committed May 26, 2015
2 parents 7cf97ac + ee651df commit 4c1ba67
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 86 deletions.
84 changes: 36 additions & 48 deletions lib/transaction/transaction.js
View file
Expand Up @@ -191,57 +191,54 @@ Transaction.prototype.invalidSatoshis = function() {
Transaction.prototype.getSerializationError = function(opts) {
opts = opts || {};

return this._isInvalidSatoshis() ||
this._hasFeeError(opts) ||
this._hasDustOutputs(opts) ||
this._isMissingSignatures(opts) ||
this._hasMoreOutputThanInput(opts);
};

Transaction.prototype._isInvalidSatoshis = function() {
if (this.invalidSatoshis()) {
return new errors.Transaction.InvalidSatoshis();
}
};

Transaction.prototype._hasFeeError = function(opts) {
return this._isFeeDifferent() ||
this._isFeeTooLarge(opts) ||
this._isFeeTooSmall(opts);
};

Transaction.prototype._isFeeDifferent = function() {
if (!_.isUndefined(this._fee)) {
var fee = this._fee;
var unspent = this._getUnspentValue();
if (fee !== unspent) {
return new errors.Transaction.FeeError.Different('Unspent value is ' + unspent + ' but specified fee is ' + fee);
var unspent = this._getUnspentValue();
var unspentError;
if (unspent < 0) {
if (!opts.disableMoreOutputThanInput) {
unspentError = new errors.Transaction.InvalidOutputAmountSum();
}
} else {
unspentError = this._hasFeeError(opts, unspent);
}

return unspentError ||
this._hasDustOutputs(opts) ||
this._isMissingSignatures(opts);
};

Transaction.prototype._isFeeTooLarge = function(opts) {
if (opts.disableLargeFees) {
return;
Transaction.prototype._hasFeeError = function(opts, unspent) {

if (!_.isUndefined(this._fee) && this._fee !== unspent) {
return new errors.Transaction.FeeError.Different(
'Unspent value is ' + unspent + ' but specified fee is ' + this._fee
);
}
var fee = this._getUnspentValue();
var maximumFee = Math.floor(Transaction.FEE_SECURITY_MARGIN * this._estimateFee());
if (fee > maximumFee) {
if (this._missingChange()) {
return new errors.Transaction.ChangeAddressMissing('Fee is too large and no change address was provided');

if (!opts.disableLargeFees) {
var maximumFee = Math.floor(Transaction.FEE_SECURITY_MARGIN * this._estimateFee());
if (unspent > maximumFee) {
if (this._missingChange()) {
return new errors.Transaction.ChangeAddressMissing(
'Fee is too large and no change address was provided'
);
}
return new errors.Transaction.FeeError.TooLarge(
'expected less than ' + maximumFee + ' but got ' + unspent
);
}
return new errors.Transaction.FeeError.TooLarge('expected less than ' + maximumFee + ' but got ' + fee);
}
};

Transaction.prototype._isFeeTooSmall = function(opts) {
if (opts.disableSmallFees) {
return;
}
var fee = this._getUnspentValue();
var minimumFee = Math.ceil(this._estimateFee() / Transaction.FEE_SECURITY_MARGIN);
if (fee < minimumFee) {
return new errors.Transaction.FeeError.TooSmall('expected more than ' + minimumFee + ' but got ' + fee);
if (!opts.disableSmallFees) {
var minimumFee = Math.ceil(this._estimateFee() / Transaction.FEE_SECURITY_MARGIN);
if (unspent < minimumFee) {
return new errors.Transaction.FeeError.TooSmall(
'expected more than ' + minimumFee + ' but got ' + unspent
);
}
}
};

Expand Down Expand Up @@ -271,15 +268,6 @@ Transaction.prototype._isMissingSignatures = function(opts) {
}
};

Transaction.prototype._hasMoreOutputThanInput = function(opts) {
if (opts.disableMoreOutputThanInput) {
return;
}
if (this._getUnspentValue() < 0) {
return new errors.Transaction.InvalidOutputAmountSum();
}
};

Transaction.prototype.inspect = function() {
return '<Transaction: ' + this.uncheckedSerialize() + '>';
};
Expand Down
102 changes: 64 additions & 38 deletions test/transaction/transaction.js
View file
Expand Up @@ -385,8 +385,32 @@ describe('Transaction', function() {
return transaction.serialize();
}).to.throw(errors.Transaction.FeeError.Different);
});
it('checks output amount before fee errors', function() {
var transaction = new Transaction();
transaction.from(simpleUtxoWith1BTC);
transaction
.to(toAddress, 10000000000000)
.change(changeAddress)
.fee(5);

expect(function() {
return transaction.serialize();
}).to.throw(errors.Transaction.InvalidOutputAmountSum);
});
it('will throw fee error with disableMoreOutputThanInput enabled (but not triggered)', function() {
var transaction = new Transaction();
transaction.from(simpleUtxoWith1BTC);
transaction
.to(toAddress, 90000000)
.change(changeAddress)
.fee(10000000);

expect(function() {
return transaction.serialize({disableMoreOutputThanInput: true});
}).to.throw(errors.Transaction.FeeError.TooLarge);
});
describe('skipping checks', function() {
var buildSkipTest = function(builder, check) {
var buildSkipTest = function(builder, check, expectedError) {
return function() {
var transaction = new Transaction();
transaction.from(simpleUtxoWith1BTC);
Expand All @@ -400,7 +424,7 @@ describe('Transaction', function() {
}).not.to.throw();
expect(function() {
return transaction.serialize();
}).to.throw();
}).to.throw(expectedError);
};
};
it('can skip the check for too much fee', buildSkipTest(
Expand All @@ -409,54 +433,39 @@ describe('Transaction', function() {
.fee(50000000)
.change(changeAddress)
.sign(privateKey);
}, 'disableLargeFees'));
}, 'disableLargeFees', errors.Transaction.FeeError.TooLarge
));
it('can skip the check for a fee that is too small', buildSkipTest(
function(transaction) {
return transaction
.fee(1)
.change(changeAddress)
.sign(privateKey);
}, 'disableSmallFees'));
}, 'disableSmallFees', errors.Transaction.FeeError.TooSmall
));
it('can skip the check that prevents dust outputs', buildSkipTest(
function(transaction) {
return transaction
.to(toAddress, 100)
.change(changeAddress)
.sign(privateKey);
}, 'disableDustOutputs'));
it('can skip the check that prevents unsigned outputs', function() {
var transaction = new Transaction();
transaction.from(simpleUtxoWith1BTC);
transaction.to(toAddress, 10000);
transaction.change(changeAddress);
var options = {};
options.disableIsFullySigned = true;
expect(function() {
return transaction.serialize(options);
}).not.to.throw(errors.Transaction.MissingSignatures);
expect(function() {
return transaction.serialize();
}).to.throw(errors.Transaction.MissingSignatures);
});
it('can skip the check that avoids spending more bitcoins than the inputs for a transaction', function() {
var transaction = new Transaction();
transaction.from(simpleUtxoWith1BTC);
transaction.to(toAddress, 10000000000000);
transaction.change(changeAddress);
expect(function() {
return transaction.serialize({
disableSmallFees: true,
disableIsFullySigned: true,
disableMoreOutputThanInput: true
});
}).not.to.throw(errors.Transaction.InvalidOutputAmountSum);
expect(function() {
return transaction.serialize({
disableIsFullySigned: true,
disableSmallFees: true
});
}).to.throw(errors.Transaction.InvalidOutputAmountSum);
});
}, 'disableDustOutputs', errors.Transaction.DustOutputs
));
it('can skip the check that prevents unsigned outputs', buildSkipTest(
function(transaction) {
return transaction
.to(toAddress, 10000)
.change(changeAddress);
}, 'disableIsFullySigned', errors.Transaction.MissingSignatures
));
it('can skip the check that avoids spending more bitcoins than the inputs for a transaction', buildSkipTest(
function(transaction) {
return transaction
.to(toAddress, 10000000000000)
.change(changeAddress)
.sign(privateKey);
}, 'disableMoreOutputThanInput', errors.Transaction.InvalidOutputAmountSum
));
});
});

Expand Down Expand Up @@ -511,6 +520,23 @@ describe('Transaction', function() {

});

it('not if has null input (and not coinbase)', function() {

var tx = new Transaction()
.from({
'txId': testPrevTx,
'outputIndex': 0,
'script': testScript,
'satoshis': testAmount
}).to('mrU9pEmAx26HcbKVrABvgL7AwA5fjNFoDc', testAmount - 10000);

tx.isCoinbase = sinon.stub().returns(false);
tx.inputs[0].isNull = sinon.stub().returns(true);
var verify = tx.verify();
verify.should.equal('transaction input 0 has null input');

});

});

describe('to and from JSON', function() {
Expand Down

0 comments on commit 4c1ba67

Please sign in to comment.