fix(synthetic-chain)!: bundle info with dir

[mhofman]

passCoreEvalProposal was relying on a fake WebCache configured with a hard coded submission base dir, even though the actual bundles may be in a different location.

This PR removes the use of the WebCache by passCoreEvalProposal and extends the BundleInfo definition to include base dir details.

Tested as patch in Agoric/agoric-sdk#8901

[turadg]

This takes care of the bulk of #81

Holding approve around the breaking change and because you seem to be unblocked with your patch.

cc @dckc re: WebCache

[turadg]

Why change this type? This is the only breaking change I see. Is it necessary?

I factored it that way to enforce that there's on bundle per name, an assumption I think the user will have. Admittedly, given that goal the loop should have enforced it.

[mhofman]

Why change this type? This is the only breaking change I see. Is it necessary?

Correct, the return value of readBundles and the param of passCoreEvalProposal are the only breaking changes. I didn't see them used anywhere so figured it was safe to break. It's necessary to break passCoreEvalProposal because it needs the directory information to come from somewhere. Before it was assuming a submission subdir

I factored it that way to enforce that there's on bundle per name, an assumption I think the user will have. Admittedly, given that goal the loop should have enforced it.

As you noticed, the change from bundleAssets -> bundleInfos doesn't change the logic at all. While it does technically allow multiple bundle infos to share the same name, I don't think it's a huge problem if that's the case?

Edit: Happy to go back to a map if preferred.

[turadg]

While it does technically allow multiple bundle infos to share the same name, I don't think it's a huge problem if that's the case?

Not huge, but worse. As I think about this more, there's no good reason to export this functionality from the package so I'm not concerned with the breaking change. I'll probably follow up with another breaking change (affecting nobody) to drop these exports. So refactor at will.

I might also add then a validation that names aren't reused, unless you want to add that now. It's probably easiest to do with a map so I might end up changing this back at that point.

[mhofman]

Holding approve around the breaking change and because you seem to be unblocked with your patch.

I would really prefer avoiding a patch as that makes maintenance by others more difficult.

[turadg]

Holding approve around the breaking change and because you seem to be unblocked with your patch.

I would really prefer avoiding a patch as that makes maintenance by others more difficult.

Of course we will fix this upstream. My point was that your work is not blocked on this landing. Is that incorrect?

[mhofman]

Of course we will fix this upstream. My point was that your work is not blocked on this landing. Is that incorrect?

I meant that I would prefer this landing before I land the agoric-sdk PR to avoid people having to deal with removing patches when they touch a3p-integration deps in agoric-sdk. But yeah it's not a blocker. Maybe we'd consider a first 0.0.7-1 alpha?

[mhofman]

cc @dckc re: WebCache

Btw, I didn't understand the purpose the the fake WebCache here, so I ripped it out as it made things simpler. Happy to backtrack that.

[dckc]

cc @dckc re: WebCache

Btw, I didn't understand the purpose the the fake WebCache here, so I ripped it out as it made things simpler. Happy to backtrack that.

It's an attempt to use POLA-shaped I/O. I try to follow the ocap discipline part of our style guide; in particular:

modules should not export powerful objects (for example, objects that close over fs or process.env)

This code represents file access as a string and uses ambient access to fsp:

export const readBundles = async (dir: string) => {
  const files = await fsp.readdir(dir);

In the original cache stuff webAsset.js, io powers are passed around explicitly:

export const makeWebRd = (root, { fetch }) => {
...
      readText: async () => {
        console.log('WebRd fetch:', there);
        const res = await fetch(there);

backtracking to here maintains API compatibility with the result of makeWebRd stuff but doesn't help with ocap discipline:

const makeFakeWebCache = (base: string): WebCache => {
  return {
    getText(segment: string) {
      return fsp.readFile(path.join(base, segment), 'utf8');

And I think we have several copies of this code for reading bundles scattered around this repo. So since I didn't clarify or add tests or clean any of that up, it seems fair game for you to rip it out.

[mhofman]

backtracking to here maintains API compatibility with the result of makeWebRd stuff but doesn't help with ocap discipline:

Yeah I figured as much. passCoreEvalProposal called makeFakeWebCache on its own. An alternative would have been to pass something like the webcache + bundle info in, so that you grant access to the data backing the bundle, not just the bundle infos, but I didn't see a good way to structure this quickly, nor a good reason to since passCoreEvalProposal is already full of ambient power usages.

[turadg]

consider a first 0.0.7-1 alpha

Published and pushed the release commit here.