Enforce proper bounds of Amounts

This commit is contained in:
Nicholas Dudfield
2016-11-15 18:23:47 +10:00
parent f9ee5aa029
commit a1a938e895
8 changed files with 103 additions and 14 deletions

View File

@@ -40,7 +40,7 @@
"prepublish": "npm test && npm run lint && npm run compile", "prepublish": "npm test && npm run lint && npm run compile",
"test": "istanbul test _mocha", "test": "istanbul test _mocha",
"codecov": "cat ./coverage/coverage.json | ./node_modules/codecov.io/bin/codecov.io.js", "codecov": "cat ./coverage/coverage.json | ./node_modules/codecov.io/bin/codecov.io.js",
"lint": "if ! [ -f eslintrc ]; then curl -o eslintrc 'https://raw.githubusercontent.com/ripple/javascript-style-guide/es6/eslintrc'; echo 'parser: babel-eslint' >> eslintrc; fi; eslint -c eslintrc src/*.js test/*.js examples/*.js" "lint": "if ! [ -f eslintrc ]; then curl -o eslintrc 'https://raw.githubusercontent.com/ripple/javascript-style-guide/es6/eslintrc'; echo 'parser: babel-eslint' >> eslintrc; fi; eslint -c eslintrc src/**/*.js test/*.js examples/*.js"
}, },
"repository": { "repository": {
"type": "git", "type": "git",

View File

@@ -9,18 +9,58 @@ const {Currency} = require('./currency');
const {AccountID} = require('./account-id'); const {AccountID} = require('./account-id');
const {UInt64} = require('./uint-64'); const {UInt64} = require('./uint-64');
const MIN_IOU_EXPONENT = -96;
const MAX_IOU_EXPONENT = 80;
const MAX_IOU_PRECISION = 16;
const MIN_IOU_MANTISSA = '1000' + '0000' + '0000' + '0000'; // 16 digits
const MAX_IOU_MANTISSA = '9999' + '9999' + '9999' + '9999'; // ..
const MAX_IOU = new Decimal(`${MAX_IOU_MANTISSA}e${MAX_IOU_EXPONENT}`);
const MIN_IOU = new Decimal(`${MIN_IOU_MANTISSA}e${MIN_IOU_EXPONENT}`);
const DROPS_PER_XRP = new Decimal('1e6');
const MAX_NETWORK_DROPS = new Decimal('1e17');
const MIN_XRP = new Decimal('1e-6')
const MAX_XRP = MAX_NETWORK_DROPS.dividedBy(DROPS_PER_XRP);
// Never use exponential form
Decimal.config({ Decimal.config({
toExpPos: 32, toExpPos: MAX_IOU_EXPONENT + MAX_IOU_PRECISION,
toExpNeg: -32 toExpNeg: MIN_IOU_EXPONENT - MAX_IOU_PRECISION
}); });
const AMOUNT_PARAMETERS_DESCRIPTION = `
Native values must be described in drops, a million of which equal one XRP.
This must be an integer number, with the absolute value not exceeding \
${MAX_NETWORK_DROPS}
IOU values must have a maximum precision of ${MAX_IOU_PRECISION} significant \
digits. They are serialized as\na canonicalised mantissa and exponent.
The valid range for a mantissa is between ${MIN_IOU_MANTISSA} and \
${MAX_IOU_MANTISSA}
The exponent must be >= ${MIN_IOU_EXPONENT} and <= ${MAX_IOU_EXPONENT}
Thus the largest serializable IOU value is:
${MAX_IOU.toString()}
And the smallest:
${MIN_IOU.toString()}
`
function isDefined(val) { function isDefined(val) {
return !_.isUndefined(val); return !_.isUndefined(val);
} }
function raiseIllegalAmountError(value) {
throw new Error(`${value.toString()} is an illegal amount\n` +
AMOUNT_PARAMETERS_DESCRIPTION);
}
const parsers = { const parsers = {
string(str) { string(str) {
return [new Decimal(str).dividedBy('1e6'), Currency.XRP]; if (!str.match(/\d+/)) {
raiseIllegalAmountError(str);
}
return [new Decimal(str).dividedBy(DROPS_PER_XRP), Currency.XRP];
}, },
object(object) { object(object) {
assert(isDefined(object.currency), 'currency must be defined'); assert(isDefined(object.currency), 'currency must be defined');
@@ -36,6 +76,7 @@ const Amount = makeClass({
this.value = value || new Decimal('0'); this.value = value || new Decimal('0');
this.currency = currency || Currency.XRP; this.currency = currency || Currency.XRP;
this.issuer = issuer || null; this.issuer = issuer || null;
this.assertValueIsValid();
}, },
mixins: SerializedType, mixins: SerializedType,
statics: { statics: {
@@ -72,10 +113,30 @@ const Amount = makeClass({
mantissa[0] &= 0x3F; mantissa[0] &= 0x3F;
const drops = new Decimal(`${sign}0x${bytesToHex(mantissa)}`); const drops = new Decimal(`${sign}0x${bytesToHex(mantissa)}`);
const xrpValue = drops.dividedBy('1e6'); const xrpValue = drops.dividedBy(DROPS_PER_XRP);
return new this(xrpValue, Currency.XRP); return new this(xrpValue, Currency.XRP);
} }
}, },
assertValueIsValid() {
// zero is always a valid amount value
if (!this.isZero()) {
if (this.isNative()) {
const abs = this.value.abs();
if (abs.lt(MIN_XRP) || abs.gt(MAX_XRP)) {
// value is in XRP scale, but show the value in canonical json form
raiseIllegalAmountError(this.value.times(DROPS_PER_XRP))
}
} else {
const p = this.value.precision();
const e = this.exponent();
if (p > MAX_IOU_PRECISION ||
e > MAX_IOU_EXPONENT ||
e < MIN_IOU_EXPONENT) {
raiseIllegalAmountError(this.value)
}
}
}
},
isNative() { isNative() {
return this.currency.isNative(); return this.currency.isNative();
}, },
@@ -90,7 +151,7 @@ const Amount = makeClass({
return this.isNative() ? -6 : this.value.e - 15; return this.isNative() ? -6 : this.value.e - 15;
}, },
valueString() { valueString() {
return (this.isNative() ? this.value.times('1e6') : this.value) return (this.isNative() ? this.value.times(DROPS_PER_XRP) : this.value)
.toString(); .toString();
}, },
toBytesSink(sink) { toBytesSink(sink) {

View File

@@ -1,4 +1,3 @@
'use strict';
/* eslint-disable no-unused-expressions */ /* eslint-disable no-unused-expressions */
const makeClass = require('../utils/make-class'); const makeClass = require('../utils/make-class');
@@ -94,11 +93,11 @@ const PathSet = makeClass({
}, },
toBytesSink(sink) { toBytesSink(sink) {
let n = 0; let n = 0;
this.forEach((path) => { this.forEach(path => {
if (n++ !== 0) { if (n++ !== 0) {
sink.put([PATH_SEPARATOR_BYTE]); sink.put([PATH_SEPARATOR_BYTE]);
} }
path.forEach((hop) => { path.forEach(hop => {
sink.put([hop.type()]); sink.put([hop.type()]);
hop.account && (hop.account.toBytesSink(sink)); hop.account && (hop.account.toBytesSink(sink));
hop.currency && (hop.currency.toBytesSink(sink)); hop.currency && (hop.currency.toBytesSink(sink));

View File

@@ -26,7 +26,7 @@ const STArray = makeClass({
} }
}, },
toJSON() { toJSON() {
return this.map((v) => v.toJSON()); return this.map(v => v.toJSON());
}, },
toBytesSink(sink) { toBytesSink(sink) {
this.forEach(so => so.toBytesSink(sink)); this.forEach(so => so.toBytesSink(sink));

View File

@@ -1,4 +1,3 @@
const assert = require('assert');
const _ = require('lodash'); const _ = require('lodash');
const makeClass = require('../utils/make-class'); const makeClass = require('../utils/make-class');
const {Field} = require('../enums'); const {Field} = require('../enums');
@@ -39,7 +38,7 @@ const STObject = makeClass({
} }
}, },
fieldKeys() { fieldKeys() {
return Object.keys(this).map((k) => Field[k]).filter(Boolean); return Object.keys(this).map(k => Field[k]).filter(Boolean);
}, },
toJSON() { toJSON() {
// Otherwise seemingly result will have same prototype as `this` // Otherwise seemingly result will have same prototype as `this`
@@ -52,7 +51,7 @@ const STObject = makeClass({
const serializer = new BinarySerializer(sink); const serializer = new BinarySerializer(sink);
const fields = this.fieldKeys(); const fields = this.fieldKeys();
const sorted = _.sortBy(fields, 'ordinal'); const sorted = _.sortBy(fields, 'ordinal');
sorted.filter(filter).forEach((field) => { sorted.filter(filter).forEach(field => {
const value = this[field]; const value = this[field];
if (!field.isSerialized) { if (!field.isSerialized) {
return; return;

View File

@@ -23,7 +23,7 @@ const Vector256 = makeClass({
this.forEach(h => h.toBytesSink(sink)); this.forEach(h => h.toBytesSink(sink));
}, },
toJSON() { toJSON() {
return this.map((hash) => hash.toJSON()); return this.map(hash => hash.toJSON());
} }
}); });

View File

@@ -1,5 +1,25 @@
const _ = require('lodash');
const assert = require('assert-diff'); const assert = require('assert-diff');
const utils = require('./utils');
const {Amount} = require('../src/coretypes'); const {Amount} = require('../src/coretypes');
const {loadFixture} = utils;
const fixtures = loadFixture('data-driven-tests.json');
function amountErrorTests() {
_.filter(fixtures.values_tests, {type: 'Amount'}).forEach(f => {
// We only want these with errors
if (!f.error) {
return
}
const testName = `${JSON.stringify(f.test_json)}\n\tis invalid ` +
`because: ${f.error}`
it(testName, () => {
assert.throws(() => {
Amount.from(f.test_json);
}, JSON.stringify(f.test_json));
});
});
}
describe('Amount', function() { describe('Amount', function() {
it('can be parsed from', function() { it('can be parsed from', function() {
@@ -18,5 +38,6 @@ describe('Amount', function() {
}; };
assert.deepEqual(amt.toJSON(), rewritten); assert.deepEqual(amt.toJSON(), rewritten);
}); });
amountErrorTests()
}); });

View File

@@ -2875,6 +2875,15 @@
"error": "value precision of 17 is greater than maximum iou precision of 16", "error": "value precision of 17 is greater than maximum iou precision of 16",
"is_negative": false "is_negative": false
}, },
{
"test_json": {
"currency": "USD",
"value": "9999999999999999000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"issuer": "rrrrrrrrrrrrrrrrrrrrBZbvji"
},
"type": "Amount",
"error": "exponent is too large"
},
{ {
"test_json": { "test_json": {
"currency": "USD", "currency": "USD",