Merge pull request #1964 from Agoric/119-decimal

feat: add a decimals parameter, and decimals method to brand

Author: michaelfig

packages/ERTP/src/displayInfo.js

@@ -0,0 +1,69 @@
+import { assert, details, q } from '@agoric/assert';
+import {
+  pureCopy,
+  passStyleOf,
+  REMOTE_STYLE,
+  getInterfaceOf,
+} from '@agoric/marshal';
+
+// TODO: assertSubset and assertKeysAllowed are copied from Zoe. Move
+// this code to a location where it can be used by ERTP and Zoe
+// easily. Perhaps another package.
+
+/**
+ * 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);
+};
+
+export const coerceDisplayInfo = allegedDisplayInfo => {
+  if (passStyleOf(allegedDisplayInfo) === REMOTE_STYLE) {
+    // These condition together try to ensure that `allegedDisplayInfo`
+    // is a plain empty object. It will accept all plain empty objects
+    // that it should. It will reject most things we want to reject including
+    // remotables that are explicitly declared `Remotable`. But a normal
+    // HandledPromise presence not explicitly declared `Remotable` will
+    // be mistaken for a plain empty object. Even in this case, the copy
+    // has a new identity, so the only danger is that we didn't reject
+    // with a diagnostic, potentially masking a programmer error.
+    assert(Object.isFrozen(allegedDisplayInfo));
+    assert.equal(Reflect.ownKeys(allegedDisplayInfo).length, 0);
+    assert.equal(Object.getPrototypeOf(allegedDisplayInfo), Object.prototype);
+    assert.equal(getInterfaceOf(allegedDisplayInfo), undefined);
+    return harden({});
+  }
+  allegedDisplayInfo = pureCopy(allegedDisplayInfo);
+  assertDisplayInfo(allegedDisplayInfo);
+  return allegedDisplayInfo;
+};

packages/ERTP/src/issuer.js

@@ -10,16 +10,20 @@ import { isPromise } from '@agoric/promise-kit';
 
 import { makeAmountMath, MathKind } from './amountMath';
 import { makeInterface, ERTPKind } from './interfaces';
+import { coerceDisplayInfo } from './displayInfo';
 
 import './types';
 
 /**
- * @param {string} allegedName
- * @param {AmountMathKind} [amountMathKind=MathKind.NAT]
- * @returns {IssuerKit}
+ * @type {MakeIssuerKit}
  */
-function makeIssuerKit(allegedName, amountMathKind = MathKind.NAT) {
+function makeIssuerKit(
+  allegedName,
+  amountMathKind = MathKind.NAT,
+  displayInfo = undefined,
+) {
   assert.typeof(allegedName, 'string');
+  displayInfo = coerceDisplayInfo(displayInfo);
 
   const brand = Remotable(
     makeInterface(allegedName, ERTPKind.BRAND),
@@ -32,6 +36,9 @@ function makeIssuerKit(allegedName, amountMathKind = MathKind.NAT) {
         });
       },
       getAllegedName: () => allegedName,
+
+      // Give information to UI on how to display the amount.
+      getDisplayInfo: () => displayInfo,
     },
   );
 

packages/ERTP/src/types.js

@@ -97,6 +97,20 @@
  * 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.)
+ *   This property is optional, and for non-fungible digital assets,
+ *   should not be specified.
+ *   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
@@ -112,6 +126,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 {() => DisplayInfo} getDisplayInfo
+ *  Give information to UI on how to display the amount.
  */
 
 /**
@@ -188,7 +204,8 @@
 /**
  * @callback MakeIssuerKit
  * @param {string} allegedName
- * @param {AmountMathKind=} amountMathKind
+ * @param {AmountMathKind} [amountMathKind=MathKind.NAT]
+ * @param {DisplayInfo=} [displayInfo=undefined]
  * @returns {IssuerKit}
  *
  * The allegedName becomes part of the brand in asset descriptions. The
@@ -200,6 +217,8 @@
  * from the mathHelpers library. For example, natMathHelpers, the
  * default, is used for basic fungible tokens.
  *
+ *  `displayInfo` gives information to UI on how to display the amount.
+ *
  * @typedef {Object} IssuerKit
  * The return value of makeIssuerKit
  *

packages/ERTP/test/unitTests/test-issuerObj.js

@@ -14,6 +14,40 @@ test('issuer.getBrand, brand.isMyIssuer', t => {
   );
   t.is(issuer.getAllegedName(), myBrand.getAllegedName());
   t.is(issuer.getAllegedName(), 'fungible');
+  t.is(brand.getDisplayInfo(), undefined);
+});
+
+test('brand.getDisplayInfo()', t => {
+  const displayInfo = harden({ decimalPlaces: 3 });
+  const { brand } = makeIssuerKit('fungible', MathKind.NAT, displayInfo);
+  t.deepEqual(brand.getDisplayInfo(), displayInfo);
+  const display = amount => {
+    const { brand: myBrand, value } = amount;
+    const { decimalPlaces } = myBrand.getDisplayInfo();
+    const valueDisplay = value.toString();
+    const length = valueDisplay.length;
+    return [
+      valueDisplay.slice(0, length - decimalPlaces),
+      '.',
+      valueDisplay.slice(length - decimalPlaces),
+    ].join('');
+  };
+  t.is(display({ brand, value: 3000 }), '3.000');
+});
+
+test('bad display info', t => {
+  const displayInfo = harden({ somethingUnexpected: 3 });
+  // @ts-ignore
+  t.throws(() => makeIssuerKit('fungible', MathKind.NAT, displayInfo), {
+    message:
+      'key "somethingUnexpected" was not one of the expected keys ["decimalPlaces"]',
+  });
+});
+
+test('empty display info', t => {
+  const displayInfo = harden({});
+  const { brand } = makeIssuerKit('fungible', MathKind.NAT, displayInfo);
+  t.deepEqual(brand.getDisplayInfo(), displayInfo);
 });
 
 test('amountMath from makeIssuerKit', async t => {
@@ -117,7 +151,7 @@ test('purse.deposit', async t => {
     .then(checkDeposit(fungible17, fungibleSum));
 });
 
-test('purse.deposit promise', t => {
+test('purse.deposit promise', async t => {
   t.plan(1);
   const { issuer, mint, amountMath } = makeIssuerKit('fungible');
   const fungible25 = amountMath.make(25);
@@ -126,7 +160,8 @@ test('purse.deposit promise', t => {
   const payment = mint.mintPayment(fungible25);
   const exclusivePaymentP = E(issuer).claim(payment);
 
-  return t.throwsAsync(
+  await t.throwsAsync(
+    // @ts-ignore
     () => E(purse).deposit(exclusivePaymentP, fungible25),
     { message: /deposit does not accept promises/ },
     'failed to reject a promise for a payment',
@@ -198,11 +233,11 @@ test('issuer.claim', async t => {
     });
 });
 
-test('issuer.splitMany bad amount', t => {
+test('issuer.splitMany bad amount', async t => {
   const { mint, issuer, amountMath } = makeIssuerKit('fungible');
   const payment = mint.mintPayment(amountMath.make(1000));
   const badAmounts = Array(2).fill(amountMath.make(10));
-  return t.throwsAsync(
+  await t.throwsAsync(
     _ => E(issuer).splitMany(payment, badAmounts),
     { message: /rights were not conserved/ },
     'successfully throw if rights are not conserved in proposed new payments',
@@ -238,11 +273,11 @@ test('issuer.splitMany good amount', async t => {
     .then(checkPayments);
 });
 
-test('issuer.split bad amount', t => {
+test('issuer.split bad amount', async t => {
   const { mint, issuer, amountMath } = makeIssuerKit('fungible');
   const { amountMath: otherUnitOps } = makeIssuerKit('other fungible');
   const payment = mint.mintPayment(amountMath.make(1000));
-  return t.throwsAsync(
+  await t.throwsAsync(
     _ => E(issuer).split(payment, otherUnitOps.make(10)),
     {
       message: /the brand in the allegedAmount in 'coerce' didn't match the amountMath brand/,
@@ -343,7 +378,7 @@ test('issuer.combine bad payments', async t => {
   const otherPayment = otherMint.mintPayment(otherAmountMath.make(10));
   payments.push(otherPayment);
 
-  return t.throwsAsync(
+  await t.throwsAsync(
     () => E(issuer).combine(payments),
     { message: /"payment" not found/ },
     'payment from other mint is not found',

packages/zoe/test/unitTests/zcf/test-zcf.js

@@ -213,7 +213,8 @@ test(`zcf.saveIssuer - bad issuer`, async t => {
   await t.throwsAsync(() => zcf.saveIssuer(moolaKit.brand, 'A'), {
     // TODO: improve error message
     // https://github.com/Agoric/agoric-sdk/issues/1701
-    message: 'target has no method "getBrand", has [getAllegedName,isMyIssuer]',
+    message:
+      'target has no method "getBrand", has [getAllegedName,getDisplayInfo,isMyIssuer]',
   });
 });