feat(smartWallet): defer deposits until purse available (#6172)

* feat(smartWallet): defer deposits until purse available * feat(smartWallet): payouts deposit eagerly and defer if necessary * fixup! feat(smartWallet): defer deposits until purse available Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Agoric · Sep 10, 2022 · 1a1cc41 · 1a1cc41
1 parent 9a2b427
commit 1a1cc41
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 31 deletions.
diff --git a/packages/smart-wallet/src/offers.js b/packages/smart-wallet/src/offers.js
@@ -28,6 +28,7 @@ export const UNPUBLISHED_RESULT = 'UNPUBLISHED';
 /**
  * @param {object} opts
  * @param {ERef<ZoeService>} opts.zoe
+ * @param {{ receive: (payment: *) => Promise<Amount> }} opts.depositFacet
  * @param {object} opts.powers
  * @param {import('./types').Cell<number>} opts.powers.lastOfferId
  * @param {(spec: import('./invitations').InvitationSpec) => ERef<Invitation>} opts.powers.invitationFromSpec
@@ -37,6 +38,7 @@ export const UNPUBLISHED_RESULT = 'UNPUBLISHED';
  */
 export const makeOfferExecutor = ({
   zoe,
+  depositFacet,
   powers,
   onStatusChange,
   onNewContinuingOffer,
@@ -52,7 +54,7 @@ export const makeOfferExecutor = ({
      * @throws if any parts of the offer can be determined synchronously to be invalid
      */
     async executeOffer(offerSpec) {
-      const paymentsManager = makePaymentsHelper(purseForBrand);
+      const paymentsManager = makePaymentsHelper(purseForBrand, depositFacet);
 
       /** @type {OfferStatus} */
       let status = {
@@ -144,8 +146,8 @@ export const makeOfferExecutor = ({
         E.when(
           E(seatRef).getPayouts(),
           payouts =>
-            paymentsManager.depositPayouts(payouts).then(amounts => {
-              updateStatus({ payouts: amounts });
+            paymentsManager.depositPayouts(payouts).then(amountsOrDeferred => {
+              updateStatus({ payouts: amountsOrDeferred });
             }),
           handleError,
         );

diff --git a/packages/smart-wallet/src/payments.js b/packages/smart-wallet/src/payments.js
@@ -7,8 +7,9 @@ import { E } from '@endo/far';
  * Used in an offer execution to manage payments state safely.
  *
  * @param {(brand: Brand) => import('./types').RemotePurse} purseForBrand
+ * @param {{ receive: (payment: *) => Promise<Amount> }} depositFacet
  */
-export const makePaymentsHelper = purseForBrand => {
+export const makePaymentsHelper = (purseForBrand, depositFacet) => {
   /** @type {PaymentPKeywordRecord | null} */
   let keywordPaymentPromises = null;
 
@@ -74,20 +75,14 @@ export const makePaymentsHelper = purseForBrand => {
       );
     },
 
-    // TODO(PS0?) when there's not a purse for a brand, hold the payout and wait for a purse to deposit it into
-    // Cheaper alternative: before offer validate we have issuers for all the 'wants' so the results can be put into purses.
     /**
-     *
      * @param {PaymentPKeywordRecord} payouts
-     * @returns {Promise<AmountKeywordRecord>}
+     * @returns {Promise<AmountKeywordRecord>} amounts for deferred deposits will be empty
      */
     async depositPayouts(payouts) {
-      /** @type {PaymentKeywordRecord} */
-      // @ts-expect-error ???
-      const paymentKeywordRecord = await deeplyFulfilledObject(payouts);
       /** Record<string, Promise<Amount>> */
-      const amountPKeywordRecord = objectMap(paymentKeywordRecord, payment =>
-        E(purseForBrand(payment.getAllegedBrand())).deposit(payment),
+      const amountPKeywordRecord = objectMap(payouts, paymentRef =>
+        E.when(paymentRef, payment => depositFacet.receive(payment)),
       );
       return deeplyFulfilledObject(amountPKeywordRecord);
     },

diff --git a/packages/smart-wallet/src/smartWallet.js b/packages/smart-wallet/src/smartWallet.js
@@ -1,5 +1,5 @@
 // @ts-check
-import { AmountShape, PaymentShape } from '@agoric/ertp';
+import { AmountMath, AmountShape, PaymentShape } from '@agoric/ertp';
 import { isNat } from '@agoric/nat';
 import {
   makeStoredPublishKit,
@@ -75,6 +75,7 @@ const { details: X, quote: q } = assert;
  * @typedef {Parameters<initState>[0] & Parameters<initState>[1]} HeldParams
  *
  * @typedef {Readonly<HeldParams & {
+ * paymentQueues: MapStore<Brand, Array<import('@endo/far').FarRef<Payment>>>,
  * offerToInvitationMakers: MapStore<number, import('./types').RemoteInvitationMakers>,
  * brandDescriptors: MapStore<Brand, BrandDescriptor>,
  * brandPurses: MapStore<Brand, RemotePurse>,
@@ -120,14 +121,19 @@ export const initState = (unique, shared) => {
   );
 
   const preciousState = {
+    // Private purses. This assumes one purse per brand, which will be valid in MN-1 but not always.
+    brandPurses: makeScalarBigMapStore('brand purses', { durable: true }),
+    // Payments that couldn't be deposited when received.
+    // NB: vulnerable to uncapped growth by unpermissioned deposits.
+    paymentQueues: makeScalarBigMapStore('payments queues', {
+      durable: true,
+    }),
     // Invitation makers yielded by offer results
     offerToInvitationMakers: makeScalarBigMapStore('invitation makers', {
       durable: true,
     }),
     // What purses have reported on construction and by getCurrentAmountNotifier updates.
     purseBalances: makeScalarMapStore(),
-    // Private purses. This assumes one purse per brand, which will be valid in MN-1 but not always.
-    brandPurses: makeScalarBigMapStore('brand purses', { durable: true }),
   };
 
   const nonpreciousState = {
@@ -202,8 +208,13 @@ const behavior = {
     /** @type {(desc: Omit<BrandDescriptor, 'displayInfo'>, purse: RemotePurse) => Promise<void>} */
     async addBrand(desc, purseRef) {
       /** @type {State} */
-      const { address, brandDescriptors, brandPurses, updatePublishKit } =
-        this.state;
+      const {
+        address,
+        brandDescriptors,
+        brandPurses,
+        paymentQueues,
+        updatePublishKit,
+      } = this.state;
       // assert haven't received this issuer before.
       const descriptorsHas = brandDescriptors.has(desc.brand);
       const pursesHas = brandPurses.has(desc.brand);
@@ -248,30 +259,52 @@ const behavior = {
       });
 
       updatePublishKit.publisher.publish({ updated: 'brand', descriptor });
+
+      // deposit queued payments
+      const payments = paymentQueues.has(desc.brand)
+        ? paymentQueues.get(desc.brand)
+        : [];
+      const deposits = payments.map(p =>
+        // @ts-expect-error deposit does take a FarRef<Payment>
+        E(purse).deposit(p),
+      );
+      Promise.all(deposits).catch(err =>
+        console.error('ERROR depositing queued payments', err),
+      );
     },
   },
   /**
    * Similar to {DepositFacet} but async because it has to look up the purse.
    */
-  // TODO(PS0) decide whether to match canonical `DepositFacet'. it would have to take a local Payment.
+  // TODO(PS0) decide whether to match canonical `DepositFacet'. it would have to take a local Payment
   deposit: {
     /**
-     * Put the assets from the payment into the appropriate purse
+     * Put the assets from the payment into the appropriate purse.
+     *
+     * If the purse doesn't exist, we hold the payment until it does.
      *
      * @param {import('@endo/far').FarRef<Payment>} payment
-     * @returns {Promise<Amount>}
-     * @throws if the purse doesn't exist
-     * NB: the previous smart wallet contract would try again each time there's a new issuer.
-     * This version does not: 1) for expedience, 2: to avoid resource exhaustion vulnerability.
+     * @returns {Promise<Amount>} amounts for deferred deposits will be empty
      */
     async receive(payment) {
       /** @type {State} */
-      const { brandPurses } = this.state;
+      const { brandPurses, paymentQueues: queues } = this.state;
       const brand = await E(payment).getAllegedBrand();
-      const purse = brandPurses.get(brand);
 
-      // @ts-expect-error deposit does take a FarRef<Payment>
-      return E(purse).deposit(payment);
+      // When there is a purse deposit into it
+      if (brandPurses.has(brand)) {
+        const purse = brandPurses.get(brand);
+        // @ts-expect-error deposit does take a FarRef<Payment>
+        return E(purse).deposit(payment);
+      }
+
+      // When there is no purse, queue the payment
+      if (queues.has(brand)) {
+        queues.get(brand).push(payment);
+      } else {
+        queues.init(brand, harden([payment]));
+      }
+      return AmountMath.makeEmpty(brand);
     },
   },
   offers: {
@@ -292,7 +325,7 @@ const behavior = {
      * @throws if any parts of the offer can be determined synchronously to be invalid
      */
     async executeOffer(offerSpec) {
-      const { state } = this;
+      const { facets, state } = this;
       const {
         zoe,
         brandPurses,
@@ -305,6 +338,7 @@ const behavior = {
 
       const executor = makeOfferExecutor({
         zoe,
+        depositFacet: facets.deposit,
         powers: {
           invitationFromSpec: makeInvitationsHelper(
             zoe,
@@ -388,7 +422,7 @@ const finish = ({ state, facets }) => {
     /** @type {RemotePurse} */ (invitationPurse),
   );
   // watch the bank for new issuers to make purses out of
-  observeIteration(E(bank).getAssetSubscription(), {
+  void observeIteration(E(bank).getAssetSubscription(), {
     async updateState(desc) {
       /** @type {RemotePurse} */
       // @ts-expect-error cast to RemotePurse

diff --git a/packages/smart-wallet/test/test-psm-integration.js b/packages/smart-wallet/test/test-psm-integration.js
@@ -1,7 +1,7 @@
 // @ts-check
 import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';
 
-import { AmountMath } from '@agoric/ertp';
+import { AmountMath, makeIssuerKit } from '@agoric/ertp';
 import { buildRootObject as buildPSMRootObject } from '@agoric/vats/src/core/boot-psm.js';
 import '@agoric/vats/src/core/types.js';
 import { Stable } from '@agoric/vats/src/tokens.js';
@@ -14,6 +14,7 @@ import { E } from '@endo/far';
 import { NonNullish } from '@agoric/assert';
 import { coalesceUpdates } from '../src/utils.js';
 import { makeDefaultTestContext } from './contexts.js';
+import { withAmountUtils } from './supports.js';
 
 /**
  * @type {import('ava').TestFn<Awaited<ReturnType<makeDefaultTestContext>>
@@ -258,6 +259,18 @@ test.skip('govern offerFilter', async t => {
   });
 });
 
+test('deposit unknown brand', async t => {
+  const rial = withAmountUtils(makeIssuerKit('rial'));
+  assert(rial.mint);
+
+  const wallet = await t.context.simpleProvideWallet('agoric1queue');
+
+  const payment = rial.mint.mintPayment(rial.make(1_000n));
+  const result = await wallet.getDepositFacet().receive(harden(payment));
+  // successful request but not deposited
+  t.deepEqual(result, { brand: rial.brand, value: 0n });
+});
+
 test.todo('bad offer schema');
 test.todo('not enough funds');
 test.todo(