diff --git a/packages/ERTP/src/issuerKit.js b/packages/ERTP/src/issuerKit.js index 65cce4993c5..3aaa0ddfbe0 100644 --- a/packages/ERTP/src/issuerKit.js +++ b/packages/ERTP/src/issuerKit.js @@ -1,6 +1,6 @@ // @jessie-check -import { assert } from '@agoric/assert'; +import { assert, Fail } from '@agoric/assert'; import { assertPattern } from '@agoric/store'; import { makeScalarBigMapStore } from '@agoric/vat-data'; import { makeDurableZone } from '@agoric/zone/durable.js'; @@ -26,6 +26,8 @@ import './types-ambient.js'; * @template {AssetKind} K * @param {IssuerRecord} issuerRecord * @param {import('@agoric/zone').Zone} issuerZone + * @param {RecoverySetsOption} recoverySetsState Omitted from issuerRecord + * because it was added in an upgrade. * @param {ShutdownWithFailure} [optShutdownWithFailure] If this issuer fails in * the middle of an atomic action (which btw should never happen), it * potentially leaves its ledger in a corrupted state. If this function was @@ -38,6 +40,7 @@ import './types-ambient.js'; const setupIssuerKit = ( { name, assetKind, displayInfo, elementShape }, issuerZone, + recoverySetsState, optShutdownWithFailure = undefined, ) => { assert.typeof(name, 'string'); @@ -62,6 +65,7 @@ const setupIssuerKit = ( assetKind, cleanDisplayInfo, elementShape, + recoverySetsState, optShutdownWithFailure, ); @@ -77,6 +81,12 @@ harden(setupIssuerKit); /** The key at which the issuer record is stored. */ const INSTANCE_KEY = 'issuer'; +/** + * The key at which the issuerKit's `RecoverySetsOption` state is stored. + * Introduced by an upgrade, so may be absent on a predecessor incarnation. See + * `RecoverySetsOption` for defaulting behavior. + */ +const RECOVERY_SETS_STATE = 'recoverySetsState'; /** * Used _only_ to upgrade a predecessor issuerKit. Use `makeDurableIssuerKit` to @@ -91,15 +101,39 @@ const INSTANCE_KEY = 'issuer'; * unit of computation, like the enclosing vat, can be shutdown before * anything else is corrupted by that corrupted state. See * https://github.com/Agoric/agoric-sdk/issues/3434 + * @param {RecoverySetsOption} [recoverySetsOption] Added in upgrade, so last + * and optional. See `RecoverySetsOption` for defaulting behavior. * @returns {IssuerKit} */ export const upgradeIssuerKit = ( issuerBaggage, optShutdownWithFailure = undefined, + recoverySetsOption = undefined, ) => { const issuerRecord = issuerBaggage.get(INSTANCE_KEY); const issuerZone = makeDurableZone(issuerBaggage); - return setupIssuerKit(issuerRecord, issuerZone, optShutdownWithFailure); + const oldRecoverySetsState = issuerBaggage.has(RECOVERY_SETS_STATE) + ? issuerBaggage.get(RECOVERY_SETS_STATE) + : 'hasRecoverySets'; + if ( + oldRecoverySetsState === 'noRecoverySets' && + recoverySetsOption === 'hasRecoverySets' + ) { + Fail`Cannot (yet?) upgrade from 'noRecoverySets' to 'hasRecoverySets'`; + } + if ( + oldRecoverySetsState === 'hasRecoverySets' && + recoverySetsOption === 'noRecoverySets' + ) { + Fail`Cannot (yet?) upgrade from 'hasRecoverySets' to 'noRecoverySets'`; + } + const recoverySetsState = recoverySetsOption || oldRecoverySetsState; + return setupIssuerKit( + issuerRecord, + issuerZone, + recoverySetsState, + optShutdownWithFailure, + ); }; harden(upgradeIssuerKit); @@ -119,8 +153,14 @@ export const hasIssuer = baggage => baggage.has(INSTANCE_KEY); * typically, the amount of an invitation payment is a singleton set. Such a * payment is often referred to in the singular as "an invitation".) * + * `recoverySetsOption` added in upgrade. Note that `IssuerOptionsRecord` is + * never stored, so we never need to worry about inheriting one from a + * predecessor predating the introduction of recovery sets. See + * `RecoverySetsOption` for defaulting behavior. + * * @typedef {Partial<{ * elementShape: Pattern; + * recoverySetsOption: RecoverySetsOption; * }>} IssuerOptionsRecord */ @@ -161,12 +201,24 @@ export const makeDurableIssuerKit = ( assetKind = AssetKind.NAT, displayInfo = harden({}), optShutdownWithFailure = undefined, - { elementShape = undefined } = {}, + { elementShape = undefined, recoverySetsOption = undefined } = {}, ) => { - const issuerData = harden({ name, assetKind, displayInfo, elementShape }); + const issuerData = harden({ + name, + assetKind, + displayInfo, + elementShape, + }); issuerBaggage.init(INSTANCE_KEY, issuerData); const issuerZone = makeDurableZone(issuerBaggage); - return setupIssuerKit(issuerData, issuerZone, optShutdownWithFailure); + const recoverySetsState = recoverySetsOption || 'hasRecoverySets'; + issuerBaggage.init(RECOVERY_SETS_STATE, recoverySetsState); + return setupIssuerKit( + issuerData, + issuerZone, + recoverySetsState, + optShutdownWithFailure, + ); }; harden(makeDurableIssuerKit); @@ -210,12 +262,19 @@ export const prepareIssuerKit = ( options = {}, ) => { if (hasIssuer(issuerBaggage)) { - const { elementShape: _ = undefined } = options; - const issuerKit = upgradeIssuerKit(issuerBaggage, optShutdownWithFailure); + const { elementShape: _ = undefined, recoverySetsOption = undefined } = + options; + const issuerKit = upgradeIssuerKit( + issuerBaggage, + optShutdownWithFailure, + recoverySetsOption, + ); // TODO check consistency with name, assetKind, displayInfo, elementShape. // Consistency either means that these are the same, or that they differ - // in a direction we are prepared to upgrade. + // in a direction we are prepared to upgrade. Note that it is the + // responsibility of `upgradeIssuerKit` to check consistency of + // `recoverySetsOption`, so continue to not do that here. // @ts-expect-error Type parameter confusion. return issuerKit; @@ -273,7 +332,7 @@ export const makeIssuerKit = ( assetKind = AssetKind.NAT, displayInfo = harden({}), optShutdownWithFailure = undefined, - { elementShape = undefined } = {}, + { elementShape = undefined, recoverySetsOption = undefined } = {}, ) => makeDurableIssuerKit( makeScalarBigMapStore('dropped issuer kit', { durable: true }), @@ -281,6 +340,6 @@ export const makeIssuerKit = ( assetKind, displayInfo, optShutdownWithFailure, - { elementShape }, + { elementShape, recoverySetsOption }, ); harden(makeIssuerKit); diff --git a/packages/ERTP/src/paymentLedger.js b/packages/ERTP/src/paymentLedger.js index bfa57b74f53..cf20111f35e 100644 --- a/packages/ERTP/src/paymentLedger.js +++ b/packages/ERTP/src/paymentLedger.js @@ -72,6 +72,7 @@ const amountShapeFromElementShape = (brand, assetKind, elementShape) => { * @param {K} assetKind * @param {DisplayInfo} displayInfo * @param {Pattern} elementShape + * @param {RecoverySetsOption} recoverySetsState * @param {ShutdownWithFailure} [optShutdownWithFailure] * @returns {PaymentLedger} */ @@ -81,6 +82,7 @@ export const preparePaymentLedger = ( assetKind, displayInfo, elementShape, + recoverySetsState, optShutdownWithFailure = undefined, ) => { /** @type {Brand} */ @@ -141,11 +143,11 @@ export const preparePaymentLedger = ( }); /** - * A withdrawn live payment is associated with the recovery set of the purse - * it was withdrawn from. Let's call these "recoverable" payments. All - * recoverable payments are live, but not all live payments are recoverable. - * We do the bookkeeping for payment recovery with this weakmap from - * recoverable payments to the recovery set they are in. A bunch of + * A (non-empty) withdrawn live payment is associated with the recovery set of + * the purse it was withdrawn from. Let's call these "recoverable" payments. + * All recoverable payments are live, but not all live payments are + * recoverable. We do the bookkeeping for payment recovery with this weakmap + * from recoverable payments to the recovery set they are in. A bunch of * interesting invariants here: * * - Every payment that is a key in the outer `paymentRecoverySets` weakMap is @@ -157,6 +159,9 @@ export const preparePaymentLedger = ( * - A purse's recovery set only contains payments withdrawn from that purse and * not yet consumed. * + * If `recoverySetsState === 'noRecoverySets'`, then nothing should ever be + * added to this WeakStore. + * * @type {WeakMapStore>} */ const paymentRecoverySets = issuerZone.weakMapStore('paymentRecoverySets'); @@ -170,7 +175,11 @@ export const preparePaymentLedger = ( * @param {SetStore} [optRecoverySet] */ const initPayment = (payment, amount, optRecoverySet = undefined) => { - if (optRecoverySet !== undefined) { + if (recoverySetsState === 'noRecoverySets') { + optRecoverySet === undefined || + Fail`when recoverSetsState === 'noRecoverySets', optRecoverySet must be empty`; + } + if (optRecoverySet !== undefined && !AmountMath.isEmpty(amount)) { optRecoverySet.add(payment); paymentRecoverySets.init(payment, optRecoverySet); } @@ -263,10 +272,10 @@ export const preparePaymentLedger = ( * * @param {import('./amountStore.js').AmountStore} balanceStore * @param {Amount} amount - the amount to be withdrawn - * @param {SetStore} recoverySet + * @param {SetStore} [recoverySet] * @returns {Payment} */ - const withdrawInternal = (balanceStore, amount, recoverySet) => { + const withdrawInternal = (balanceStore, amount, recoverySet = undefined) => { amount = coerce(amount); const payment = makePayment(); // COMMIT POINT Move the withdrawn assets from this purse into @@ -294,6 +303,8 @@ export const preparePaymentLedger = ( depositInternal, withdrawInternal, }), + recoverySetsState, + paymentRecoverySets, ); /** @type {Issuer} */ diff --git a/packages/ERTP/src/purse.js b/packages/ERTP/src/purse.js index e8cafa50fa5..11dda7655af 100644 --- a/packages/ERTP/src/purse.js +++ b/packages/ERTP/src/purse.js @@ -1,10 +1,12 @@ -import { M } from '@agoric/store'; +import { M, makeCopySet } from '@agoric/store'; import { AmountMath } from './amountMath.js'; import { makeTransientNotifierKit } from './transientNotifier.js'; import { makeAmountStore } from './amountStore.js'; const { Fail } = assert; +const EMPTY_COPY_SET = makeCopySet([]); + // TODO Type InterfaceGuard better than InterfaceGuard /** * @param {import('@agoric/zone').Zone} issuerZone @@ -19,6 +21,8 @@ const { Fail } = assert; * depositInternal: any; * withdrawInternal: any; * }} purseMethods + * @param {RecoverySetsOption} recoverySetsState + * @param {WeakMapStore>} paymentRecoverySets */ export const preparePurseKind = ( issuerZone, @@ -27,6 +31,8 @@ export const preparePurseKind = ( brand, PurseIKit, purseMethods, + recoverySetsState, + paymentRecoverySets, ) => { const amountShape = brand.getAmountShape(); @@ -35,6 +41,34 @@ export const preparePurseKind = ( // TODO propagate zonifying to notifiers, maybe? const { provideNotifier, update: updateBalance } = makeTransientNotifierKit(); + /** + * If `recoverySetsState === 'hasRecoverySets'` (the normal state), then just + * return `state.recoverySet`. + * + * If `recoverySetsState === 'noRecoverySets'`, return `undefined`. Callers + * must be aware that the `undefined` return happens iff `recoverySetsState + * === 'noRecoverySets'`, and to avoid storing or retrieving anything from the + * actual recovery set. + * + * @param {{ recoverySet: SetStore }} state + * @returns {SetStore | undefined} + */ + const maybeRecoverySet = state => { + const { recoverySet } = state; + if (recoverySetsState === 'hasRecoverySets') { + return recoverySet; + } else { + recoverySetsState === 'noRecoverySets' || + Fail`recoverSetsState must be noRecoverySets if it isn't hasRecoverSets`; + paymentRecoverySets !== undefined || + Fail`paymentRecoverySets must always be defined`; + recoverySet.getSize() === 0 || + Fail`With noRecoverySets, recoverySet must be empty`; + + return undefined; + } + }; + // - This kind is a pair of purse and depositFacet that have a 1:1 // correspondence. // - They are virtualized together to share a single state record. @@ -76,12 +110,14 @@ export const preparePurseKind = ( withdraw(amount) { const { state } = this; const { purse } = this.facets; + + const optRecoverySet = maybeRecoverySet(state); const balanceStore = makeAmountStore(state, 'currentBalance'); // Note COMMIT POINT within withdraw. const payment = withdrawInternal( balanceStore, amount, - state.recoverySet, + optRecoverySet, ); updateBalance(purse, balanceStore.getAmount()); return payment; @@ -103,18 +139,27 @@ export const preparePurseKind = ( }, getRecoverySet() { - return this.state.recoverySet.snapshot(); + const { state } = this; + const optRecoverySet = maybeRecoverySet(state); + if (optRecoverySet === undefined) { + return EMPTY_COPY_SET; + } + return optRecoverySet.snapshot(); }, recoverAll() { const { state, facets } = this; let amount = AmountMath.makeEmpty(brand, assetKind); - for (const payment of state.recoverySet.keys()) { + const optRecoverySet = maybeRecoverySet(state); + if (optRecoverySet === undefined) { + return amount; // empty at this time + } + for (const payment of optRecoverySet.keys()) { // This does cause deletions from the set while iterating, // but this special case is allowed. const delta = facets.purse.deposit(payment); amount = AmountMath.add(amount, delta, brand); } - state.recoverySet.getSize() === 0 || + optRecoverySet.getSize() === 0 || Fail`internal: Remaining unrecovered payments: ${facets.purse.getRecoverySet()}`; return amount; }, diff --git a/packages/ERTP/src/types-ambient.js b/packages/ERTP/src/types-ambient.js index 59c4388b7d0..46b1ea472aa 100644 --- a/packages/ERTP/src/types-ambient.js +++ b/packages/ERTP/src/types-ambient.js @@ -173,8 +173,9 @@ * @template {AssetKind} [K=AssetKind] * @typedef {object} PaymentLedger * @property {Mint} mint - * @property {Purse} mintRecoveryPurse Useful only to get the recovery set - * associated with minted payments that are still live. + * @property {Purse} mintRecoveryPurse Externally useful only if this issuer + * uses recovery sets. Can be used to get the recovery set associated with + * minted payments that are still live. * @property {Issuer} issuer * @property {Brand} brand */ @@ -183,8 +184,9 @@ * @template {AssetKind} [K=AssetKind] * @typedef {object} IssuerKit * @property {Mint} mint - * @property {Purse} mintRecoveryPurse Useful only to get the recovery set - * associated with minted payments that are still live. + * @property {Purse} mintRecoveryPurse Externally useful only if this issuer + * uses recovery sets. Can be used to get the recovery set associated with + * minted payments that are still live. * @property {Issuer} issuer * @property {Brand} brand * @property {DisplayInfo} displayInfo @@ -217,6 +219,24 @@ // /////////////////////////// Purse / Payment ///////////////////////////////// +/** + * Issuers first became durable with mandatory recovery sets. Later they were + * made optional, but there is no support for converting from one state to the + * other. Thus, absence of a `RecoverySetsOption` state is equivalent to + * `'hasRecoverySets'`. In the absence of a `recoverySetsOption` parameter, + * upgradeIssuerKit defaults to the predecessor's `RecoverySetsOption` state, or + * `'hasRecoverySets'` if none. + * + * At this time, issuers started in one of the states (`'noRecoverySets'`, or + * `'hasRecoverySets'`) cannot be converted to the other on upgrade. If this + * transition is needed, it can likely be supported in a future upgrade. File an + * issue on github and explain what you need and why. + * + * @typedef {'hasRecoverySets' | 'noRecoverySets'} RecoverySetsOption + */ + +// /////////////////////////// Purse / Payment ///////////////////////////////// + /** * @callback DepositFacetReceive * @param {Payment} payment @@ -276,10 +296,14 @@ * can spend the assets at stake on other things. Afterwards, if the recipient * of the original check finally gets around to depositing it, their deposit * fails. + * + * Returns an empty set if this issuer does not support recovery sets. * @property {() => Amount} recoverAll For use in emergencies, such as coming * back from a traumatic crash and upgrade. This deposits all the payments in * this purse's recovery set into the purse itself, returning the total amount * of assets recovered. + * + * Returns an empty amount if this issuer does not support recovery sets. */ /** diff --git a/packages/inter-protocol/src/price/fluxAggregatorContract.js b/packages/inter-protocol/src/price/fluxAggregatorContract.js index d8050c26a18..84f6b8eb17d 100644 --- a/packages/inter-protocol/src/price/fluxAggregatorContract.js +++ b/packages/inter-protocol/src/price/fluxAggregatorContract.js @@ -74,6 +74,7 @@ export const start = async (zcf, privateArgs, baggage) => { 'set', undefined, undefined, + { recoverySetsOption: 'noRecoverySets' }, ); const { diff --git a/packages/zoe/src/contractSupport/priceAuthorityQuoteMint.js b/packages/zoe/src/contractSupport/priceAuthorityQuoteMint.js index 769b08643ac..46531ffc125 100644 --- a/packages/zoe/src/contractSupport/priceAuthorityQuoteMint.js +++ b/packages/zoe/src/contractSupport/priceAuthorityQuoteMint.js @@ -17,6 +17,7 @@ export const provideQuoteMint = baggage => { AssetKind.SET, undefined, undefined, + { recoverySetsOption: 'noRecoverySets' }, ); return issuerKit.mint; };