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.

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

Update moduleFormat so that require is not needed as an endowment #362

Closed
katelynsills opened this issue Dec 21, 2019 · 8 comments · Fixed by #1320
Closed

Update moduleFormat so that require is not needed as an endowment #362

katelynsills opened this issue Dec 21, 2019 · 8 comments · Fixed by #1320
Assignees
Labels
bundle-source package: bundle-source cosmic-swingset package: cosmic-swingset import-bundle package: import-bundle

Comments

@katelynsills
Copy link
Contributor

katelynsills commented Dec 21, 2019

There is a suspicious require endowment needed in:

Posted by @erights in #355

const zoe = makeZoe({ require });

As @michaelfig says below, when we upgrade the moduleFormat, we should not need to pass require anymore.

@michaelfig
Copy link
Member

michaelfig commented Dec 21, 2019

require under SwingSet on SES is a crafted endowment to give access to @agoric/harden and @agoric/evaluate with specific exports. It is safe in that context.

This will change when we migrate our moduleFormat from getExport to something else, dropping the need for the endowment in exchange for per-module globals instead. This is part of a wider design that still needs approval and implementation.

@katelynsills katelynsills added the Zoe package: Zoe label Jan 9, 2020
@katelynsills
Copy link
Contributor Author

@erights does this need to stay open or can we close it given Michael's answer?

@dckc
Copy link
Member

dckc commented Jan 10, 2020

Is the change @michaelfig refers to captured in an open issue? I'd prefer to see that before this is closed.

@michaelfig
Copy link
Member

Let's not close this yet. I'll remember to reference this issue when we start discussing least-authority linkage formats.

@michaelfig michaelfig changed the title [Zoe] require as an endowment! Why is this safe? Update moduleFormat so that require is not needed as an endowment Jan 13, 2020
@michaelfig michaelfig added cosmic-swingset package: cosmic-swingset bundle-source package: bundle-source labels Jan 13, 2020
@warner
Copy link
Member

warner commented Jun 26, 2020

This will be fixed by the new-SES PR (#1201), and it changes all occurrences of makeZoe() with the evaluate-less one.

@warner
Copy link
Member

warner commented Jun 27, 2020

This landed, and makeZoe() no longer needs a { require } endowment. It has code to tolerate its presence, but I removed it from all the instances I could find (including the three off-repo dapps).

Apart from @michaelfig 's least-authority linkage format question, I think the remaining issue here is that importBundle must still provide a require to the code created by bundleSource, even if that code does not have have an "external" items left to require(). We still require('@agoric/harden') in a lot of code, but if/once #1214 changes that, then our vat and contract bundles should not have any require needs anymore, and I'd like to change importBundle to stop providing the require endowment.

So I think this task now lies within bundle-source, not Zoe, so I'm going to assign it to @michaelfig as the owner of that package.

@warner
Copy link
Member

warner commented Jul 20, 2020

@michaelfig did you change bundle-source to not emit bundles that need require yet? I just ran a test in which swingset no longer provides a require endowment to vat code, and it failed:

UnhandledPromiseRejectionWarning: ReferenceError: require is not defined
    at getExportWithNestedEvaluate (/bundled-source-preamble.js:106:39)
    at importBundle (/home/warner/stuff/agoric/agoric-sdk/packages/import-bundle/src/index.js:57:45)
    at loadStaticVat (/home/warner/stuff/agoric/agoric-sdk/packages/SwingSet/src/controller.js:187:25)
    at async buildVatController (/home/warner/stuff/agoric/agoric-sdk/packages/SwingSet/src/controller.js:231:23)
    at async Test.<anonymous> (/home/warner/stuff/agoric/agoric-sdk/packages/SwingSet/test/test-controller.js:28:22)

I think this was closed in error.. I'll reopen it. I think we need that bundle-source fix before we can close it.

@michaelfig
Copy link
Member

michaelfig commented Jul 20, 2020

@michaelfig did you change bundle-source to not emit bundles that need require yet? I just ran a test in which swingset no longer provides a require endowment to vat code, and it failed:

UnhandledPromiseRejectionWarning: ReferenceError: require is not defined
    at getExportWithNestedEvaluate (/bundled-source-preamble.js:106:39)
    at importBundle (/home/warner/stuff/agoric/agoric-sdk/packages/import-bundle/src/index.js:57:45)
    at loadStaticVat (/home/warner/stuff/agoric/agoric-sdk/packages/SwingSet/src/controller.js:187:25)
    at async buildVatController (/home/warner/stuff/agoric/agoric-sdk/packages/SwingSet/src/controller.js:231:23)
    at async Test.<anonymous> (/home/warner/stuff/agoric/agoric-sdk/packages/SwingSet/test/test-controller.js:28:22)

I think this was closed in error.. I'll reopen it. I think we need that bundle-source fix before we can close it.

AFK: I won't be able to get to this right away, but IIRC, it's a one-line change to bundle-source:

   { require }

to

   { require: typeof require === 'undefined' ? undefined : require }

Or equivalent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bundle-source package: bundle-source cosmic-swingset package: cosmic-swingset import-bundle package: import-bundle
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants