Merge pull request #7139 from Agoric/ta/remove-mapToRecord

use entries to represent maps in vstorage

packages/agoric-cli/src/commands/vaults.js

@@ -43,9 +43,9 @@ export const makeVaultsCommand = async logger => {
     .action(async function (opts) {
       const current = await getCurrent(opts.from, fromBoard, { vstorage });
 
-      const vaultStoragePaths = Object.values(
-        current.offerToPublicSubscriberPaths,
-      ).map(pathmap => pathmap.vault);
+      const vaultStoragePaths = current.offerToPublicSubscriberPaths.map(
+        ([_offerId, pathmap]) => pathmap.vault,
+      );
 
       for (const path of vaultStoragePaths) {
         process.stdout.write(path);

packages/agoric-cli/src/lib/format.js

@@ -155,12 +155,11 @@ export const offerStatusTuples = (state, agoricNames) => {
  */
 export const summarize = (current, coalesced, agoricNames) => {
   return {
-    lastOfferId: [current.lastOfferId],
     purses: purseBalanceTuples(
       [...current.purses.values()],
       Object.values(agoricNames.vbankAsset),
     ),
-    usedInvitations: Object.entries(current.offerToUsedInvitation).map(
+    usedInvitations: current.offerToUsedInvitation.map(
       ([offerId, invitationAmt]) => [
         agoricNames.reverse[invitationAmt.value[0].instance.boardId],
         invitationAmt.value[0].description,

packages/inter-protocol/src/clientSupport.js

@@ -136,9 +136,7 @@ const makeCloseOffer = (brands, opts, previousOffer) => {
 export const lookupOfferIdForVault = async (vaultId, currentP) => {
   const { offerToPublicSubscriberPaths } = await currentP;
 
-  for (const [offerId, publicSubscribers] of Object.entries(
-    offerToPublicSubscriberPaths,
-  )) {
+  for (const [offerId, publicSubscribers] of offerToPublicSubscriberPaths) {
     if (publicSubscribers.vault?.endsWith(vaultId)) {
       return offerId;
     }

packages/inter-protocol/test/smartWallet/test-oracle-integration.js

@@ -212,9 +212,13 @@ test.serial('invitations', async t => {
   const currentSub = E(wallet).getCurrentSubscriber();
   /** @type {import('@agoric/smart-wallet/src/smartWallet.js').CurrentWalletRecord} */
   const currentState = await headValue(currentSub);
-  t.deepEqual(Object.keys(currentState.offerToUsedInvitation), [id]);
+  t.deepEqual(
+    currentState.offerToUsedInvitation.map(([k, _]) => k),
+    [id],
+  );
+  const usedInvitations = new Map(currentState.offerToUsedInvitation);
   t.is(
-    currentState.offerToUsedInvitation[id].value[0].description,
+    usedInvitations.get(id)?.value[0].description,
     ORACLE_INVITATION_MAKERS_DESC,
   );
 });
@@ -389,12 +393,13 @@ test.serial('govern oracles list', async t => {
       1,
       'one invitation consumed, one left',
     );
-    t.deepEqual(Object.keys(currentState.offerToUsedInvitation), [
-      'acceptEcInvitationOID',
-    ]);
+    t.deepEqual(
+      currentState.offerToUsedInvitation.map(([k, _]) => k),
+      ['acceptEcInvitationOID'],
+    );
+    let usedInvitations = new Map(currentState.offerToUsedInvitation);
     t.is(
-      currentState.offerToUsedInvitation.acceptEcInvitationOID.value[0]
-        .description,
+      usedInvitations.get('acceptEcInvitationOID')?.value[0].description,
       'charter member invitation',
     );
     await offersFacet.executeOffer({
@@ -413,15 +418,13 @@ test.serial('govern oracles list', async t => {
       0,
       'last invitation consumed, none left',
     );
-    t.deepEqual(Object.keys(currentState.offerToUsedInvitation), [
-      'acceptEcInvitationOID',
-      'acceptVoterOID',
-    ]);
-    // 'acceptEcInvitationOID' tested above
-    t.is(
-      currentState.offerToUsedInvitation.acceptVoterOID.value[0].description,
-      'Voter0',
+    t.deepEqual(
+      currentState.offerToUsedInvitation.map(([k, _]) => k),
+      ['acceptEcInvitationOID', 'acceptVoterOID'],
     );
+    // 'acceptEcInvitationOID' tested above
+    usedInvitations = new Map(currentState.offerToUsedInvitation);
+    t.is(usedInvitations.get('acceptVoterOID')?.value[0].description, 'Voter0');
   }
 
   const feed = await E(agoricNames).lookup('instance', 'ATOM-USD price feed');

packages/inter-protocol/test/smartWallet/test-psm-integration.js

@@ -280,12 +280,14 @@ test('govern offerFilter', async t => {
     1,
     'one invitation consumed, one left',
   );
-  t.deepEqual(Object.keys(currentState.offerToUsedInvitation), [
-    'acceptEcInvitationOID',
-  ]);
+  t.deepEqual(
+    currentState.offerToUsedInvitation.map(([k, _]) => k),
+    ['acceptEcInvitationOID'],
+  );
+
+  let usedInvitations = new Map(currentState.offerToUsedInvitation);
   t.is(
-    currentState.offerToUsedInvitation.acceptEcInvitationOID.value[0]
-      .description,
+    usedInvitations.get('acceptEcInvitationOID')?.value[0].description,
     'charter member invitation',
   );
   const voteInvitationDetails = await getInvitationFor(
@@ -317,15 +319,13 @@ test('govern offerFilter', async t => {
     0,
     'last invitation consumed, none left',
   );
-  t.deepEqual(Object.keys(currentState.offerToUsedInvitation), [
-    'acceptEcInvitationOID',
-    'acceptVoterOID',
-  ]);
-  // acceptEcInvitationOID tested above
-  t.is(
-    currentState.offerToUsedInvitation.acceptVoterOID.value[0].description,
-    'Voter0',
+  t.deepEqual(
+    currentState.offerToUsedInvitation.map(([k, _]) => k),
+    ['acceptEcInvitationOID', 'acceptVoterOID'],
   );
+  // acceptEcInvitationOID tested above
+  usedInvitations = new Map(currentState.offerToUsedInvitation);
+  t.is(usedInvitations.get('acceptVoterOID')?.value[0].description, 'Voter0');
 
   // Call for a vote ////////////////////////////////
 

packages/smart-wallet/src/smartWallet.js

@@ -28,15 +28,6 @@ import { objectMapStoragePath } from './utils.js';
 const { Fail, quote: q } = assert;
 const { StorageNodeShape } = makeTypeGuards(M);
 
-const ERROR_LAST_OFFER_ID = -1;
-
-/**
- * @template K, V
- * @param {MapStore<K, V> } map
- * @returns {Record<K, V>}
- */
-const mapToRecord = map => Object.fromEntries(map.entries());
-
 /**
  * @file Smart wallet module
  *
@@ -56,11 +47,23 @@ const mapToRecord = map => Object.fromEntries(map.entries());
 /**
  * Purses is an array to support a future requirement of multiple purses per brand.
  *
+ * Each map is encoded as an array of entries because a Map doesn't serialize directly.
+ * We also considered having a vstorage key for each offer but for now are sticking with this design.
+ *
+ * Cons
+ *    - Reserializes previously written results when a new result is added
+ *    - Optimizes reads though writes are on-chain (~100 machines) and reads are off-chain (to 1 machine)
+ *
+ * Pros
+ *    - Reading all offer results happens much more (>100) often than storing a new offer result
+ *    - Reserialization and writes are paid in execution gas, whereas reads are not
+ *
+ * This design should be revisited if ever batch querying across vstorage keys become cheaper or reads be paid.
+ *
  * @typedef {{
  *   purses: Array<{brand: Brand, balance: Amount}>,
- *   offerToUsedInvitation: { [offerId: string]: Amount },
- *   offerToPublicSubscriberPaths: { [offerId: string]: { [subscriberName: string]: string } },
- *   lastOfferId: string,
+ *   offerToUsedInvitation: Array<[ offerId: string, usedInvitation: Amount ]>,
+ *   offerToPublicSubscriberPaths: Array<[ offerId: string, publicTopics: { [subscriberName: string]: string } ]>,
  * }} CurrentWalletRecord
  */
 
@@ -256,7 +259,6 @@ export const prepareSmartWallet = (baggage, shared) => {
     }),
     offers: M.interface('offers facet', {
       executeOffer: M.call(shape.OfferSpec).returns(M.promise()),
-      getLastOfferId: M.call().returns(M.number()),
     }),
     self: M.interface('selfFacetI', {
       handleBridgeAction: M.call(shape.StringCapData, M.boolean()).returns(
@@ -313,12 +315,10 @@ export const prepareSmartWallet = (baggage, shared) => {
               brand: a.brand,
               balance: a,
             })),
-            offerToUsedInvitation: mapToRecord(offerToUsedInvitation),
-            offerToPublicSubscriberPaths: mapToRecord(
-              offerToPublicSubscriberPaths,
-            ),
-            // @ts-expect-error FIXME leftover from offer id string conversion
-            lastOfferId: ERROR_LAST_OFFER_ID,
+            offerToUsedInvitation: [...offerToUsedInvitation.entries()],
+            offerToPublicSubscriberPaths: [
+              ...offerToPublicSubscriberPaths.entries(),
+            ],
           });
         },
 
@@ -384,13 +384,6 @@ export const prepareSmartWallet = (baggage, shared) => {
         },
       },
       offers: {
-        /**
-         * @deprecated
-         * @returns {number} an error code, for backwards compatibility with clients expecting a number
-         */
-        getLastOfferId() {
-          return ERROR_LAST_OFFER_ID;
-        },
         /**
          * Take an offer description provided in capData, augment it with payments and call zoe.offer()
          *

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

@@ -28,7 +28,6 @@ test.before(async t => {
 test('bridge handler', async t => {
   const smartWallet = await t.context.simpleProvideWallet(mockAddress1);
   const updates = await E(smartWallet).getUpdatesSubscriber();
-  const current = await E(smartWallet).getCurrentSubscriber();
 
   const ctx = makeImportContext();
 
@@ -55,10 +54,6 @@ test('bridge handler', async t => {
       value: [],
     },
   });
-  t.like(await headValue(current), {
-    // error because it's deprecated
-    lastOfferId: -1,
-  });
 
   const validMsg = {
     type: ActionType.WALLET_SPEND_ACTION,