Skip to content

Commit

Permalink
feat(ertp): review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Oct 3, 2023
1 parent ee41ba5 commit a63fa4c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 12 deletions.
18 changes: 9 additions & 9 deletions packages/ERTP/src/issuerKit.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ harden(setupIssuerKit);
const INSTANCE_KEY = 'issuer';
/**
* The key at which the issuerKit's `RecoverySetsOption` state is stored.
* Introduced by an upgrade, so may be absent on an ancestor. See
* 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 an ancestor issuerKit. Use `makeDurableIssuerKit` to
* Used _only_ to upgrade a predecessor issuerKit. Use `makeDurableIssuerKit` to
* make a new one.
*
* @template {AssetKind} K
Expand Down Expand Up @@ -135,7 +135,7 @@ harden(upgradeIssuerKit);

/**
* Confusingly, `prepareIssuerKit` was the original name for `upgradeIssuerKit`,
* even though it is used only to upgrade an ancestor issuerKit. Use
* even though it is used only to upgrade a predecessor issuerKit. Use
* `makeDurableIssuerKit` to make a new one.
*
* @deprecated Use `upgradeIssuerKit` instead if that's what you want. Or
Expand All @@ -161,9 +161,9 @@ export const hasIssuer = baggage => baggage.has(INSTANCE_KEY);
* 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 an ancestor
* predating the introduction of recovery sets. See `RecoverySetsOption` for
* defaulting behavior.
* 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;
Expand Down Expand Up @@ -229,9 +229,9 @@ export const makeDurableIssuerKit = (
harden(makeDurableIssuerKit);

/**
* What _should_ have been named `prepareIssuerKit`. Used to either revive an
* ancestor issuer kit, or to make a new durable if it absent, and to place it
* in baggage for the next successor.
* What _should_ have been named `prepareIssuerKit`. Used to either revive a
* predecessor issuerKit, or to make a new durable one if it is absent, and to
* place it in baggage for the next successor.
*
* @template {AssetKind} K The name becomes part of the brand in asset
* descriptions. The name is useful for debugging and double-checking
Expand Down
11 changes: 11 additions & 0 deletions packages/ERTP/src/paymentLedger.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ export const preparePaymentLedger = (
}

/**
* An issuer may choose to omit recovery sets. The quote issuer, for example,
* omits recovery sets since its "payments" do not actually represent
* transferable value. Recovery sets preverse payments that are otherwise
* inaccessible, so that they can be recovered. This is their point. But this
* means that useless payments might accumulate into a large storage leak.
* Quote payments should never need to be recovered, since one can always ask
* for a more recent quote. Thus, they should not pay the cost of this
* potential storage leak.
*
* For an issuerKit that has not opted out of recovery sets...
*
* 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.
Expand Down
6 changes: 3 additions & 3 deletions packages/ERTP/src/types-ambient.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,19 +224,19 @@
* Issuers first became durable with recovery sets and no option to suppress
* them. Thus, absence of a `RecoverySetsOption` state is equivalent to
* `'hasRecoverySets'`. By contrast, the absence of `RecoverySetsOption` provide
* parameter defaults to the ancestor's `RecoverySetsOption` state, or
* parameter defaults to the predecessor's `RecoverySetsOption` state, or
* `'hasRecoverySets'` if none.
*
* The `'noRecoverySets'` state, if used for the first incarnation, makes an
* issuer without recovery sets. If used for a successor incarnation, no matter
* whether the ancestor was `'hasRecoverySets'` or `'noRecoverySets'`,
* whether the predecessor was `'hasRecoverySets'` or `'noRecoverySets'`,
*
* - will start emptying recovery sets,
* - will prevent any new payments from being added to recovery sets,
* - and (controversially) will not provide access via recovery sets of any
* payments that have not yet been emptied out.
*
* At this time, a `'noRecoverySets'` ancestor cannot be upgraded to a
* At this time, a `'noRecoverySets'` predecessor cannot be upgraded to a
* `'hasRecoverySets'` successor. If it turns out this transition is needed, it
* can likely be supported in a future upgrade.
*
Expand Down

0 comments on commit a63fa4c

Please sign in to comment.