refactor: paramManager separates each param updater

change 'any' to 'unknown' drop an unneeded guard clause a single getParams() describes all parameters
Agoric · Jun 17, 2021 · bf2dc9e · bf2dc9e
1 parent b07a981
commit bf2dc9e
Show file tree

Hide file tree

Showing 5 changed files with 227 additions and 176 deletions.
diff --git a/packages/governance/package.json b/packages/governance/package.json
@@ -32,6 +32,7 @@
   "dependencies": {
     "@agoric/assert": "^0.3.0",
     "@agoric/ertp": "^0.11.2",
+    "@agoric/marshal": "^0.4.13",
     "@agoric/nat": "^4.0.0",
     "@agoric/notifier": "^0.3.14",
     "@agoric/store": "^0.4.15",

diff --git a/packages/governance/src/paramManager.js b/packages/governance/src/paramManager.js
@@ -1,13 +1,15 @@
 // @ts-check
 
-import { makeStore } from '@agoric/store';
 import { assert, details as X } from '@agoric/assert';
 import { assertIsRatio } from '@agoric/zoe/src/contractSupport';
 import { AmountMath, looksLikeBrand } from '@agoric/ertp';
+import { Far } from '@agoric/marshal';
+import { assertKeywordName } from '@agoric/zoe/src/cleanProposal';
+
 /**
  * @type {{
  *  AMOUNT: 'amount',
- *  ANY: 'any',
+ *  UNKNOWN: 'unknown',
  *  BRAND: 'brand',
  *  INSTANCE: 'instance',
  *  INSTALLATION: 'installation',
@@ -18,22 +20,17 @@ import { AmountMath, looksLikeBrand } from '@agoric/ertp';
  */
 const ParamType = {
   AMOUNT: 'amount',
-  ANY: 'any',
   BRAND: 'brand',
   INSTANCE: 'instance',
   INSTALLATION: 'installation',
   NAT: 'nat',
   RATIO: 'ratio',
   STRING: 'string',
+  UNKNOWN: 'unknown',
 };
 harden(ParamType);
 
 const assertType = (type, value, name) => {
-  if (!type) {
-    // undefined type means don't verify. Did we omit an interesting type?
-    return;
-  }
-
   switch (type) {
     case ParamType.AMOUNT:
       // It would be nice to have a clean way to assert something is an amount.
@@ -42,8 +39,6 @@ const assertType = (type, value, name) => {
         X`value for ${name} must be an Amount, was ${value}`,
       );
       break;
-    case ParamType.ANY:
-      break;
     case ParamType.BRAND:
       assert(
         looksLikeBrand(value),
@@ -73,46 +68,65 @@ const assertType = (type, value, name) => {
     case ParamType.STRING:
       assert.typeof(value, 'string');
       break;
+    case ParamType.UNKNOWN:
+      break;
     default:
       assert.fail(X`unknown type guard ${type}`);
   }
 };
 
 const parse = paramDesc => {
-  const bindings = makeStore('name');
-  const types = makeStore('name');
+  const values = {};
+  // manager has an updateFoo() for each Foo param. It will be returned.
+  const manager = {};
+  // getParams() uses describers to generate descriptions of each param.
+  const describers = [];
 
   paramDesc.forEach(({ name, value, type }) => {
+    assert(
+      !values[name],
+      X`each parameter name must be unique: ${name} duplicated`,
+    );
     assertType(type, value, name);
-    bindings.init(name, value);
-    types.init(name, type);
+    // we want to create function names like updateFeeRatio(), so we insist that
+    // the name has Keyword-nature.
+    assertKeywordName(name);
+
+    values[name] = { type, value };
+    manager[`update${name}`] = newValue => {
+      assertType(type, newValue, name);
+      values[name].value = newValue;
+    };
+
+    const describer = () => ({
+      name,
+      type,
+      value: values[name].value,
+    });
+    describers.push(describer);
   });
 
-  return { bindings, types };
+  /** @type {() => Record<Keyword,ParamDescription>} */
+  const getParams = () => {
+    const descriptions = {};
+    describers.forEach(d => {
+      const description = d();
+      descriptions[description.name] = description;
+    });
+    return descriptions;
+  };
+
+  return { getParams, manager };
 };
 
 /** @type {BuildParamManager} */
 const buildParamManager = paramDesc => {
-  const { bindings, types } = parse(paramDesc);
-
-  const params = {
-    lookup: name => bindings.get(name),
-    getDetails: name => ({
-      name,
-      value: bindings.get(name),
-      type: types.get(name),
-    }),
-    definedNames: () => bindings.keys(),
-  };
-
-  const manager = {
-    update(name, value) {
-      assertType(types.get(name), value, name);
-      bindings.set(name, value);
-    },
-  };
+  const { getParams, manager } = parse(paramDesc);
 
-  return { params, manager };
+  return Far('param manager', {
+    getParams,
+    ...manager,
+  });
 };
 harden(buildParamManager);
 

diff --git a/packages/governance/src/types.js b/packages/governance/src/types.js
@@ -1,16 +1,11 @@
 // @ts-check
 
 /**
- * @typedef { 'amount' | 'any' | 'brand' | 'installation' | 'instance' | 'nat' | 'ratio' | 'string' } ParamType
+ * @typedef { 'amount' | 'unknown' | 'brand' | 'installation' | 'instance' | 'nat' | 'ratio' | 'string' } ParamType
  */
 
 /**
- * @typedef { Amount | Brand | Instance| Installation | bigint | Ratio | string | any } ParamValue
- */
-
-/**
- * @typedef  {Object} ParamManager
- * @property {(name: string, value: ParamValue) => void} update
+ * @typedef { Amount | Brand | Instance| Installation | bigint | Ratio | string | unknown } ParamValue
  */
 
 /**
@@ -20,26 +15,25 @@
  * @property {ParamType} type
  */
 
-/**
- * @typedef  {Object} ParamManagerPublic
- * @property {(name: string) => ParamValue} lookup
- * @property {(name: string) => ParamDetails} getDetails
- * @property {() => string[]} definedNames
- */
-
 /**
  * @typedef {Object} ParamDescription
  * @property {string} name
  * @property {ParamValue} value
  * @property {ParamType} type
  */
 
+/**
+ * @typedef  {Object} ParamManagerPublic
+ * ParamManagerPublic also has updateFoo methods for each defined parameter.
+ * @property {() => Record<Keyword,ParamDescription>} getParams
+ */
+
 /**
  * @typedef {Array<ParamDescription>} ParamDescriptions
  */
 
 /**
  * @callback BuildParamManager
  * @param {ParamDescriptions} paramDesc
- * @returns {{params: ParamManagerPublic, manager: ParamManager }}
+ * @returns {ParamManagerPublic}
  */