Skip to content

Commit

Permalink
chore: address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
katelynsills committed Nov 3, 2020
1 parent 53d5cb7 commit 8b57c4b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 23 deletions.
38 changes: 38 additions & 0 deletions packages/ERTP/src/displayInfo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { assert, details, q } from '@agoric/assert';

/**
* Assert all values from `part` appear in `whole`.
*
* @param {string[]} whole
* @param {string[]} part
*/
export const assertSubset = (whole, part) => {
part.forEach(key => {
assert.typeof(key, 'string');
assert(
whole.includes(key),
details`key ${q(key)} was not one of the expected keys ${q(whole)}`,
);
});
};

// Assert that the keys of `record` are all in `allowedKeys`. If a key
// of `record` is not in `allowedKeys`, throw an error. If a key in
// `allowedKeys` is not a key of record, we do not throw an error.
export const assertKeysAllowed = (allowedKeys, record) => {
const keys = Object.getOwnPropertyNames(record);
assertSubset(allowedKeys, keys);
// assert that there are no symbol properties.
assert(
Object.getOwnPropertySymbols(record).length === 0,
details`no symbol properties allowed`,
);
};

export const assertDisplayInfo = allegedDisplayInfo => {
if (allegedDisplayInfo === undefined) {
return;
}
const displayInfoKeys = harden(['decimalPlaces']);
assertKeysAllowed(displayInfoKeys, allegedDisplayInfo);
};
13 changes: 8 additions & 5 deletions packages/ERTP/src/issuer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,20 @@ import { isPromise } from '@agoric/promise-kit';

import { makeAmountMath, MathKind } from './amountMath';
import { makeInterface, ERTPKind } from './interfaces';
import { assertDisplayInfo } from './displayInfo';

import './types';

/**
* @type {MakeIssuerKit}
*/
function makeIssuerKit(allegedName, amountMathKind = MathKind.NAT, decimals) {
function makeIssuerKit(
allegedName,
amountMathKind = MathKind.NAT,
displayInfo = undefined,
) {
assert.typeof(allegedName, 'string');
assertDisplayInfo(displayInfo);

const brand = Remotable(
makeInterface(allegedName, ERTPKind.BRAND),
Expand All @@ -32,10 +38,7 @@ function makeIssuerKit(allegedName, amountMathKind = MathKind.NAT, decimals) {
getAllegedName: () => allegedName,

// Give information to UI on how to display the amount.
// Fungible digital assets should be represented in integers, in
// the smallest unit (i.e. USD might be represented in mill,
// a thousandth of a dollar. In that case, `decimals` would be 3).
decimals: () => decimals,
getDisplayInfo: () => displayInfo,
},
);

Expand Down
27 changes: 16 additions & 11 deletions packages/ERTP/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@
* to set subtraction.
*/

/**
* @typedef {Object} DisplayInfo
* @property {number} decimalPlaces
* Tells the display software how many decimal places to move the
* decimal over to the left, or in other words, which position corresponds to whole
* numbers. We require fungible digital assets to be represented in
* integers, in the smallest unit (i.e. USD might be represented in mill,
* a thousandth of a dollar. In that case, `decimalPlaces` would be 3.)
* For non-fungible digital assets, this should be left as undefined.
* The decimalPlaces property should be used for *display purposes only*. Any
* other use is an anti-pattern.
*/

/**
* @typedef {Object} Brand
* The brand identifies the kind of issuer, and has a function to get the
Expand All @@ -112,12 +125,8 @@
* @property {(allegedIssuer: ERef<Issuer>) => Promise<boolean>} isMyIssuer Should be used with
* `issuer.getBrand` to ensure an issuer and brand match.
* @property {() => string} getAllegedName
* @property {() => number} decimals
* @property {() => DisplayInfo} getDisplayInfo
* Give information to UI on how to display the amount.
* Fungible digital assets should be represented in integers, in
* the smallest unit (i.e. USD might be represented in mill,
* a thousandth of a dollar. In that case, `decimals` would be 3).
* Should not be used for nonfungible digital assets.
*/

/**
Expand Down Expand Up @@ -195,7 +204,7 @@
* @callback MakeIssuerKit
* @param {string} allegedName
* @param {AmountMathKind} [amountMathKind=MathKind.NAT]
* @param {number} [decimals]
* @param {DisplayInfo=} [displayInfo=undefined]
* @returns {IssuerKit}
*
* The allegedName becomes part of the brand in asset descriptions. The
Expand All @@ -207,11 +216,7 @@
* from the mathHelpers library. For example, natMathHelpers, the
* default, is used for basic fungible tokens.
*
* `decimals` gives information to UI on how to display the amount.
* Fungible digital assets should be represented in integers, in
* the smallest unit (i.e. USD might be represented in mill,
* a thousandth of a dollar. In that case, `decimals` would be 3).
* `decimals` should not be used for nonfungible digital assets.
* `displayInfo` gives information to UI on how to display the amount.
*
* @typedef {Object} IssuerKit
* The return value of makeIssuerKit
Expand Down
23 changes: 16 additions & 7 deletions packages/ERTP/test/unitTests/test-issuerObj.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,35 @@ test('issuer.getBrand, brand.isMyIssuer', t => {
);
t.is(issuer.getAllegedName(), myBrand.getAllegedName());
t.is(issuer.getAllegedName(), 'fungible');
t.is(brand.decimals(), undefined);
t.is(brand.getDisplayInfo(), undefined);
});

test('brand decimals', t => {
const { brand } = makeIssuerKit('fungible', MathKind.NAT, 3);
t.is(brand.decimals(), 3);
test('brand.getDisplayInfo()', t => {
const displayInfo = harden({ decimalPlaces: 3 });
const { brand } = makeIssuerKit('fungible', MathKind.NAT, displayInfo);
t.is(brand.getDisplayInfo(), displayInfo);
const display = amount => {
const { brand: myBrand, value } = amount;
const decimals = myBrand.decimals();
const { decimalPlaces } = myBrand.getDisplayInfo();
const valueDisplay = value.toString();
const length = valueDisplay.length;
return [
valueDisplay.slice(0, length - decimals),
valueDisplay.slice(0, length - decimalPlaces),
'.',
valueDisplay.slice(length - decimals),
valueDisplay.slice(length - decimalPlaces),
].join('');
};
t.is(display({ brand, value: 3000 }), '3.000');
});

test('bad display info', t => {
const displayInfo = harden({ somethingUnexpected: 3 });
t.throws(() => makeIssuerKit('fungible', MathKind.NAT, displayInfo), {
message:
'key "somethingUnexpected" was not one of the expected keys ["decimalPlaces"]',
});
});

test('amountMath from makeIssuerKit', async t => {
const { issuer, amountMath, brand } = makeIssuerKit('fungible');
const ibrand = await E(issuer).getBrand();
Expand Down

0 comments on commit 8b57c4b

Please sign in to comment.