Skip to content

Commit

Permalink
chore: clean-ups from review
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris-Hibbert committed Jan 4, 2024
1 parent 73bed28 commit 7c1d677
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 38 deletions.
4 changes: 1 addition & 3 deletions packages/inter-protocol/src/price/roundsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -435,10 +435,8 @@ export const prepareRoundsManagerKit = baggage =>
);
}

if (status.lastReportedRound >= roundId) {
if (status.lastReportedRound >= roundId)
return 'cannot report on previous rounds';
}

if (
roundId !== reportingRoundId &&
roundId !== add(reportingRoundId, 1) &&
Expand Down
37 changes: 22 additions & 15 deletions packages/smart-wallet/src/smartWallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,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()),
}),

Expand Down Expand Up @@ -566,14 +566,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;
Expand Down Expand Up @@ -1014,22 +1016,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;

Expand Down
18 changes: 13 additions & 5 deletions packages/smart-wallet/src/walletFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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);
}
Expand Down
21 changes: 9 additions & 12 deletions packages/smart-wallet/test/test-addAsset.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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);
Expand Down Expand Up @@ -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: {
Expand All @@ -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(', '));

Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions packages/vats/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"license": "Apache-2.0",
"dependencies": {
"@agoric/assert": "^0.6.1-u11wf.0",
"@agoric/deploy-script-support": "^0.10.4-u13.0",
"@agoric/ertp": "^0.16.3-u13.0",
"@agoric/governance": "^0.10.4-u13.0",
"@agoric/inter-protocol": "^0.16.2-u13.0",
Expand All @@ -46,10 +47,13 @@
"@endo/init": "0.5.56",
"@endo/marshal": "0.8.5",
"@endo/nat": "4.1.27",
"@endo/patterns": "^0.2.2",
"@endo/promise-kit": "0.2.56",
"import-meta-resolve": "^2.2.1",
"jessie.js": "^0.3.2"
},
"devDependencies": {
"@agoric/vats": "^0.15.2-u13.0",
"@agoric/cosmic-swingset": "^0.42.0-u13.0",
"@agoric/deploy-script-support": "^0.10.4-u13.0",
"@agoric/smart-wallet": "^0.5.4-u13.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
5 changes: 3 additions & 2 deletions packages/vats/test/bootstrapTests/test-vaults-integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ test.after.always(t => {
test('metrics path', async t => {
const { EV } = t.context.runUtils;
// example of awaitVatObject
const vaultFactoryKit =
await EV.vat('bootstrap').consumeItem('vaultFactoryKit');
const vaultFactoryKit = await EV.vat('bootstrap').consumeItem(
'vaultFactoryKit',
);
const vfTopics = await EV(vaultFactoryKit.publicFacet).getPublicTopics();
const vfMetricsPath = await EV.get(vfTopics.metrics).storagePath;
t.is(vfMetricsPath, 'published.vaultFactory.metrics');
Expand Down

0 comments on commit 7c1d677

Please sign in to comment.