Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup agoricNames publishing #9480

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+58 −107
Open

cleanup agoricNames publishing #9480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b67a444
chore: remove unused publishAgoricNames
turadg Jun 10, 2024
e50344d
feat: mock chainStorage without bridge
turadg Jun 12, 2024
af9e1c1
chore: require chainStorage
turadg Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions packages/inter-protocol/test/smartWallet/boot-psm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
makeChainStorage,
noProvisioner,
produceHighPrioritySendersManager,
publishAgoricNames,
startTimerService,
} from '@agoric/vats/src/core/chain-behaviors.js';
import { makePromiseSpace } from '@agoric/vats/src/core/promise-space.js';
Expand All @@ -35,6 +34,8 @@ import {
import * as utils from '@agoric/vats/src/core/utils.js';
import { makeHeapZone } from '@agoric/zone';
import { Stable, Stake } from '@agoric/internal/src/tokens.js';
import { makeScalarBigMapStore } from '@agoric/vat-data';
import { prepareRecorderKit } from '@agoric/zoe/src/contractSupport/recorder.js';
import {
ECON_COMMITTEE_MANIFEST,
startEconomicCommittee,
Expand Down Expand Up @@ -177,6 +178,49 @@ export const ParametersShape = M.splitRecord(
},
);

/**
* @deprecated use publishAgoricNamesToChainStorage
* @param {BootstrapPowers} powers
* @param {{
* options?: {
* agoricNamesOptions?: {
* topLevel?: string[];
* };
* };
* }} config
*/
export const legacyPublishAgoricNames = async (
{ consume: { agoricNamesAdmin, board, chainStorage: rootP } },
{ options: { agoricNamesOptions } = {} } = {},
) => {
const root = await rootP;
assert(root, 'legacyPublishAgoricNames requires chainStorage');
const nameStorage = E(root).makeChildNode('agoricNames');
const marshaller = E(board).getPublishingMarshaller();

// XXX will fail upon restart, but so would the makeStoredPublishKit this is replacing
// Since we expect the bootstrap vat to be replaced instead of upgraded this should be
// fine. See {@link ./README.md bootstrap documentation} for details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is that link supposed to go? I don't see a README in this dir.

Possible alternative link:

const fakeBaggage = makeScalarBigMapStore(
'fake baggage for AgoricNames kinds',
);
const makeRecorderKit = prepareRecorderKit(fakeBaggage, marshaller);

// brand, issuer, ...
const { topLevel = Object.keys(agoricNamesReserved) } =
agoricNamesOptions || {};
await Promise.all(
topLevel.map(async kind => {
const kindAdmin = await E(agoricNamesAdmin).lookupAdmin(kind);

const kindNode = await E(nameStorage).makeChildNode(kind);
const { recorder } = makeRecorderKit(kindNode);
kindAdmin.onUpdate(recorder);
return recorder.write([]);
}),
);
};

/**
* Build root object of the PSM-only bootstrap vat.
*
Expand Down Expand Up @@ -267,7 +311,7 @@ export const buildRootObject = async (vatPowers, vatParameters) => {
bridgeProvisioner(powersFor('bridgeProvisioner')),
makeChainStorage(powersFor('makeChainStorage')),
makeAddressNameHubs(allPowers),
publishAgoricNames(allPowers, {
legacyPublishAgoricNames(allPowers, {
options: {
agoricNamesOptions: { topLevel: Object.keys(agoricNamesReserved) },
},
Expand Down
5 changes: 1 addition & 4 deletions packages/orchestration/src/proposals/orchestration-proposal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,7 @@ const publishChainInfoToChainStorage = async (
chainStorageP,
) => {
const chainStorage = await chainStorageP;
if (!chainStorage) {
console.warn('no chain storage, not registering chain info');
return;
}
assert(chainStorage, 'publishChainInfoToChainStorage requires chainStorage');

const agoricNamesNode = await E(chainStorage).makeChildNode('agoricNames');

Expand Down
58 changes: 5 additions & 53 deletions packages/vats/src/core/chain-behaviors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import {
makeSubscriptionKit,
observeIteration,
} from '@agoric/notifier';
import { makeScalarBigMapStore } from '@agoric/vat-data';
import { prepareRecorderKit } from '@agoric/zoe/src/contractSupport/recorder.js';
import * as farExports from '@endo/far';
import { E, Far } from '@endo/far';
import { importBundle } from '@endo/import-bundle';
import { makePromiseKit } from '@endo/promise-kit';
import { makeMockChainStorageRoot } from '@agoric/internal/src/storage-test-utils.js';
import { PowerFlags } from '../walletFlags.js';
import { BASIC_BOOTSTRAP_PERMITS } from './basic-behaviors.js';
import { agoricNamesReserved, callProperties, extractPowers } from './utils.js';
Expand Down Expand Up @@ -357,8 +356,8 @@ export const makeChainStorage = async ({
}) => {
const bridgeManager = await bridgeManagerP;
if (!bridgeManager) {
console.warn('Cannot support chainStorage without an actual chain.');
chainStorageP.resolve(null);
console.warn('no bridge so chainStorage will not write.');
chainStorageP.resolve(makeMockChainStorageRoot());
Comment on lines +359 to +360
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay! The undefined case for chainStorage was only for the sim chain, I'm pretty sure.

console.warn is exactly the way I think this should be treated.

A big part of me would like to tell the type system that this thing is always there so we don't have to keep supporting the sim chain in new code. That might be more involved, though.
cc @michaelfig @0xpatrickdev

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to remove sim-chain concerns from chain-behaviors entirely? I figure we could provide a fake bridge device here:

agoric-sdk/packages/vats/src/core/chain-behaviors.js

Lines 319 to 328 in af9e1c1

if (!bridge) {
console.warn(
'Running without a bridge device; this is not an actual chain.',
);
bridgeManagerP.resolve(undefined);
provisionBridgeManager.resolve(undefined);
provisionWalletBridgeManager.resolve(undefined);
walletBridgeManager.resolve(undefined);
return;
}

But I don't know what fidelity it needs or if anything downstream relies on detecting its absence.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to remove sim-chain concerns from chain-behaviors entirely?

I can't think of any technical obstacles. @michaelfig , maybe you know?

Seems like product (@mitdralla, @sufyaankhan ) should be involved. @sufyaankhan led the charge to hide the sim chain from docs.agoric.com .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to remove sim-chain concerns from chain-behaviors entirely?

I can't think of any technical obstacles. @michaelfig , maybe you know?

No technical obstacles. Earlier in our history, it was considered onerous to force dapp-developers to install Docker or compile agd themselves. Nowadays, we rely on Docker without batting an eye. The times have changed, and sim-chain is a relic we can safely jettison.

// @ts-expect-error expects value or undefined
storageBridgeManagerP.resolve(null);
return;
Expand Down Expand Up @@ -428,10 +427,8 @@ export const publishAgoricNamesToChainStorage = async ({
},
}) => {
const root = await rootP;
if (!root) {
console.warn('no chainStorage: not publishing agoricNames');
return;
}
assert(root, 'publishAgoricNamesToChainStorage requires chainStorage');

const nameStorage = E(root).makeChildNode('agoricNames');
await E(agoricNames).publishNameHubs(
nameStorage,
Expand All @@ -440,51 +437,6 @@ export const publishAgoricNamesToChainStorage = async ({
);
};

/**
* @deprecated use publishAgoricNamesToChainStorage
* @param {BootstrapPowers} powers
* @param {{
* options?: {
* agoricNamesOptions?: {
* topLevel?: string[];
* };
* };
* }} config
*/
export const publishAgoricNames = async (
{ consume: { agoricNamesAdmin, board, chainStorage: rootP } },
{ options: { agoricNamesOptions } = {} } = {},
) => {
const root = await rootP;
if (!root) {
console.warn('cannot publish agoricNames without chainStorage');
return;
}
const nameStorage = E(root).makeChildNode('agoricNames');
const marshaller = E(board).getPublishingMarshaller();

// XXX will fail upon restart, but so would the makeStoredPublishKit this is replacing
// Since we expect the bootstrap vat to be replaced instead of upgraded this should be
// fine. See {@link ./README.md bootstrap documentation} for details.
const fakeBaggage = makeScalarBigMapStore(
'fake baggage for AgoricNames kinds',
);
const makeRecorderKit = prepareRecorderKit(fakeBaggage, marshaller);

// brand, issuer, ...
const { topLevel = keys(agoricNamesReserved) } = agoricNamesOptions || {};
await Promise.all(
topLevel.map(async kind => {
const kindAdmin = await E(agoricNamesAdmin).lookupAdmin(kind);

const kindNode = await E(nameStorage).makeChildNode(kind);
const { recorder } = makeRecorderKit(kindNode);
kindAdmin.onUpdate(recorder);
return recorder.write([]);
}),
);
};

/**
* no free lunch on chain
*
Expand Down
5 changes: 1 addition & 4 deletions packages/vats/src/proposals/localchain-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ export const testLocalChain = async (
console.warn('=== localchain test started (result in', testResultPath, ')!');
/** @type {null | ERef<StorageNode>} */
let node = await chainStorage;
if (!node) {
console.error('testLocalChain no chainStorage');
throw new Error('no chainStorage');
}
assert(node, 'testLocalChain requires chainStorage');

let result;
try {
Expand Down
49 changes: 5 additions & 44 deletions packages/vats/test/name-hub-published.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,12 @@
import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js';

import { makeMockChainStorageRoot } from '@agoric/internal/src/storage-test-utils.js';
import { makeHandle } from '@agoric/zoe/src/makeHandle.js';
import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
import {
makeAgoricNamesAccess,
makePromiseSpaceForNameHub,
} from '../src/core/utils.js';
import { makePromiseSpace } from '../src/core/promise-space.js';
import {
publishAgoricNames,
setupClientManager,
} from '../src/core/chain-behaviors.js';
import { makeFakeBoard } from '../tools/board-utils.js';
import { makeAddressNameHubs } from '../src/core/basic-behaviors.js';
import { makePromiseSpaceForNameHub } from '../src/core/utils.js';
import { makeNameHubKit } from '../src/nameHub.js';

test('publishAgoricNames publishes AMM instance', async t => {
const space = makePromiseSpace();
const storageRoot = makeMockChainStorageRoot();
const { agoricNames, agoricNamesAdmin } = await makeAgoricNamesAccess();
const board = makeFakeBoard();
const marshaller = board.getPublishingMarshaller();
space.produce.agoricNames.resolve(agoricNames);
space.produce.agoricNamesAdmin.resolve(agoricNamesAdmin);
space.produce.chainStorage.resolve(storageRoot);
space.produce.board.resolve(board);

await Promise.all([
setupClientManager(/** @type {any} */ (space)),
makeAddressNameHubs(/** @type {any} */ (space)),
publishAgoricNames(/** @type {any} */ (space)),
]);
const ammInstance = makeHandle('instance');
const instanceAdmin = await agoricNamesAdmin.lookupAdmin('instance');
instanceAdmin.update('amm', ammInstance);

await eventLoopIteration(); // wait for publication to settle

t.deepEqual(
storageRoot.getBody(
'mockChainStorageRoot.agoricNames.instance',
marshaller,
),
[['amm', ammInstance]],
);
});
// This suite used to also test publishAgoricNames but that code wasn't used
// anywhere so the test was removed. Its replacement,
// publishAgoricNamesToChainStorage, requires more more context so it's tested
// through integration instead of as a unit test.

test('promise space reserves non-well-known names', async t => {
const { nameHub, nameAdmin } = makeNameHubKit();
Expand Down
Loading