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

Jump to bottom

Daemon: identify based lookup #2051

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

Daemon: identify based lookup #2051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6ca0a19
fix(daemon): rename petStore lookup to identifyLocal
kumavis Feb 10, 2024
eb9b3aa
fix(daemon): rename mail lookupFormulaIdentifierForName to identifyLocal
kumavis Feb 10, 2024
f19e5e6
feat(daemon): daemon exposes identifyFrom method
kumavis Feb 10, 2024
5b3dc78
feat(daemon): petStore exposes an identify function
kumavis Feb 10, 2024
80cf019
feat(daemon): mail exposes an identify function
kumavis Feb 10, 2024
3cdb9c5
feat(daemon): host exposes an identify function
kumavis Feb 10, 2024
06d96c8
feat(daemon): guest exposes an identify function
kumavis Feb 10, 2024
7f0acf8
feat(daemon): petStoreInspector exposes an identify function
kumavis Feb 11, 2024
dd6f2f8
fix(daemon): identifyFrom should return originIdentifier when path em…
kumavis Feb 11, 2024
34b2cfa
feat(cli): add identify command
kumavis Feb 11, 2024
d0c160f
wip: implement lookup on top of identify
kumavis Feb 11, 2024
f300808
feat(daemon): daemon exposes lookupFrom method
kumavis Feb 11, 2024
9f5a079
feat(cli): show command uses daemon lookupFrom
kumavis Feb 11, 2024
482a657
feat(daemon): attempt to shortcut identify for host + host-id512
kumavis Feb 11, 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
14 changes: 14 additions & 0 deletions packages/cli/src/commands/identify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/* global process */
import os from 'os';
import { E } from '@endo/far';
import { withEndoBootstrap } from '../context.js';

export const identify = async ({ cancel, cancelled, sockPath, namePath }) =>
withEndoBootstrap({ os, process }, async ({ bootstrap }) => {
const defaultHostFormulaIdentifier = 'host';
const formulaId = await E(bootstrap).identifyFrom(
defaultHostFormulaIdentifier,
namePath,
);
console.log(formulaId);
});
Comment on lines +6 to +14
Copy link
Member

Choose a reason for hiding this comment

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

This is very close to what I imagined and a step in the right direction.

High level:

  • We need an API and a command that can reveal the formula type. I had envisioned that endo identify would do that, but perhaps that should have a name like describe. I think endo describe should also be able to print the nonce/swissnum/identifier like endo describe --id <name>
  • Whatever API and command reveals the nonce/swissnum/identifier for a pet name path should probably be producing a URL so maybe that should be called endo locate <pet-name-path> and produce the URL by default, including the connection hints for the listening networks. Maybe endo locate --json <path> gives you the JSON representation. Maybe endo locate --id <path> trims that back to just the nonce.
  • Maybe locate and describe are the same command or two commands with the same behavior but different defaults.

Nits:

  • Let’s get rid of cancel, cancelled, and sockPath here and where this refactor-cruft was copied from.
  • host is not a formula identifier anymore. We should always start looking stuff up at E(bootstrap).host().
  • If we move some functions from the endo bootstrap object to host objects, maybe we can just present the primary host facet as the bootstrap object.
  • Maybe we should use process.stdout.write(`${id}\n`) instead of console.log just to break the bad habit of confusing the debugger for an output printer. ymmv.

12 changes: 8 additions & 4 deletions packages/cli/src/commands/show.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
/* global process */
import os from 'os';
import { E } from '@endo/far';
import { withEndoParty } from '../context.js';
import { withEndoBootstrap } from '../context.js';

export const show = async ({ cancel, cancelled, sockPath, name, partyNames }) =>
withEndoParty(partyNames, { os, process }, async ({ party }) => {
const pet = await E(party).lookup(name);
export const show = async ({ cancel, cancelled, sockPath, namePath }) =>
withEndoBootstrap({ os, process }, async ({ bootstrap }) => {
const defaultHostFormulaIdentifier = 'host';
const pet = await E(bootstrap).lookupFrom(
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. In my head, lookupFrom is equivalent to E(E(host).locate(identifier)).lookup(path) but consolidated. The consolidation might be important since we might want to locate and lookup locally in a single event.

I think we should move the method to Host in order to stay one step away from folding the Endo Bootstrap object into Host objects. They are equally powerful since you can’t get one without the other and there’s little reason to keep them separate.

I’m not attached to the locate(nonce) ~=> identifier function name, and I think I at least temporarily need the name of this method to be explicitly lookupFromFormulaNumber, with goal of reducing that to lookupFromIdentifier when we no longer need to distinguish formula identifier with type from formula number.

defaultHostFormulaIdentifier,
namePath,
);
console.log(pet);
});
23 changes: 13 additions & 10 deletions packages/cli/src/endo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,18 +341,21 @@ export const main = async rawArgs => {
});

program
.command('show <name>')
.command('identify <namePath>')
.description('prints the formulaId for the named path')
.action(async namePathString => {
const namePath = namePathString.split('.');
const { identify } = await import('./commands/identify.js');
return identify({ namePath });
});

program
.command('show <namePath>')
.description('prints the named value')
.option(
'-a,--as <party>',
'Pose as named party (as named by current party)',
collect,
[],
)
.action(async (name, cmd) => {
const { as: partyNames } = cmd.opts();
.action(async (namePathString, cmd) => {
const namePath = namePathString.split('.');
const { show } = await import('./commands/show.js');
return show({ name, partyNames });
return show({ namePath });
});

program
Expand Down
145 changes: 134 additions & 11 deletions packages/daemon/src/daemon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,51 @@ const makeEndoBootstrap = (
return `readable-blob-sha512:${sha512Hex}`;
};

/** @type {import('./types.js').IdentifyFromFn} */
const identifyFrom = async (originFormulaIdentifier, namePath) => {
await null;
if (namePath.length === 0) {
return originFormulaIdentifier;
}
// Attempt to shortcut the identify by reading directly from the petStore.
const { type: formulaType, number: formulaNumber } = parseFormulaIdentifier(
originFormulaIdentifier,
);
if (formulaType === 'host' || formulaType === 'host-id512') {
let storeFormulaIdentifier = `pet-store`;
if (formulaType === 'host-id512') {
storeFormulaIdentifier = `pet-store-id512:${formulaNumber}`;
}
// eslint-disable-next-line no-use-before-define
const petStore = await provideValueForFormulaIdentifier(
Copy link
Member

Choose a reason for hiding this comment

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

I suppose we can’t make this synchronous!

storeFormulaIdentifier,
);
const result = await E(petStore).identify(namePath);
Copy link
Member

Choose a reason for hiding this comment

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

We should definitely rename petStore.locate to petStore.identify as this suggests. It should be synchronous, though.

// Use result if present, otherwise fallback to identifying from the host.
if (result !== undefined) {
return result;
}
}
// eslint-disable-next-line no-use-before-define
const origin = await provideValueForFormulaIdentifier(
originFormulaIdentifier,
);
return E(origin).identify(namePath);
Copy link
Member

Choose a reason for hiding this comment

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

Given this is async tail recursive and has to at least reïfy pet stores (an unavoidably async operation), I withdraw my hope that identify can even be locally synchronous or transactional.

Given that it can’t be synchronous, there’s no reason to bundle up locate(identifier)~.lookup(path). They can be peanut and butter: two separate methods that are better together.

};

/** @type {import('./types.js').LookupFromFn} */
const lookupFrom = async (originFormulaIdentifier, namePath) => {
const formulaIdentifier = await identifyFrom(
originFormulaIdentifier,
namePath,
);
if (formulaIdentifier === undefined) {
throw new Error(`Failed to lookup ${namePath.join('.')}`);
}
// eslint-disable-next-line no-use-before-define
return provideValueForFormulaIdentifier(formulaIdentifier);
};

/**
* @param {string} workerId512
*/
Expand Down Expand Up @@ -413,6 +458,7 @@ const makeEndoBootstrap = (
const external = petStorePowers.makeOwnPetStore(
'pet-store',
assertPetName,
identifyFrom,
);
return { external, internal: undefined };
} else if (formulaIdentifier === 'pet-inspector') {
Expand Down Expand Up @@ -469,6 +515,7 @@ const makeEndoBootstrap = (
const external = petStorePowers.makeIdentifiedPetStore(
formulaNumber,
assertPetName,
identifyFrom,
);
return { external, internal: undefined };
} else if (formulaType === 'host-id512') {
Expand Down Expand Up @@ -627,6 +674,7 @@ const makeEndoBootstrap = (
});

const makeMailbox = makeMailboxMaker({
identifyFrom,
formulaIdentifierForRef,
provideValueForFormulaIdentifier,
provideControllerForFormulaIdentifier,
Expand All @@ -651,6 +699,24 @@ const makeEndoBootstrap = (
makeMailbox,
});

const allowedInspectorFormulaTypes = [
'eval-id512',
'lookup-id512',
'make-unconfined-id512',
'make-bundle-id512',
'guest-id512',
'web-bundle',
];

const allowedInspectorFormulaIdentificationsByType = {
eval: ['endowments', 'worker'],
Copy link
Member

Choose a reason for hiding this comment

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

I’m not sure that inspectors generalize this well. endowments is an array of identifiers and we want the corresponding values, whereas worker is a single identifier. This does not mention source which is just a string.

In short, I think we should have a boring identify function for each formula type rather than a generalization because it will be more legible and easier to debug.

lookup: ['hub'],
guest: ['host'],
'make-bundle': ['bundle', 'powers', 'worker'],
'make-unconfined': ['powers', 'worker'],
'web-bundle': ['bundle', 'powers'],
};

/**
* Creates an inspector for the current party's pet store, used to create
* inspectors for values therein. Notably, can provide references to otherwise
Expand All @@ -665,28 +731,81 @@ const makeEndoBootstrap = (
petStoreFormulaIdentifier,
);

const identifyOnFormula = (formula, propertyName, namePath) => {
if (
allowedInspectorFormulaIdentificationsByType[formula.type] === undefined
) {
// Cannot provide a reference under the requested formula type.
return { formulaIdentifier: undefined, remainderPath: namePath };
}
const allowedInspectorFormulaIdentifications =
allowedInspectorFormulaIdentificationsByType[formula.type];
if (!allowedInspectorFormulaIdentifications.includes(propertyName)) {
// Cannot provide a reference for the requested property.
return { formulaIdentifier: undefined, remainderPath: namePath };
}
if (formula.type === 'eval' && propertyName === 'endowments') {
// We consume an additional part of the path for the endowment name.
const [endowmentName, ...namePathRest] = namePath;
assertPetName(endowmentName);
const endowmentIndex = formula.names.indexOf(endowmentName);
if (endowmentIndex === -1) {
// Cannot provide a reference to the missing endowment.
return { formulaIdentifier: undefined, remainderPath: namePath };
}
const formulaIdentifier = formula.values[endowmentIndex];
return { formulaIdentifier, remainderPath: namePathRest };
}
const formulaIdentifier = formula[propertyName];
return { formulaIdentifier, remainderPath: namePath };
};

/** @type {import('./types.js').IdentifyFn} */
const identify = async maybeNamePath => {
const namePath = Array.isArray(maybeNamePath)
? maybeNamePath
: [maybeNamePath];
const [petName, propertyName, ...namePathRest] = namePath;
assertPetName(petName);
assertPetName(propertyName);
const entryFormulaIdentifier = petStore.identifyLocal(petName);
if (entryFormulaIdentifier === undefined) {
return undefined;
}
const { type: formulaType, number: formulaNumber } =
parseFormulaIdentifier(entryFormulaIdentifier);
Comment on lines +775 to +776
Copy link
Member

Choose a reason for hiding this comment

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

By the time we get here, we may have removed the type from the formula identifier, but it can be trivially recovered from the controller object for the number.

Copy link
Member

Choose a reason for hiding this comment

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

When we have a formula for every formula identifier (just the number), it might make sense to just generally store that on the controller too, so we benefit from the memo and avoid redundant reads from the formula store.

// Identify is only supported on these types of formulas.
if (!allowedInspectorFormulaTypes.includes(formulaType)) {
return undefined;
}
const formula = await persistencePowers.readFormula(
formulaType,
formulaNumber,
);
const { formulaIdentifier, remainderPath } = identifyOnFormula(
formula,
propertyName,
namePathRest,
);
if (formulaIdentifier === undefined) {
return undefined;
}
return identifyFrom(formulaIdentifier, remainderPath);
};

/**
* @param {string} petName - The pet name to inspect.
* @returns {Promise<import('./types').KnownEndoInspectors[string]>} An
* inspector for the value of the given pet name.
*/
const lookup = async petName => {
const formulaIdentifier = petStore.lookup(petName);
const formulaIdentifier = petStore.identifyLocal(petName);
if (formulaIdentifier === undefined) {
throw new Error(`Unknown pet name ${petName}`);
}
const { type: formulaType, number: formulaNumber } =
parseFormulaIdentifier(formulaIdentifier);
if (
![
'eval-id512',
'lookup-id512',
'make-unconfined-id512',
'make-bundle-id512',
'guest-id512',
'web-bundle',
].includes(formulaType)
) {
if (!allowedInspectorFormulaTypes.includes(formulaType)) {
return makeInspector(formulaType, formulaNumber, harden({}));
}
const formula = await persistencePowers.readFormula(
Expand Down Expand Up @@ -765,6 +884,7 @@ const makeEndoBootstrap = (
const list = () => petStore.list();

const info = Far('Endo inspector facet', {
identify,
lookup,
list,
});
Expand All @@ -783,6 +903,9 @@ const makeEndoBootstrap = (

host: () => provideValueForFormulaIdentifier('host'),

identifyFrom,
lookupFrom,

leastAuthority: () => leastAuthority,

webPageJs: () => provideValueForFormulaIdentifier('web-page-js'),
Expand Down
2 changes: 2 additions & 0 deletions packages/daemon/src/guest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const makeGuestMaker = ({
}

const {
identify,
lookup,
reverseLookup,
followMessages,
Expand Down Expand Up @@ -80,6 +81,7 @@ export const makeGuestMaker = ({
/** @type {import('@endo/eventual-send').ERef<import('./types.js').EndoGuest>} */
const guest = Far('EndoGuest', {
has,
identify,
lookup,
reverseLookup,
request,
Expand Down
24 changes: 11 additions & 13 deletions packages/daemon/src/host.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ export const makeHostMaker = ({
);

const {
identifyLocal,
identify,
lookup,
reverseLookup,
lookupFormulaIdentifierForName,
listMessages,
provideLookupFormula,
followMessages,
Expand Down Expand Up @@ -75,7 +76,7 @@ export const makeHostMaker = ({
/** @type {string | undefined} */
let formulaIdentifier;
if (petName !== undefined) {
formulaIdentifier = lookupFormulaIdentifierForName(petName);
formulaIdentifier = identifyLocal(petName);
}
if (formulaIdentifier === undefined) {
/** @type {import('./types.js').GuestFormula} */
Expand Down Expand Up @@ -129,7 +130,7 @@ export const makeHostMaker = ({
if (typeof workerName !== 'string') {
throw new Error('worker name must be string');
}
let workerFormulaIdentifier = lookupFormulaIdentifierForName(workerName);
let workerFormulaIdentifier = identifyLocal(workerName);
if (workerFormulaIdentifier === undefined) {
const workerId512 = await randomHex512();
workerFormulaIdentifier = `worker-id512:${workerId512}`;
Expand All @@ -156,7 +157,7 @@ export const makeHostMaker = ({
return `worker-id512:${workerId512}`;
}
assertPetName(workerName);
let workerFormulaIdentifier = lookupFormulaIdentifierForName(workerName);
let workerFormulaIdentifier = identifyLocal(workerName);
if (workerFormulaIdentifier === undefined) {
const workerId512 = await randomHex512();
workerFormulaIdentifier = `worker-id512:${workerId512}`;
Expand All @@ -170,7 +171,7 @@ export const makeHostMaker = ({
* @param {string | 'NONE' | 'SELF' | 'ENDO'} partyName
*/
const providePowersFormulaIdentifier = async partyName => {
let guestFormulaIdentifier = lookupFormulaIdentifierForName(partyName);
let guestFormulaIdentifier = identifyLocal(partyName);
Copy link
Member

Choose a reason for hiding this comment

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

I like abbreviating lookupFormulaIdentifierForName to identifyLocal generally. That could be one easy-to-review mechanical refactor on its own.

Since the difference between identify and identifyLocal is more like identifyPetNamePath vs identifyPetName respectively, I could buy identify (implied pet name path since it’s public API) and identifyName instead.

if (guestFormulaIdentifier === undefined) {
const guest = await provideGuest(partyName);
guestFormulaIdentifier = formulaIdentifierForRef.get(guest);
Expand Down Expand Up @@ -216,9 +217,7 @@ export const makeHostMaker = ({

const petNamePath = petNamePathFrom(petNameOrPath);
if (petNamePath.length === 1) {
const formulaIdentifier = lookupFormulaIdentifierForName(
petNamePath[0],
);
const formulaIdentifier = identifyLocal(petNamePath[0]);
if (formulaIdentifier === undefined) {
throw new Error(`Unknown pet name ${q(petNamePath[0])}`);
}
Expand Down Expand Up @@ -308,8 +307,7 @@ export const makeHostMaker = ({
workerName,
);

const bundleFormulaIdentifier =
lookupFormulaIdentifierForName(bundleName);
const bundleFormulaIdentifier = identifyLocal(bundleName);
if (bundleFormulaIdentifier === undefined) {
throw new TypeError(`Unknown pet name for bundle: ${bundleName}`);
}
Expand Down Expand Up @@ -364,7 +362,7 @@ export const makeHostMaker = ({
/** @type {string | undefined} */
let formulaIdentifier;
if (petName !== undefined) {
formulaIdentifier = lookupFormulaIdentifierForName(petName);
formulaIdentifier = identifyLocal(petName);
}
if (formulaIdentifier === undefined) {
const id512 = await randomHex512();
Expand Down Expand Up @@ -393,8 +391,7 @@ export const makeHostMaker = ({
* @param {string | 'NONE' | 'SELF' | 'ENDO'} powersName
*/
const provideWebPage = async (webPageName, bundleName, powersName) => {
const bundleFormulaIdentifier =
lookupFormulaIdentifierForName(bundleName);
const bundleFormulaIdentifier = identifyLocal(bundleName);
if (bundleFormulaIdentifier === undefined) {
throw new Error(`Unknown pet name: ${q(bundleName)}`);
}
Expand Down Expand Up @@ -442,6 +439,7 @@ export const makeHostMaker = ({
/** @type {import('./types.js').EndoHost} */
const host = Far('EndoHost', {
has,
identify,
lookup,
reverseLookup,
listMessages,
Expand Down
Loading
Loading