endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

feat(daemon): Support pet name path lookup #1915

Closed kriskowal closed 9 months ago

kriskowal commented 10 months ago

This change allows dot delimited pet name paths to follow references through lookup and establishes a protocol where any object can serve as a name hub if it implements simple pet name lookup.

dckc commented 10 months ago

I'd expect this to work in endo eval CLI bindings. It doesn't seem to. Is that by design?

This works:

$ endo eval "E(wk.query).lookup('agoricNames', 'brand', 'IST')" wk:alice-wk
Object [Alleged: IST brand#board0257] {}

But using a dotted path doesn't:

$ endo eval "E(wk.query).lookup('agoricNames', 'brand')" -n brand1 wk:alice-wk
Object [Alleged: NameHub] {}

$ endo eval 'x' x:brand1.IST
CapTP cli exception: (RemoteError(error:captp:Endo#20001)#1)
RemoteError(error:captp:Endo#20001)#1: Invalid pet name "brand1.IST"

  at decodeErrorCommon (packages/marshal/src/marshal.js:281:28)
  at decodeErrorFromCapData (packages/marshal/src/marshal.js:297:14)
  at decodeFromCapData (packages/marshal/src/encodeToCapData.js:404:27)
  at fromCapData (packages/marshal/src/marshal.js:359:23)
  at CTP_RETURN (packages/captp/src/captp.js:697:24)
  at dispatch (packages/captp/src/captp.js:776:7)
  at packages/daemon/src/connection.js:34:7

(RemoteError(error:captp:Endo#20001)#1)
rekmarks commented 9 months ago

@dckc I think you're seeing that error because it's hitting the pet store lookup instead of the mailbox lookup modified in this PR: https://github.com/endojs/endo/blob/kriskowal-daemon-pet-name-path-lookup/packages/daemon/src/pet-store.js/#L70-L74

@kriskowal, should the pet store lookup also be updated or are they serving different purposes?

kriskowal commented 9 months ago

I have not gone so far in this PR to support both dot delimited pet paths and also variadic lookup arguments. We could do both. I feel that we must support the dot delimited paths since a lot of the Endo API uses string arguments for pet names / pet paths and it would be awkward to make them all arrays or variadic.

dckc commented 9 months ago

... I feel that we must support the dot delimited paths since a lot of the Endo API uses string arguments for pet names / pet paths and it would be awkward to make them all arrays or variadic.

The dotted paths should just be a CLI / UI feature, right? They should get parsed before use in other parts of the API. In agoric-sdk, a PetName is string | string[].

kriskowal commented 9 months ago

The dotted paths should just be a CLI / UI feature, right? They should get parsed before use in other parts of the API. In agoric-sdk, a PetName is string | string[].

That would work and I do like moving the concern of pet name language out to the UI layers.

kriskowal commented 9 months ago

(Then the question is whether to flatten both variadic arguments and array arguments here)

dckc commented 9 months ago

(Then the question is whether to flatten both variadic arguments and array arguments here)

I think so, yes... but I'm not sure about the question. Give me an example, please?

kriskowal commented 9 months ago

(Then the question is whether to flatten both variadic arguments and array arguments here) I think so, yes... but I'm not sure about the question. Give me an example, please?

Should this work: E(hub).lookup(['a', 'b'], ['c', 'd'])?

Should this work: E(hub).lookup('a', ['b', 'c'])?

dckc commented 9 months ago

(Then the question is whether to flatten both variadic arguments and array arguments here) I think so, yes... but I'm not sure about the question. Give me an example, please?

Should this work: E(hub).lookup(['a', 'b'], ['c', 'd'])?

Should this work: E(hub).lookup('a', ['b', 'c'])?

No.

const KeyShape = M.string();
const PathShape = M.arrayOf(KeyShape);

export const NameHubIKit = harden({
  nameHub: M.interface('NameHub', {
...
    lookup: M.call().rest(PathShape).returns(M.promise()),

https://github.com/Agoric/agoric-sdk/blob/master/packages/vats/src/nameHub.js

export type NameHub = {
...
  /**
   * Look up a path of keys starting from the current NameHub. Wait on any
   * reserved promises.
   */
  lookup: (...path: string[]) => Promise<any>;

https://github.com/Agoric/agoric-sdk/blob/master/packages/vats/src/types.d.ts

kriskowal commented 9 months ago

Should this work: E(hub).lookup(['a', 'b'], ['c', 'd'])? Should this work: E(hub).lookup('a', ['b', 'c'])? No.

So, only E(hub).lookup('a', 'b', 'c', 'd'). It would follow E(hub).lookup(petName), E(hub).lookup(...petNamePath), or E(hub).lookup(...petNamePathFrom(petNameOrPetNamePath).

/**
 * @typedef {string} PetName
 * @typedef {string[]} PetNamePath
 */

/**
 * @param {string | string[]} petNameOrPetNamePath
 */
const petNamePathFrom = arg => typeof arg === 'string' ? [ arg ] : arg;

E(hub).lookup(...petNamePathFrom(petNameOrPetNamePath);
kriskowal commented 9 months ago

Attn @rekmarks I’m in favor of reworking this PR so that the Daemon API uses arrays for pet name paths instead of dot delimited strings. The CLI would be responsible for splitting pet name path strings to make those arrays. The Daemon API might accept string | string[] in some positions, except the lookup method would become variadic and not accept arrays.

rekmarks commented 9 months ago

Superseded by #2022.