diff --git a/packages/governance/test/unitTests/test-shareHolders.js b/packages/governance/test/unitTests/test-shareHolders.js index c6d80309100d..e573b6495dac 100644 --- 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 index d4cdc52207a6..f5729409be32 100644 --- 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 index 5f7c97a5cc67..6cbfc525a26c 100644 --- 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 index cfe9f02bbdba..71634c4ee9c4 100644 --- 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 index 02baac15ad08..74746e3070b8 100644 --- 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`, ); }; diff --git a/packages/zoe/src/typeGuards.js b/packages/zoe/src/typeGuards.js index 2c161931a015..79c1b60ed98a 100644 --- a/packages/zoe/src/typeGuards.js +++ b/packages/zoe/src/typeGuards.js @@ -3,36 +3,30 @@ import { AmountShape } from '@agoric/ertp'; import { M } from '@agoric/store'; -export const ExitOnDemandShape = harden({ onDemand: null }); +export const AmountRecordKeywordShape = M.recordOf(M.string(), AmountShape); -export const ExitWaivedShape = harden({ waived: null }); - -export const ExitAfterDeadlineShape = harden({ - afterDeadline: { timer: M.remotable(), deadline: M.nat() }, +/** + * After defaults are filled in + */ +export const ProposalShape = harden({ + want: M.recordOf(M.string(), M.pattern()), + give: AmountRecordKeywordShape, + // To accept only one, we could use M.or rather than M.partial, + // but the error messages would have been worse. Rather, + // cleanProposal's assertExit checks that there's exactly one. + exit: M.partial( + { + onDemand: null, + waived: null, + afterDeadline: { + timer: M.remotable(), + deadline: M.nat(), + }, + }, + {}, + ), }); -export const ExitRuleShape = M.or( - ExitOnDemandShape, - ExitWaivedShape, - ExitAfterDeadlineShape, -); - -export const KeywordRecordPatternOf = valuePatt => - M.recordOf(M.string(), valuePatt); - -export const AmountKeywordRecordShape = KeywordRecordPatternOf(AmountShape); - -export const PatternKeywordRecordShape = KeywordRecordPatternOf(M.pattern()); - -export const ProposalShape = M.partial( - { - want: M.or(undefined, PatternKeywordRecordShape), - give: M.or(undefined, AmountKeywordRecordShape), - exit: M.or(undefined, ExitRuleShape), - }, - {}, -); - export const isOnDemandExitRule = exit => { const [exitKey] = Object.getOwnPropertyNames(exit); return exitKey === 'onDemand'; diff --git a/packages/zoe/src/zoeService/types.js b/packages/zoe/src/zoeService/types.js index de49bbf858e3..8b681123adc2 100644 --- a/packages/zoe/src/zoeService/types.js +++ b/packages/zoe/src/zoeService/types.js @@ -206,7 +206,7 @@ */ /** - * @typedef {Partial} Proposal + * @typedef {Partial} Proposal * * @typedef {{give: AmountKeywordRecord, * want: AmountKeywordRecord, diff --git a/packages/zoe/test/unitTests/test-cleanProposal.js b/packages/zoe/test/unitTests/test-cleanProposal.js index 44d96e2d9986..21ff23f9d56b 100644 --- a/packages/zoe/test/unitTests/test-cleanProposal.js +++ b/packages/zoe/test/unitTests/test-cleanProposal.js @@ -100,6 +100,21 @@ test('cleanProposal - other wrong stuff', t => { message, }); + proposeBad( + 'foo', + 'nat', + /"proposal" "foo" must be a pass-by-copy record, not "string"/, + ); + proposeBad( + { want: 'foo' }, + 'nat', + /"keywordRecord" "foo" must be a pass-by-copy record, not "string"/, + ); + proposeBad( + { give: 'foo' }, + 'nat', + /"keywordRecord" "foo" must be a pass-by-copy record, not "string"/, + ); proposeBad( { want: { lowercase: simoleans(1n) } }, 'nat', @@ -116,9 +131,9 @@ test('cleanProposal - other wrong stuff', t => { /keyword "Not Ident" must be an ascii identifier starting with upper case./, ); proposeBad( - { what: { 'Not Ident': simoleans(1n) } }, + { what: { A: simoleans(1n) } }, 'nat', - /key "what" was not one of the expected keys \["want","give","exit"\]/, + /.* - Must only have want:, give:, exit: properties: {"what":.*}/, ); proposeBad( { [Symbol.for('what')]: { 'Not Ident': simoleans(1n) } }, @@ -130,15 +145,60 @@ test('cleanProposal - other wrong stuff', t => { 'nat', /cannot serialize Remotables with non-methods like "Symbol\(S\)" in {}/, ); + proposeBad( + { exit: 'foo' }, + 'nat', + /"foo" - Must have shape of base: "copyRecord"/, + ); + proposeBad( + { exit: { onDemand: 'foo' } }, + 'nat', + /{"onDemand":"foo"} - Must be equivalent to: {"onDemand":null}/, + ); + proposeBad( + { exit: { afterDeadline: 'foo' } }, + 'nat', + /"foo" - Must be a copyRecord to match a copyRecord pattern: {"timer":.*/, + ); + proposeBad( + { exit: { afterDeadline: { timer: 'foo', deadline: 3n } } }, + 'nat', + /"foo" - Must have passStyle or tag "remotable"/, + ); + proposeBad( + { exit: { afterDeadline: { timer, deadline: 'foo' } } }, + 'nat', + /"foo" - Must be >= "\[0n\]"/, + ); + proposeBad( + { exit: { afterDeadline: { timer: 'foo', deadline: 3n } } }, + 'nat', + /"foo" - Must have passStyle or tag "remotable"/, + ); + proposeBad( + { exit: { afterDeadline: { timer, deadline: 3n, extra: 'foo' } } }, + 'nat', + /.* - Must have same property names as record pattern:.*/, + ); + proposeBad( + { exit: { afterDeadline: { timer } } }, + 'nat', + /.* - Must have same property names as record pattern:.*/, + ); + proposeBad( + { exit: { afterDeadline: { deadline: 3n } } }, + 'nat', + /.* - Must have same property names as record pattern:.*/, + ); proposeBad( { exit: { afterDeadline: { timer, deadline: 3 } } }, 'nat', - /deadline must be a Nat BigInt/, + /3 - Must be >= "\[0n\]"/, ); proposeBad( { exit: { afterDeadline: { timer, deadline: -3n } } }, 'nat', - /deadline must be a Nat BigInt/, + /"\[-3n\]" - Must be >= "\[0n\]"/, ); proposeBad({ exit: {} }, 'nat', /exit {} should only have one key/); proposeBad( diff --git a/packages/zoe/test/unitTests/zcf/test-zcf.js b/packages/zoe/test/unitTests/zcf/test-zcf.js index fd6047280197..3b9e2b5ef192 100644 --- a/packages/zoe/test/unitTests/zcf/test-zcf.js +++ b/packages/zoe/test/unitTests/zcf/test-zcf.js @@ -430,7 +430,7 @@ test(`zcf.makeZCFMint - mintGains - no args`, async t => { // @ts-ignore deliberate invalid arguments for testing t.throws(() => zcfMint.mintGains(), { message: - '"amountKeywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', + '"keywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', }); }); @@ -456,7 +456,7 @@ test(`zcf.makeZCFMint - mintGains - no gains`, async t => { // @ts-ignore deliberate invalid arguments for testing t.throws(() => zcfMint.mintGains(undefined, zcfSeat), { message: - '"amountKeywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', + '"keywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', }); }); @@ -466,7 +466,7 @@ test(`zcf.makeZCFMint - burnLosses - no args`, async t => { // @ts-ignore deliberate invalid arguments for testing t.throws(() => zcfMint.burnLosses(), { message: - '"amountKeywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', + '"keywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', }); }); @@ -477,7 +477,7 @@ test(`zcf.makeZCFMint - burnLosses - no losses`, async t => { // @ts-ignore deliberate invalid arguments for testing t.throws(() => zcfMint.burnLosses(undefined, zcfSeat), { message: - '"amountKeywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', + '"keywordRecord" "[undefined]" must be a pass-by-copy record, not "undefined"', }); });