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

swingset kernel: avoid use of require #444

Closed
warner opened this issue Jan 23, 2020 · 1 comment
Closed

swingset kernel: avoid use of require #444

warner opened this issue Jan 23, 2020 · 1 comment
Assignees
Labels
SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Jan 23, 2020

in #410 the dynamic-vat-creation code needs to evaluate the new vat's code string in an environment that allows it to call require() with the usual three suspects: Nat, harden, and evaluate. To build the require endowment, this function (kernel.js kernelRequire(), roughly around line 530) needs to import those three values into its own environment.

As we discussed during the review of #410, we should do this with import statements rather than require calls. We can get away with require for now, because we feed all the kernel code through rollup and then evaluate it with a require endowment. But that will probably change soon, either when we run under XS, or when we move to the upcoming new SES API, both of which are entirely focussed on modules.

My comments from the review were:

I'm not sure we should do this now, but in a future cleanup I'm wondering if we should add a import evaluate from '@agoric/evaluate'; import Nat from '@agoric/nat'; at the top of this file, and remove the require() calls from this function (so return harden and Nat and evaluate). Then in createVatDynamically we could just call evaluate.evaluateProgram(buildFnSrc, req) instead of using the generated require() function to extract the evaluator.

@warner warner added the SwingSet package: SwingSet label Jan 23, 2020
@Chris-Hibbert Chris-Hibbert self-assigned this Jan 23, 2020
@Chris-Hibbert
Copy link
Contributor

#24

warner added a commit that referenced this issue Jan 24, 2020
This got accidentally pushed to trunk, but we've reviewed it and it looks
fine. This commit pretends to be the PR merge that didn't happen.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

2 participants