chore: clean-ups from review

packages/smart-wallet/src/proposals/upgrade-wallet-factory2.js → packages/builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js

@@ -2,7 +2,7 @@ import { makeHelpers } from '@agoric/deploy-script-support';
 
 /**
  * @file
- *   `agoric run scripts/vats/upgrade-wallet-factory2.js | tee run-report.txt`
+ *   `agoric run scripts/smart-wallet/build-wallet-factory2-upgrade.js`
  * produces a proposal and permit file, as well as the necessary bundles. It
  * also prints helpful instructions for copying the files and installing them.
  */

packages/smart-wallet/package.json

@@ -27,7 +27,6 @@
   "dependencies": {
     "@agoric/assert": "^0.6.0",
     "@agoric/casting": "^0.4.2",
-    "@agoric/deploy-script-support": "^0.10.3",
     "@agoric/ertp": "^0.16.2",
     "@agoric/internal": "^0.3.2",
     "@agoric/notifier": "^0.6.2",

packages/smart-wallet/src/smartWallet.js

@@ -367,7 +367,7 @@ export const prepareSmartWallet = (baggage, shared) => {
       purseForBrand: M.call(BrandShape).returns(M.promise()),
       logWalletInfo: M.call(M.any()).returns(),
       logWalletError: M.call(M.any()).returns(),
-      // XXX is there a better guard for a bigMapStore?
+      // XXX is there a tighter guard for a bigMapStore than M.any()?
       getLiveOfferPayments: M.call().returns(M.any()),
     }),
 
@@ -561,14 +561,16 @@ export const prepareSmartWallet = (baggage, shared) => {
           return purse;
         },
 
-        // see https://github.com/Agoric/agoric-sdk/issues/8445 and
-        // https://github.com/Agoric/agoric-sdk/issues/8286. As originally
-        // released, the smartWallet didn't durably monitor the promises for the
-        // outcomes of offers, and would have dropped them on upgrade of Zoe or
-        // the smartWallet itself. Using watchedPromises, (see offerWatcher.js)
-        // we've addressed the problem for new offers. The function will
-        // backfill the solution for offers that were outstanding before the
-        // transition to incarnation 2 of the smartWallet.
+        /**
+         * see https://github.com/Agoric/agoric-sdk/issues/8445 and
+         * https://github.com/Agoric/agoric-sdk/issues/8286. As originally
+         * released, the smartWallet didn't durably monitor the promises for the
+         * outcomes of offers, and would have dropped them on upgrade of Zoe or
+         * the smartWallet itself. Using watchedPromises, (see offerWatcher.js)
+         * we've addressed the problem for new offers. This function will
+         * backfill the solution for offers that were outstanding before the
+         * transition to incarnation 2 of the smartWallet.
+         */
         async repairUnwatchedSeats() {
           const { state, facets } = this;
           const { address, invitationPurse } = state;
@@ -1009,22 +1011,27 @@ export const prepareSmartWallet = (baggage, shared) => {
         },
         getPublicTopics() {
           const { state } = this;
+          const { currentRecorderKit, updateRecorderKit } = state;
 
           return harden({
             current: {
               description: 'Current state of wallet',
-              subscriber: state.currentRecorderKit.subscriber,
-              storagePath: state.currentRecorderKit.recorder.getStoragePath(),
+              subscriber: currentRecorderKit.subscriber,
+              storagePath: currentRecorderKit.recorder.getStoragePath(),
             },
             updates: {
               description: 'Changes to wallet',
-              subscriber: state.updateRecorderKit.subscriber,
-              storagePath: state.updateRecorderKit.recorder.getStoragePath(),
+              subscriber: updateRecorderKit.subscriber,
+              storagePath: updateRecorderKit.recorder.getStoragePath(),
             },
           });
         },
-        // one-time use function. Remove this and repairUnwatchedSeats once the
-        // repair has taken place.
+        /**
+         * one-time use function. Remove this and repairUnwatchedSeats once the
+         * repair has taken place.
+         *
+         * @param {object} key
+         */
         repairWalletForIncarnation2(key) {
           const { facets } = this;
 

packages/smart-wallet/src/walletFactory.js

@@ -2,6 +2,9 @@
  * @file Wallet Factory
  *
  * Contract to make smart wallets.
+ *
+ * Note: The upgrade test uses a slightly modified copy of this file. When the
+ * interface changes here, that will also need to change.
  */
 
 import { makeTracer, WalletName } from '@agoric/internal';
@@ -223,11 +226,13 @@ export const prepare = async (zcf, privateArgs, baggage) => {
 
   const registry = makeAssetRegistry(assetPublisher);
 
-  // An object known only to walletFactory and smartWallets. The WalletFactory
-  // only has the self facet for the pre-existing wallets that must be repaired.
-  // Self is too accessible, so use of the repair function requires use of a
-  // secret that clients won't have. This can be removed once the upgrade has
-  // taken place.
+  /**
+   * An object known only to walletFactory and smartWallets. The WalletFactory
+   * only has the self facet for the pre-existing wallets that must be repaired.
+   * Self is too accessible, so use of the repair function requires use of a
+   * secret that clients won't have. This can be removed once the upgrade has
+   * taken place.
+   */
   const upgradeToIncarnation2Key = harden({});
 
   const shared = harden({
@@ -255,6 +260,9 @@ export const prepare = async (zcf, privateArgs, baggage) => {
   if (!baggage.has(UPGRADE_TO_INCARNATION_TWO)) {
     trace('Wallet Factory upgrading to incarnation 2');
 
+    // This could take a while, depending on how many outstanding wallets exist.
+    // The current plan is that it will run exactly once, and inside an upgrade
+    // handler, between blocks.
     for (const wallet of walletsByAddress.values()) {
       wallet.repairWalletForIncarnation2(upgradeToIncarnation2Key);
     }

packages/smart-wallet/test/test-addAsset.js

@@ -14,7 +14,7 @@ import { makeDefaultTestContext } from './contexts.js';
 import { ActionType, headValue, makeMockTestSpace } from './supports.js';
 import { makeImportContext } from '../src/marshal-contexts.js';
 
-const { Fail, quote: q } = assert;
+const { Fail } = assert;
 
 const importSpec = spec =>
   importMetaResolve(spec, import.meta.url).then(u => new URL(u).pathname);
@@ -420,10 +420,8 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => {
 
     /** @type {import('../src/smartWallet.js').UpdateRecord} */
     const update = await headValue(updates);
-    assert(
-      update.updated === 'offerStatus',
-      `Should have had "updated":"offerStatus", had "${q(update)}"`,
-    );
+    assert(update.updated === 'offerStatus');
+    // t.log(update.status);
     t.like(update, {
       updated: 'offerStatus',
       status: {
@@ -437,7 +435,7 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => {
     const {
       status: { id, result, payouts },
     } = update;
-    // @ts-expect-error status includes payload.
+    // @ts-expect-error cast value to copyBag
     const names = payouts?.Places.value.payload.map(([name, _qty]) => name);
     t.log(id, 'result:', result, ', payouts:', names.join(', '));
 
@@ -497,15 +495,13 @@ test.serial('non-vbank asset: give before deposit', async t => {
       proposal: { give, want },
     });
     t.log('goofy client: propose to give', choices.join(', '));
-    await t.throwsAsync(
-      () => E(walletBridge).proposeOffer(ctx.fromBoard.toCapData(offer1)),
-      { message: /Withdrawal of .* failed because the purse only contained/ },
-    );
+    await E(walletBridge).proposeOffer(ctx.fromBoard.toCapData(offer1));
   };
 
   {
     const addr2 = 'agoric1player2';
     const walletUIbridge = makePromiseKit();
+    // await eventLoopIteration();
 
     const { simpleProvideWallet, consume, sendToBridge } = t.context;
     const wallet = simpleProvideWallet(addr2);
@@ -515,8 +511,9 @@ test.serial('non-vbank asset: give before deposit', async t => {
     const mockStorage = await consume.chainStorage;
     const { aPlayer } = makeScenario(t);
 
-    await aPlayer(addr2, walletUIbridge, mockStorage, sendToBridge, updates);
-    await goofyClient(mockStorage, walletUIbridge.promise);
+    aPlayer(addr2, walletUIbridge, mockStorage, sendToBridge, updates);
+    const c2 = goofyClient(mockStorage, walletUIbridge.promise);
+    await t.throwsAsync(c2, { message: /Withdrawal of {.*} failed/ });
     await eventLoopIteration();
 
     // wallet balance was also updated

packages/vats/package.json

@@ -23,6 +23,7 @@
   "license": "Apache-2.0",
   "dependencies": {
     "@agoric/assert": "^0.6.0",
+    "@agoric/deploy-script-support": "^0.10.3",
     "@agoric/ertp": "^0.16.2",
     "@agoric/governance": "^0.10.3",
     "@agoric/internal": "^0.3.2",