From 8c1ab7c819e949b7d6e3b2f80980d61b76a6dd05 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Fri, 15 Dec 2023 15:02:03 -0800 Subject: [PATCH] chore: clean-ups from review --- .../build-wallet-factory2-upgrade.js} | 2 +- packages/smart-wallet/package.json | 1 - packages/smart-wallet/src/smartWallet.js | 37 +++++++++++-------- packages/smart-wallet/src/walletFactory.js | 18 ++++++--- packages/smart-wallet/test/test-addAsset.js | 21 +++++------ packages/vats/package.json | 1 + 6 files changed, 46 insertions(+), 34 deletions(-) rename packages/{smart-wallet/src/proposals/upgrade-wallet-factory2.js => builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js} (92%) diff --git a/packages/smart-wallet/src/proposals/upgrade-wallet-factory2.js b/packages/builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js similarity index 92% rename from packages/smart-wallet/src/proposals/upgrade-wallet-factory2.js rename to packages/builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js index a58f856ca442..bb2ed3bce756 100644 --- a/packages/smart-wallet/src/proposals/upgrade-wallet-factory2.js +++ b/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. */ diff --git a/packages/smart-wallet/package.json b/packages/smart-wallet/package.json index 883c0ce0898b..ddf9ae13c392 100644 --- a/packages/smart-wallet/package.json +++ b/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", diff --git a/packages/smart-wallet/src/smartWallet.js b/packages/smart-wallet/src/smartWallet.js index 07489b59019b..d26ae50200e3 100644 --- a/packages/smart-wallet/src/smartWallet.js +++ b/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; diff --git a/packages/smart-wallet/src/walletFactory.js b/packages/smart-wallet/src/walletFactory.js index 6c91ec9a9f04..58d8aa0b0675 100644 --- a/packages/smart-wallet/src/walletFactory.js +++ b/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); } diff --git a/packages/smart-wallet/test/test-addAsset.js b/packages/smart-wallet/test/test-addAsset.js index 7c423626a154..87f226af64f1 100644 --- a/packages/smart-wallet/test/test-addAsset.js +++ b/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 diff --git a/packages/vats/package.json b/packages/vats/package.json index 7559949ca84a..e7f7ece54a18 100644 --- a/packages/vats/package.json +++ b/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",