From b894b95a04f219d58462047d8b8986689e0f7d26 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sun, 26 Dec 2021 11:30:46 -0800 Subject: [PATCH] fix: ertp use patterns --- packages/ERTP/src/paymentLedger.js | 34 +++++----- packages/ERTP/src/purse.js | 4 +- packages/ERTP/src/typeGuards.js | 21 +++--- packages/ERTP/src/types.js | 68 ++++++++++++------- .../test/unitTests/test-inputValidation.js | 3 +- packages/vats/src/virtual-purse.js | 13 ++-- 6 files changed, 82 insertions(+), 61 deletions(-) diff --git a/packages/ERTP/src/paymentLedger.js b/packages/ERTP/src/paymentLedger.js index c2f7dde7979..11d10a63a3b 100644 --- a/packages/ERTP/src/paymentLedger.js +++ b/packages/ERTP/src/paymentLedger.js @@ -4,7 +4,7 @@ import { assert, details as X } from '@agoric/assert'; import { E } from '@agoric/eventual-send'; import { isPromise } from '@agoric/promise-kit'; import { Far, assertCopyArray } from '@agoric/marshal'; -import { makeWeakStore } from '@agoric/store'; +import { makeWeakStore, fit } from '@agoric/store'; import { AmountMath } from './amountMath.js'; import { makePayment } from './payment.js'; @@ -61,22 +61,20 @@ export const makePaymentLedger = ( const emptyAmount = AmountMath.makeEmpty(brand, assetKind); /** - * Methods like deposit() have an optional second parameter `amount` - * which, if present, is supposed to be equal to the balance of the + * Methods like deposit() have an optional second parameter + * `optAmountPattern` + * which, if present, is supposed to match the balance of the * payment. This helper function does that check. * - * Note: `amount` is user-supplied with no previous validation. + * Note: `optAmountPattern` is user-supplied with no previous validation. * * @param {Amount} paymentBalance - * @param {Amount | undefined} amount + * @param {Pattern=} optAmountPattern * @returns {void} */ - const assertAmountConsistent = (paymentBalance, amount) => { - if (amount !== undefined) { - assert( - isEqual(amount, paymentBalance), - X`payment balance ${paymentBalance} must equal amount ${amount}`, - ); + const assertAmountConsistent = (paymentBalance, optAmountPattern) => { + if (optAmountPattern !== undefined) { + fit(paymentBalance, optAmountPattern); } }; @@ -170,11 +168,11 @@ export const makePaymentLedger = ( }; /** @type {IssuerBurn} */ - const burn = (paymentP, optAmount = undefined) => { + const burn = (paymentP, optAmountPattern = undefined) => { return E.when(paymentP, payment => { assertLivePayment(payment); const paymentBalance = paymentLedger.get(payment); - assertAmountConsistent(paymentBalance, optAmount); + assertAmountConsistent(paymentBalance, optAmountPattern); try { // COMMIT POINT. paymentLedger.delete(payment); @@ -187,11 +185,11 @@ export const makePaymentLedger = ( }; /** @type {IssuerClaim} */ - const claim = (paymentP, optAmount = undefined) => { + const claim = (paymentP, optAmountPattern = undefined) => { return E.when(paymentP, srcPayment => { assertLivePayment(srcPayment); const srcPaymentBalance = paymentLedger.get(srcPayment); - assertAmountConsistent(srcPaymentBalance, optAmount); + assertAmountConsistent(srcPaymentBalance, optAmountPattern); // Note COMMIT POINT within moveAssets. const [payment] = moveAssets( harden([srcPayment]), @@ -266,14 +264,14 @@ export const makePaymentLedger = ( * @param {(newPurseBalance: Amount) => void} updatePurseBalance - * commit the purse balance * @param {Payment} srcPayment - * @param {Amount=} optAmount + * @param {Pattern=} optAmountPattern * @returns {Amount} */ const deposit = ( currentBalance, updatePurseBalance, srcPayment, - optAmount = undefined, + optAmountPattern = undefined, ) => { assert( !isPromise(srcPayment), @@ -282,7 +280,7 @@ export const makePaymentLedger = ( ); assertLivePayment(srcPayment); const srcPaymentBalance = paymentLedger.get(srcPayment); - assertAmountConsistent(srcPaymentBalance, optAmount); + assertAmountConsistent(srcPaymentBalance, optAmountPattern); const newPurseBalance = add(srcPaymentBalance, currentBalance); try { // COMMIT POINT diff --git a/packages/ERTP/src/purse.js b/packages/ERTP/src/purse.js index bf7d18f998b..3e8ae57403e 100644 --- a/packages/ERTP/src/purse.js +++ b/packages/ERTP/src/purse.js @@ -18,13 +18,13 @@ export const makePurse = (allegedName, assetKind, brand, purseMethods) => { /** @type {Purse} */ const purse = Far(`${allegedName} purse`, { - deposit: (srcPayment, optAmount = undefined) => { + deposit: (srcPayment, optAmountPattern = undefined) => { // Note COMMIT POINT within deposit. return purseMethods.deposit( currentBalance, updatePurseBalance, srcPayment, - optAmount, + optAmountPattern, ); }, withdraw: amount => diff --git a/packages/ERTP/src/typeGuards.js b/packages/ERTP/src/typeGuards.js index 4e1b0feb424..c84147016a7 100644 --- a/packages/ERTP/src/typeGuards.js +++ b/packages/ERTP/src/typeGuards.js @@ -1,6 +1,13 @@ -import { isNat } from '@agoric/nat'; -import { passStyleOf } from '@agoric/marshal'; -import { isKey } from '@agoric/store'; +import { M, matches } from '@agoric/store'; + +export const NatValuePattern = M.nat(); +export const SetValuePattern = M.arrayOf(M.key()); +export const AmountValuePattern = M.or(NatValuePattern, SetValuePattern); + +export const AmountPattern = harden({ + brand: M.remotable(), + value: AmountValuePattern, +}); /** * Returns true if value is a Nat bigint. @@ -8,9 +15,7 @@ import { isKey } from '@agoric/store'; * @param {Value} value * @returns {value is NatValue} */ -const isNatValue = value => { - return typeof value === 'bigint' && isNat(value); -}; +const isNatValue = value => matches(value, NatValuePattern); harden(isNatValue); /** @@ -20,9 +25,7 @@ harden(isNatValue); * @param {Value} value * @returns {value is SetValue} */ -const isSetValue = value => { - return passStyleOf(value) === 'copyArray' && isKey(value); -}; +const isSetValue = value => matches(value, SetValuePattern); harden(isSetValue); export { isNatValue, isSetValue }; diff --git a/packages/ERTP/src/types.js b/packages/ERTP/src/types.js index a2dde9195d7..070bad0bc06 100644 --- a/packages/ERTP/src/types.js +++ b/packages/ERTP/src/types.js @@ -93,25 +93,41 @@ * @property {(amount: Amount, brand?: Brand) => boolean} isEmpty * Return true if the Amount is empty. Otherwise false. * - * @property {(leftAmount: Amount, rightAmount: Amount, brand?: Brand) => boolean} isGTE + * @property {( + * leftAmount: Amount, + * rightAmount: Amount, + * brand?: Brand + * ) => boolean} isGTE * Returns true if the leftAmount is greater than or equal to the * rightAmount. For non-scalars, "greater than or equal to" depends * on the kind of amount, as defined by the MathHelpers. For example, * whether rectangle A is greater than rectangle B depends on whether rectangle * A includes rectangle B as defined by the logic in MathHelpers. * - * @property {(leftAmount: Amount, rightAmount: Amount, brand?: Brand) => boolean} isEqual + * @property {( + * leftAmount: Amount, + * rightAmount: Amount, + * brand?: Brand + * ) => boolean} isEqual * Returns true if the leftAmount equals the rightAmount. We assume * that if isGTE is true in both directions, isEqual is also true * - * @property {(leftAmount: Amount, rightAmount: Amount, brand?: Brand) => Amount} add + * @property {( + * leftAmount: Amount, + * rightAmount: Amount, + * brand?: Brand + * ) => Amount} add * Returns a new amount that is the union of both leftAmount and rightAmount. * * For fungible amount this means adding the values. For other kinds of * amount, it usually means including all of the elements from both * left and right. * - * @property {(leftAmount: Amount, rightAmount: Amount, brand?: Brand) => Amount} subtract + * @property {( + * leftAmount: Amount, + * rightAmount: Amount, + * brand?: Brand + * ) => Amount} subtract * Returns a new amount that is the leftAmount minus the rightAmount * (i.e. everything in the leftAmount that is not in the * rightAmount). If leftAmount doesn't include rightAmount @@ -148,11 +164,11 @@ * represents the issuer they intended, since the same brand can be reused by * a misbehaving issuer. * - * @property {(allegedIssuer: ERef) => Promise} isMyIssuer Should be used with - * `issuer.getBrand` to ensure an issuer and brand match. + * @property {(allegedIssuer: ERef) => Promise} 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. + * Give information to UI on how to display the amount. */ /** @@ -168,7 +184,7 @@ * resolution. * * @param {ERef} payment - * @param {Amount=} optAmount + * @param {Pattern=} optAmountPattern * @returns {Promise} */ @@ -185,7 +201,7 @@ * resolution. * * @param {ERef} payment - * @param {Amount=} optAmount + * @param {Pattern=} optAmountPattern * @returns {Promise} */ @@ -376,7 +392,7 @@ /** * @callback DepositFacetReceive * @param {Payment} payment - * @param {Amount=} optAmount + * @param {Pattern=} optAmountPattern * @returns {Amount} */ @@ -393,20 +409,20 @@ /** * @callback PurseDeposit * @param {Payment} payment - * @param {Amount=} optAmount + * @param {Pattern=} optAmountPattern * @returns {Amount} */ /** * @typedef {Object} Purse - * Purses hold amount of digital assets of the same brand, but unlike Payments, they are - * not meant to be sent to others. To transfer digital assets, a + * Purses hold amount of digital assets of the same brand, but unlike Payments, + * they are not meant to be sent to others. To transfer digital assets, a * Payment should be withdrawn from a Purse. The amount of digital * assets in a purse can change through the action of deposit() and withdraw(). * * The primary use for Purses and Payments is for currency-like and goods-like - * digital assets, but they can also be used to represent other kinds of rights, such - * as the right to participate in a particular contract. + * digital assets, but they can also be used to represent other kinds of rights, + * such as the right to participate in a particular contract. * * @property {() => Brand} getAllegedBrand Get the alleged Brand for this Purse * @@ -432,18 +448,20 @@ /** * @typedef {Object} Payment - * Payments hold amount of digital assets of the same brand in transit. Payments can - * be deposited in purses, split into multiple payments, combined, and + * Payments hold amount of digital assets of the same brand in transit. Payments + * can be deposited in purses, split into multiple payments, combined, and * claimed (getting an exclusive payment). Payments are linear, meaning * that either a payment has the same amount of digital assets it - * started with, or it is used up entirely. It is impossible to partially use a payment. + * started with, or it is used up entirely. It is impossible to partially use a + * payment. * * Payments are often received from other actors and therefore should - * not be trusted themselves. To get the amount of digital assets in a payment, use the - * trusted issuer: issuer.getAmountOf(payment), + * not be trusted themselves. To get the amount of digital assets in a payment, + * use the trusted issuer: issuer.getAmountOf(payment), * * Payments can be converted to Purses by getting a trusted issuer and - * calling `issuer.makeEmptyPurse()` to create a purse, then `purse.deposit(payment)`. + * calling `issuer.makeEmptyPurse()` to create a purse, then + * `purse.deposit(payment)`. * * @property {() => Brand} getAllegedBrand * Get the allegedBrand, indicating the type of digital asset this @@ -455,12 +473,12 @@ /** * @template V * @typedef {Object} MathHelpers - * All of the difference in how digital asset amount are manipulated can be reduced to - * the behavior of the math on values. We extract this + * All of the difference in how digital asset amount are manipulated can be + * reduced to the behavior of the math on values. We extract this * custom logic into mathHelpers. MathHelpers are about value * arithmetic, whereas AmountMath is about amounts, which are the - * values labeled with a brand. AmountMath use mathHelpers to do their value arithmetic, - * and then brand the results, making a new amount. + * values labeled with a brand. AmountMath use mathHelpers to do their value + * arithmetic, and then brand the results, making a new amount. * * The MathHelpers are designed to be called only from AmountMath, and so * all methods but coerce can assume their inputs are valid. They only diff --git a/packages/ERTP/test/unitTests/test-inputValidation.js b/packages/ERTP/test/unitTests/test-inputValidation.js index 8c1c86f9bbb..479d4b268f2 100644 --- a/packages/ERTP/test/unitTests/test-inputValidation.js +++ b/packages/ERTP/test/unitTests/test-inputValidation.js @@ -104,7 +104,8 @@ test('brand.isMyIssuer bad issuer', async t => { t.false(result); }); -// Tested in the context of an issuer.claim call, as assertLivePayment is not exported +// Tested in the context of an issuer.claim call, as assertLivePayment is not +// exported test('assertLivePayment', async t => { const { issuer, mint, brand } = makeIssuerKit('fungible'); const { mint: mintB, brand: brandB } = makeIssuerKit('fungibleB'); diff --git a/packages/vats/src/virtual-purse.js b/packages/vats/src/virtual-purse.js index 770a2a3c4b7..064a48b1b77 100644 --- a/packages/vats/src/virtual-purse.js +++ b/packages/vats/src/virtual-purse.js @@ -48,17 +48,18 @@ function makeVirtualPurse(vpc, kit) { /** @type {(amt: Amount) => Promise} */ let redeem; - /** @type {(pmt: Payment, optAmount: Amount | undefined) => Promise} */ + /** @type {(pmt: Payment, optAmountPattern?: Pattern) => Promise} */ let retain; if (mint) { - retain = (payment, optAmount) => E(issuer).burn(payment, optAmount); + retain = (payment, optAmountPattern) => + E(issuer).burn(payment, optAmountPattern); redeem = amount => E(mint).mintPayment(amount); } else { // If we can't mint, then we need to escrow. const myEscrowPurse = escrowPurse || E(issuer).makeEmptyPurse(); - retain = (payment, optAmount) => - E(myEscrowPurse).deposit(payment, optAmount); + retain = (payment, optAmountPattern) => + E(myEscrowPurse).deposit(payment, optAmountPattern); redeem = amount => E(myEscrowPurse).withdraw(amount); } @@ -93,13 +94,13 @@ function makeVirtualPurse(vpc, kit) { /** @type {EOnly} */ const depositFacet = Far('Virtual Deposit Facet', { - async receive(payment, optAmount = undefined) { + async receive(payment, optAmountPattern = undefined) { if (isPromise(payment)) { throw TypeError( `deposit does not accept promises as first argument. Instead of passing the promise (deposit(paymentPromise)), consider unwrapping the promise first: paymentPromise.then(actualPayment => deposit(actualPayment))`, ); } - const amt = await retain(payment, optAmount); + const amt = await retain(payment, optAmountPattern); // The push must always succeed. //