Stop importing harden #1244

FUDCo · 2020-06-30T07:04:54Z

A very, very large number of mostly superficial changes to eliminate importation of @agoric/harden, since harden is now a global in the SES environment.

Interestingly, testing this turned up a considerable number of tests that needed to import @agoric/install-ses. These imports have been added, and various package.json files adjusted to reflect the many new uses of @agoric/install-ses and the disappearance of @agoric/harden.

Since these changes intersect with a large swath of our code base, I've tagged a number of different people as reviewers, but you don't all need to review all of it, just the pieces you worry about. I'm thinking the following:

@warner SwingSet, asssert, captp, same-structure, marshall, sparse-ints, store, swingset-runner, weak-store
@Chris-Hibbert and/or @katelynsills ERTP, notifier, registrar, spawner, sharing-service, zoe
@michaelfig eventual-send, agoric-cli, cosmic-swingset, produce-promise
@kriskowal bundle-source, import-manager

Of course if I've mistakenly assigned you a wrong thing, ignore it, and if you see I've assigned something to somebody else that you really should look at, look at it. None of this is magic, just trying to spread the load.

warner · 2020-06-30T16:39:13Z

Yeah, a lot of those files were being used both from vats under swingset (where they can rely upon a harden global) and by non-swingset unit tests (where they must import @agoric/harden instead).

Once the big zoe branch lands, we'll need to rebase and update this PR; I'm sure they added a bunch of new files/tests. We might consider making these changes one package at a time, if that makes it easier to keep up with trunk.

katelynsills

LGTM!

michaelfig · 2020-07-01T23:18:45Z

One complication. @kriskowal do we have JSDoc typing for globalThis.harden yet? If not, we should make an issue.

michaelfig

LGTM!

kriskowal · 2020-07-02T00:32:22Z

One complication. @kriskowal do we have JSDoc typing for globalThis.harden yet? If not, we should make an issue.

No, we don’t have TS tooling in SES yet, to the best of my knowledge.

kriskowal

🎉

warner

It looks like the XS test works by compiling two programs into C code and then running them. The first program is the equivalent of running a tape loader against all the test/test*.js files in packages/eventual-send:

agoric-sdk/.github/workflows/ag-solo-xs.yml

Lines 71 to 74 in 36d63fc

    
                   cd packages/eventual-send 
        
                   node -r esm $TAPE/bin/tape-xs-build.js $PWD test/test*.js 
        
                   noflake mcconfig -m -p x-cli-lin test-xs-manifest.json 
        
                   $MODDABLE/build/bin/lin/release/eventual-send

The second is the same but looking at packages/marshal. Both of those packages now include test.js files that do install-ses. SES includes @agoric/babel-standalone. That includes a regexp that looks like:

  function parseSourceMapInput(str) {
    return JSON.parse(str.replace(/^\)]}'[^\n]*\n/, ''));
  }

which appears to break in XS with:

/home/runner/work/agoric-sdk/agoric-sdk/node_modules/@agoric/babel-standalone/babel.js:21342: error: ^\)] invalid character!

cc @phoddie : is there a difference between the RegExp engine used in XS and the one that we were getting from Node? I see https://github.com/Moddable-OpenSource/moddable/blob/public/documentation/xs/XS%20Differences.md#conformance says Unicode property escapes aren't loaded by default, but that doesn't look like unicode to me. https://github.com/Moddable-OpenSource/moddable/blob/public/documentation/xs/XS%20Conformance.md#built-ins-1 lists a few RegExp tests which fail but I can't tell if any of them would cover this

@FUDCo I think the best thing to do here would be to switch these tests to use other agoric-sdk packages, which do not use install-ses. That way we'd still get some basic assurance that the XS makefiles still work. I'll open a separate issue to figure out the RegExp thing.

Change .github/workflows/ag-solo-xs.yml to use, say, same-structure and weak-store instead of eventual-send and marshal. And add a note with the regexp that XS didn't like, and a pointer here. I think that should let the CI test pass again.

All the other parts look good to me, modulo the comments below.

warner · 2020-07-02T06:54:53Z

packages/SwingSet/src/controller.js

 /* global Compartment harden */
+// eslint-disable-next-line no-redeclare


this disable marker can go away, it referred to something that used to be in the /* global */ marker that it used to precede

warner · 2020-07-02T06:55:46Z

packages/SwingSet/src/vats/network/network.js

@@ -1,12 +1,11 @@
+/* global harden */


@michaelfig does this impact any typescript checking? or is that an incomplete experiment?

Yes, this impacts typing. It's not an incomplete experiment, it actually worked (i.e. provided typechecking for the files).

I wrote:

One complication. @kriskowal do we have JSDoc typing for globalThis.harden yet? If not, we should make an issue.

I will probably be the one to make the issue and implement it.

warner · 2020-07-02T06:59:06Z

packages/SwingSet/test/vat-imports-1.js

@@ -1,14 +1,12 @@
-import harden from '@agoric/harden';
+/* global harden */

 function build(E, log) {
  const obj0 = {
    bootstrap(argv, _vats) {
      if (argv[0] === 'harden') {
        log('harden-1');


This clause was testing that vats can still do require('@agoric/harden'). Since we're removing both the users of that feature and the bundle-source ability to reach it, we should also remove this test entirely (delete vat-imports-1.js and test-vat-imports.js), and we should remove the code that used to support it (the if (what === '@agoric/harden') in vatRequire in controller.js). Note that we must leave vatRequire in place, even though it no longer accepts any names, until #362 is fixed.

warner · 2020-07-02T07:04:13Z

packages/acorn-eventual-send/test/test-rollup.js

@@ -8,7 +8,7 @@ test('SwingSet bug', async t => {
    const bundle = await rollup({
      input: require.resolve('../encouragementBotCommsWavyDot/bootstrap.js'),
      treeshake: false,
-      external: ['@agoric/evaluate', '@agoric/nat', '@agoric/harden'],
+      external: ['@agoric/evaluate', '@agoric/nat'],


This list should match the contents of vatRequire in controller.js. If vatRequire is now empty, this list should be too. I should have emptied it out when I landed the new-SES branch, sorry :).

warner · 2020-07-02T07:06:52Z

packages/eventual-send/rollup.config.js

-        globals: {
-          '@agoric/harden': 'harden',
-        },
+        // globals: {


@michaelfig can we delete this config file now? I think I got rid of the bundling step, but missed the config file.

Yes, that can be deleted.

phoddie · 2020-07-02T17:31:44Z

is there a difference between the RegExp engine used in XS and the one that we were getting from Node?

The implementation in XS is independent of Node. An introduction of our RegExp engine is available on our blog. Of course, it should be conforming to ECMA-262, except where documented.

The RegExp you note above does fail to parse with XS and succeed in Safari. We'll take a look. Thank you for the report.

We learned (in #1244) that the XS RegExp engine rejects several of the regexp literals that appear in the SES shim (via babel-standalone). The XS test works by compiling some other unit tests (eventual-send and marshal) into a C executable, then running it. #1244 added `install-ses` to most packages, including those two, which means XS is now being exposed to SES-shim code that it wasn't seeing before, triggering the rejection bug. When XS is updated to tolerate these cases, we'll turn this test back on. closes #1251

FUDCo force-pushed the stop-importing-harden branch from d6cf911 to 519a6a2 Compare June 30, 2020 07:32

FUDCo force-pushed the stop-importing-harden branch 6 times, most recently from aa31945 to 94761da Compare June 30, 2020 23:43

FUDCo requested review from warner, michaelfig, katelynsills, Chris-Hibbert and kriskowal July 1, 2020 22:05

katelynsills approved these changes Jul 1, 2020

View reviewed changes

michaelfig approved these changes Jul 1, 2020

View reviewed changes

kriskowal approved these changes Jul 2, 2020

View reviewed changes

warner requested changes Jul 2, 2020

View reviewed changes

FUDCo force-pushed the stop-importing-harden branch from b097460 to da04e94 Compare July 2, 2020 17:04

This was referenced Jul 2, 2020

maybe rewrite three regexps for XS compatibility agoric-labs/babel#4

Open

disable XS test (temporarily) #1251

Closed

warner mentioned this pull request Jul 2, 2020

chore: temporarily disable XS test #1252

Merged

FUDCo added 4 commits July 2, 2020 15:26

refactor: remove import @agoric/harden from SwingSet sources

a9e7dc2

refactor: remove import @agoric/harden from cosmic-swingset sources

4ec5b09

refactor: remove import @agoric/harden from swingset-runner sources

b001800

refactor: remove import @agoric/harden from zoe sources

b7d1946

FUDCo force-pushed the stop-importing-harden branch from d7e821d to 57aadad Compare July 2, 2020 22:26

FUDCo added 2 commits July 2, 2020 15:52

refactor: remove import @agoric/harden from miscellaneous sources

2b1d619

chore: incorporate review feedback

e94f4d3

FUDCo force-pushed the stop-importing-harden branch from 57aadad to e94f4d3 Compare July 2, 2020 22:53

FUDCo merged commit 54f67a7 into master Jul 2, 2020

FUDCo deleted the stop-importing-harden branch July 2, 2020 23:20

snyk-bot mentioned this pull request Sep 16, 2021

[Snyk] Security upgrade webpack-cli from 3.2.3 to 4.0.0 Kashuhackerone/agoric-sdk#23

Open

dckc mentioned this pull request Jun 26, 2022

migrate to @endo/errors package #5672

Closed

snyk-bot mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities Kashuhackerone/agoric-sdk#152

Open

Kashuhackerone mentioned this pull request Dec 25, 2022

[Snyk] Fix for 1 vulnerabilities Kashuhackerone/agoric-sdk#155

Open

Kashuhackerone mentioned this pull request Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities Kashuhackerone/agoric-sdk#200

Open

Kashuhackerone mentioned this pull request Dec 19, 2023

[Snyk] Fix for 24 vulnerabilities Kashuhackerone/agoric-sdk#245

Open

Kashuhackerone mentioned this pull request May 13, 2024

[Snyk] Fix for 2 vulnerabilities Kashuhackerone/agoric-sdk#295

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop importing harden #1244

Stop importing harden #1244

FUDCo commented Jun 30, 2020 •

edited by warner

Loading

warner commented Jun 30, 2020

katelynsills left a comment

michaelfig commented Jul 1, 2020

michaelfig left a comment

kriskowal commented Jul 2, 2020

kriskowal left a comment

warner left a comment •

edited

Loading

warner Jul 2, 2020

warner Jul 2, 2020

michaelfig Jul 2, 2020

warner Jul 2, 2020

warner Jul 2, 2020

warner Jul 2, 2020

michaelfig Jul 2, 2020

phoddie commented Jul 2, 2020

	cd packages/eventual-send
	node -r esm $TAPE/bin/tape-xs-build.js $PWD test/test*.js
	noflake mcconfig -m -p x-cli-lin test-xs-manifest.json
	$MODDABLE/build/bin/lin/release/eventual-send

		/* global Compartment harden */
		// eslint-disable-next-line no-redeclare

Stop importing harden #1244

Stop importing harden #1244

Conversation

FUDCo commented Jun 30, 2020 • edited by warner Loading

warner commented Jun 30, 2020

katelynsills left a comment

Choose a reason for hiding this comment

michaelfig commented Jul 1, 2020

michaelfig left a comment

Choose a reason for hiding this comment

kriskowal commented Jul 2, 2020

kriskowal left a comment

Choose a reason for hiding this comment

warner left a comment • edited Loading

Choose a reason for hiding this comment

warner Jul 2, 2020

Choose a reason for hiding this comment

warner Jul 2, 2020

Choose a reason for hiding this comment

michaelfig Jul 2, 2020

Choose a reason for hiding this comment

warner Jul 2, 2020

Choose a reason for hiding this comment

warner Jul 2, 2020

Choose a reason for hiding this comment

warner Jul 2, 2020

Choose a reason for hiding this comment

michaelfig Jul 2, 2020

Choose a reason for hiding this comment

phoddie commented Jul 2, 2020

FUDCo commented Jun 30, 2020 •

edited by warner

Loading

warner left a comment •

edited

Loading