fix: use ProposalShape

Agoric · Jan 6, 2022 · fecfaee · fecfaee
1 parent c89503e
commit fecfaee
Show file tree

Hide file tree

Showing 9 changed files with 209 additions and 167 deletions.
diff --git a/packages/governance/test/unitTests/test-shareHolders.js b/packages/governance/test/unitTests/test-shareHolders.js
@@ -59,19 +59,21 @@ const attest = (addr, amountLiened, expiration) => {
  * @param {Timestamp} deadline
  */
 const makeDefaultBallotSpec = (issue, positions, timer, deadline) => {
-  const questionSpec = looksLikeQuestionSpec({
-    method: ChoiceMethod.UNRANKED,
-    issue,
-    positions,
-    electionType: ElectionType.ELECTION,
-    maxChoices: 1,
-    closingRule: {
-      timer,
-      deadline,
-    },
-    quorumRule: QuorumRule.NO_QUORUM,
-    tieOutcome: positions[1],
-  });
+  const questionSpec = looksLikeQuestionSpec(
+    harden({
+      method: ChoiceMethod.UNRANKED,
+      issue,
+      positions,
+      electionType: ElectionType.ELECTION,
+      maxChoices: 1,
+      closingRule: {
+        timer,
+        deadline,
+      },
+      quorumRule: QuorumRule.NO_QUORUM,
+      tieOutcome: positions[1],
+    }),
+  );
 
   return questionSpec;
 };
@@ -92,9 +94,13 @@ const offerToVoteSeat = (attestationMint, publicElectorate, attestation) => {
     give: { Attestation: attestation },
     want: {},
   });
-  return E(zoe).offer(E(publicElectorate).makeVoterInvitation(), proposal, {
-    Attestation: attestation1,
-  });
+  return E(zoe).offer(
+    E(publicElectorate).makeVoterInvitation(),
+    proposal,
+    harden({
+      Attestation: attestation1,
+    }),
+  );
 };
 
 /**
@@ -112,10 +118,10 @@ const voterFacet = (mint, publicFacet, attestAmmount) => {
  */
 const addDeposeQuestion = async (timer, creatorFacet) => {
   const depose = harden({ text: 'Replace the CEO?' });
-  const deposePositions = [
+  const deposePositions = harden([
     harden({ text: 'Yes, replace' }),
     harden({ text: 'no change' }),
-  ];
+  ]);
   const deposeSpec = makeDefaultBallotSpec(depose, deposePositions, timer, 2n);
   const { publicFacet: deposeCounter, questionHandle } = await E(
     creatorFacet,

diff --git a/packages/store/src/patterns/patternMatchers.js b/packages/store/src/patterns/patternMatchers.js
@@ -860,7 +860,11 @@ const makePatternKit = () => {
       harden(specR);
       return (
         (checkMatches(specB, base, check) &&
-        (rest === undefined || checkMatches(specR, rest, check)))
+        (rest === undefined ||
+          check(
+            matches(specR, rest),
+            X`Remainder ${specR} - Must match ${rest}`,
+          )))
       );
     },
 
@@ -935,7 +939,11 @@ const makePatternKit = () => {
       harden(newBase);
       return (
         (checkMatches(specB, newBase, check) &&
-        (rest === undefined || checkMatches(specR, rest, check)))
+        (rest === undefined ||
+          check(
+            matches(specR, rest),
+            X`Remainder ${specR} - Must match ${rest}`,
+          )))
       );
     },
 

diff --git a/packages/store/test/test-patterns.js b/packages/store/test/test-patterns.js
@@ -87,7 +87,7 @@ const matchTests = harden([
       [M.split([3, 4, 5, 6]), /\[3,4\] - Must be equivalent to: \[3,4,5,6\]/],
       [M.split([5]), /\[3\] - Must be equivalent to: \[5\]/],
       [M.split({}), /\[3,4\] - Must have shape of base: "copyRecord"/],
-      [M.split([3], 'x'), /\[4\] - Must be equivalent to: "x"/],
+      [M.split([3], 'x'), /Remainder \[4\] - Must match "x"/],
 
       [M.partial([5]), /\[3\] - Must be equivalent to: \[5\]/],
 
@@ -158,7 +158,7 @@ const matchTests = harden([
       ],
       [
         M.split({ foo: 3 }, { foo: 3, bar: 4 }),
-        /{"bar":4} - Must be equivalent to: {"foo":3,"bar":4}/,
+        /Remainder {"bar":4} - Must match {"foo":3,"bar":4}/,
       ],
       [
         M.partial({ foo: 7, zip: 5 }, { bar: 4 }),

diff --git a/packages/zoe/src/cleanProposal.js b/packages/zoe/src/cleanProposal.js
@@ -1,20 +1,17 @@
 // @ts-check
 
 import { assert, details as X, q } from '@agoric/assert';
-import { isNat } from '@agoric/nat';
 import { AmountMath, getAssetKind } from '@agoric/ertp';
 import { assertRecord } from '@agoric/marshal';
-import { assertPattern } from '@agoric/store';
-import {
-  isOnDemandExitRule,
-  isWaivedExitRule,
-  isAfterDeadlineExitRule,
-} from './typeGuards.js';
-import { arrayToObj, assertSubset } from './objArrayConversion.js';
+import { assertKey, assertPattern, fit, isKey } from '@agoric/store';
+import { ProposalShape } from './typeGuards.js';
+import { arrayToObj } from './objArrayConversion.js';
 
 import '../exported.js';
 import './internal-types.js';
 
+const { ownKeys } = Reflect;
+
 const firstCapASCII = /^[A-Z][a-zA-Z0-9_$]*$/;
 
 // We adopt simple requirements on keywords so that they do not accidentally
@@ -40,116 +37,83 @@ export const assertKeywordName = keyword => {
   );
 };
 
-// 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.
-const assertKeysAllowed = (allowedKeys, record) => {
-  const keys = Object.getOwnPropertyNames(record);
-  assertSubset(allowedKeys, keys);
-  // assert that there are no symbol properties.
-  // TODO unreachable: already rejected as unpassable
-  assert(
-    Object.getOwnPropertySymbols(record).length === 0,
-    X`no symbol properties allowed`,
-  );
-};
+/**
+ * @param {{[name: string]: any}} uncleanKeywordRecord
+ * @returns {string[]}
+ */
+export const cleanKeywords = uncleanKeywordRecord => {
+  harden(uncleanKeywordRecord);
+  assertRecord(uncleanKeywordRecord, 'keywordRecord');
+  const keywords = ownKeys(uncleanKeywordRecord);
 
-const cleanKeys = (allowedKeys, record) => {
-  assertKeysAllowed(allowedKeys, record);
-  return harden(Object.getOwnPropertyNames(record));
-};
+  // Assert all names are ascii identifiers starting with
+  // an upper case letter.
+  keywords.forEach(assertKeywordName);
 
-export const getKeywords = keywordRecord =>
-  harden(Object.getOwnPropertyNames(keywordRecord));
+  return /** @type {string[]} */ (keywords);
+};
 
-export const coerceAmountKeywordRecord = (
+export const coerceAmountPatternKeywordRecord = (
   allegedAmountKeywordRecord,
   getAssetKindByBrand,
 ) => {
-  assertRecord(allegedAmountKeywordRecord, 'amountKeywordRecord');
-  const keywords = getKeywords(allegedAmountKeywordRecord);
-  keywords.forEach(assertKeywordName);
+  const keywords = cleanKeywords(allegedAmountKeywordRecord);
 
   const amounts = Object.values(allegedAmountKeywordRecord);
   // Check that each value can be coerced using the AmountMath
   // indicated by brand. `AmountMath.coerce` throws if coercion fails.
   const coercedAmounts = amounts.map(amount => {
-    const brandAssetKind = getAssetKindByBrand(amount.brand);
-    const assetKind = getAssetKind(amount);
-    // TODO: replace this assertion with a check of the assetKind
-    // property on the brand, when that exists.
-    assert(
-      assetKind === brandAssetKind,
-      X`The amount ${amount} did not have the assetKind of the brand ${brandAssetKind}`,
-    );
-    return AmountMath.coerce(amount.brand, amount);
+    if (isKey(amount)) {
+      const brandAssetKind = getAssetKindByBrand(amount.brand);
+      const assetKind = getAssetKind(amount);
+      // TODO: replace this assertion with a check of the assetKind
+      // property on the brand, when that exists.
+      assert(
+        assetKind === brandAssetKind,
+        X`The amount ${amount} did not have the assetKind of the brand ${brandAssetKind}`,
+      );
+      return AmountMath.coerce(amount.brand, amount);
+    } else {
+      assertPattern(amount);
+      return amount;
+    }
   });
 
   // Recreate the amountKeywordRecord with coercedAmounts.
   return harden(arrayToObj(coercedAmounts, keywords));
 };
 
-export const cleanKeywords = keywordRecord => {
-  // `getOwnPropertyNames` returns all the non-symbol properties
-  // (both enumerable and non-enumerable).
-  const keywords = Object.getOwnPropertyNames(keywordRecord);
-
-  // Insist that there are no symbol properties.
-  // TODO unreachable: already rejected as unpassable
-  assert(
-    Object.getOwnPropertySymbols(keywordRecord).length === 0,
-    X`no symbol properties allowed`,
-  );
-
-  // Assert all key characters are ascii and keys start with a
-  // capital letter.
-  keywords.forEach(assertKeywordName);
-
-  return keywords;
-};
-
-const expectedAfterDeadlineKeys = harden(['timer', 'deadline']);
-
-const assertAfterDeadlineExitRule = exit => {
-  assertKeysAllowed(expectedAfterDeadlineKeys, exit.afterDeadline);
-  assert(exit.afterDeadline.timer !== undefined, X`timer must be defined`);
-  assert(
-    typeof exit.afterDeadline.deadline === 'bigint' &&
-      isNat(exit.afterDeadline.deadline),
-    X`deadline must be a Nat BigInt`,
+export const coerceAmountKeywordRecord = (
+  allegedAmountKeywordRecord,
+  getAssetKindByBrand,
+) => {
+  const result = coerceAmountPatternKeywordRecord(
+    allegedAmountKeywordRecord,
+    getAssetKindByBrand,
   );
+  assertKey(result);
+  return result;
 };
 
-const assertExitValueNull = (exit, exitKey) =>
-  assert(exit[exitKey] === null, `exit value must be null for key ${exitKey}`);
-
-// We expect the single exit key to be one of the following:
-const allowedExitKeys = harden(['onDemand', 'afterDeadline', 'waived']);
-
-const assertExit = exit => {
-  assert(
-    Object.getOwnPropertyNames(exit).length === 1,
-    X`exit ${exit} should only have one key`,
-  );
-
-  const [exitKey] = cleanKeys(allowedExitKeys, exit);
-  if (isOnDemandExitRule(exit) || isWaivedExitRule(exit)) {
-    assertExitValueNull(exit, exitKey);
-  }
-  if (isAfterDeadlineExitRule(exit)) {
-    assertAfterDeadlineExitRule(exit);
-  }
-};
+/**
+ * Just checks residual issues after matching ProposalShape.
+ * Only known residual issue is verifying that it only has one of the
+ * optional properties.
+ *
+ * @param {ExitRule} exit
+ */
+const assertExit = exit =>
+  assert(ownKeys(exit).length === 1, X`exit ${exit} should only have one key`);
 
 /**
  * check that keyword is not in both 'want' and 'give'.
  *
- * @param {Proposal["want"]} want
- * @param {Proposal["give"]} give
+ * @param {ProposalRecord["want"]} want
+ * @param {ProposalRecord["give"]} give
  */
 const assertKeywordNotInBoth = (want, give) => {
-  const wantKeywordSet = new Set(Object.getOwnPropertyNames(want));
-  const giveKeywords = Object.getOwnPropertyNames(give);
+  const wantKeywordSet = new Set(ownKeys(want));
+  const giveKeywords = ownKeys(give);
 
   giveKeywords.forEach(keyword => {
     assert(
@@ -159,8 +123,6 @@ const assertKeywordNotInBoth = (want, give) => {
   });
 };
 
-const rootKeysAllowed = harden(['want', 'give', 'exit']);
-
 /**
  * cleanProposal checks the keys and values of the proposal, including
  * the keys and values of the internal objects. The proposal may have
@@ -178,22 +140,32 @@ const rootKeysAllowed = harden(['want', 'give', 'exit']);
  * @returns {ProposalRecord}
  */
 export const cleanProposal = (proposal, getAssetKindByBrand) => {
-  assertPattern(proposal);
-  assertKeysAllowed(rootKeysAllowed, proposal);
-
-  // We fill in the default values if the keys are undefined.
-  let { want = harden({}), give = harden({}) } = proposal;
-
+  assertRecord(proposal, 'proposal');
+  // We fill in the default values if the keys are absent or undefined.
   const {
-    /** @type {ExitRule} */ exit = harden({ onDemand: null }),
+    want = harden({}),
+    give = harden({}),
+    exit = harden({ onDemand: null }),
+    ...rest
   } = proposal;
+  assert(
+    ownKeys(rest).length === 0,
+    X`${proposal} - Must only have want:, give:, exit: properties: ${rest}`,
+  );
 
-  want = coerceAmountKeywordRecord(want, getAssetKindByBrand);
-  give = coerceAmountKeywordRecord(give, getAssetKindByBrand);
+  const cleanedWant = coerceAmountPatternKeywordRecord(
+    want,
+    getAssetKindByBrand,
+  );
+  const cleanedGive = coerceAmountKeywordRecord(give, getAssetKindByBrand);
 
+  const cleanedProposal = harden({
+    want: cleanedWant,
+    give: cleanedGive,
+    exit,
+  });
+  fit(cleanedProposal, ProposalShape);
   assertExit(exit);
-
-  assertKeywordNotInBoth(want, give);
-
-  return harden({ want, give, exit });
+  assertKeywordNotInBoth(cleanedWant, cleanedGive);
+  return cleanedProposal;
 };
diff --git a/packages/zoe/src/instanceRecordStorage.js b/packages/zoe/src/instanceRecordStorage.js
@@ -1,7 +1,9 @@
 // @ts-check
 
 import { assert, details as X, q } from '@agoric/assert';
-import { assertKeywordName, getKeywords } from './cleanProposal.js';
+import { assertKeywordName } from './cleanProposal.js';
+
+const { ownKeys } = Reflect;
 
 /**
  * The InstanceRecord stores the installation, customTerms, issuers,
@@ -75,7 +77,7 @@ export const makeInstanceRecordStorage = () => {
     assertInstantiated();
     assertKeywordName(keyword);
     assert(
-      !getKeywords(instanceRecord.terms.issuers).includes(keyword),
+      !ownKeys(instanceRecord.terms.issuers).includes(keyword),
       X`keyword ${q(keyword)} must be unique`,
     );
   };