Skip to content

Commit

Permalink
fix: endow with original unstructured assert (#2323)
Browse files Browse the repository at this point in the history
Closes: #XXXX
Refs: Agoric/agoric-sdk#9515
Agoric/agoric-sdk#9514

## Description

Addresses Agoric/agoric-sdk#9515 as applied to
endo, by doing for endo what
Agoric/agoric-sdk#9514 does for agoric-sdk

To address Agoric/agoric-sdk#5672 for endo ,
we should change all applicable uses of `assert` to obtain `assert` by
importing it from the `@endo/errors` package. However, attempts to do so
ran into the symptoms reported at
Agoric/agoric-sdk#9515 for the reasons
reported there -- accidentally using the imported `assert` as the
endowment for the global `assert` of new Compartments.

This PR prepares the ground for these fixes to
Agoric/agoric-sdk#5672 for endo by
unambiguously endowing with the original unstructured
`globalThis.assert`, which will remain the correct endowment even when
other uses of `assert` have migrated to the one imported from
`@endo/errors`. By itself, this PR, preceding those fixes to
Agoric/agoric-sdk#5672 for endo , is not
actually fixing a bug in the sense of a behavioral change. Reviewers,
let me know if you think this PR should be labeled a "refactor" because
of this.


### Security Considerations

none
### Scaling Considerations

none
### Documentation Considerations

anyone gathering endowments for a new compartment should be aware of
this issue, and be sure to use `globalThis.assert` as the `assert`
endowment. Fortunately, this will only be of concern to advanced
developers.

### Testing Considerations

Since this PR doesn't actually cause any behavioral change, it cannot be
tested in place. Rather, its test will be whether #2324 still passes CI
once rebased on this PR.

***Update***: #2324 is now passing CI well enough to consider this PR
(#2323) to be adequately tested.


### Compatibility Considerations

The point. This PR itself only prepares the ground for the equivalent of
Agoric/agoric-sdk#9513 for endo. By itself it
has no other effect, and so no other compat issues.

### Upgrade Considerations

none
  • Loading branch information
erights authored Jun 17, 2024
1 parent 8355aba commit 8b2bedb
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 15 deletions.
5 changes: 3 additions & 2 deletions packages/cli/src/commands/run.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global process */
/* global globalThis, process */
import url from 'url';
import os from 'os';
import { E, Far } from '@endo/far';
Expand All @@ -10,7 +10,8 @@ import { withEndoAgent } from '../context.js';
import { parsePetNamePath } from '../pet-name.js';

const endowments = harden({
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
E,
Far,
makeExo,
Expand Down
5 changes: 4 additions & 1 deletion packages/compartment-mapper/test/main.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global globalThis */

import 'ses';
import test from 'ava';
import { loadLocation, importArchive, makeBundle } from '../index.js';
Expand Down Expand Up @@ -96,7 +98,8 @@ test('makeBundle / importArchive', async t => {
TextEncoder,
TextDecoder,
URL,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
});
const evasiveArchiverBundle = archiverBundle
.replace(/(?<!\.)\bimport\b(?![:"'])/g, 'IMPORT')
Expand Down
5 changes: 3 additions & 2 deletions packages/daemon/src/web-page.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-check
/* global window, document */
/* global globalThis, window, document */

import '@endo/init/debug.js';
import { makeCapTP } from '@endo/captp';
Expand Down Expand Up @@ -56,7 +56,8 @@ const collectPropsAndBind = target => {
};

const hardenedEndowments = harden({
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
E,
Far,
makeExo,
Expand Down
4 changes: 3 additions & 1 deletion packages/daemon/src/worker.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* global globalThis */
// @ts-check
/// <reference types="ses"/>

Expand All @@ -12,7 +13,8 @@ import { WorkerFacetForDaemonInterface } from './interfaces.js';
/** @import { EndoReadable, MignonicPowers } from './types.js' */

const endowments = harden({
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
console,
E,
Far,
Expand Down
13 changes: 8 additions & 5 deletions packages/eventual-send/src/track-turns.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import {
} from '@endo/env-options';

// NOTE: We can't import these because they're not in scope before lockdown.
// import { assert, details as X, Fail } from '@agoric/assert';
// We also cannot currently import them because it would create a cyclic
// dependency, though this is more easily fixed.
// import { assert, X, Fail } from '@endo/errors';
// See also https://github.com/Agoric/agoric-sdk/issues/9515

// WARNING: Global Mutable State!
// This state is communicated to `assert` that makes it available to the
Expand Down Expand Up @@ -33,7 +36,7 @@ const ENABLED =

const addRejectionNote = detailsNote => reason => {
if (reason instanceof Error) {
assert.note(reason, detailsNote);
globalThis.assert.note(reason, detailsNote);
}
if (VERBOSE) {
console.log('REJECTED at top of event loop', reason);
Expand All @@ -52,7 +55,7 @@ const wrapFunction =
result = func(...args);
} catch (err) {
if (err instanceof Error) {
assert.note(
globalThis.assert.note(
err,
X`Thrown from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
Expand Down Expand Up @@ -90,14 +93,14 @@ export const trackTurns = funcs => {
if (!ENABLED || typeof globalThis === 'undefined' || !globalThis.assert) {
return funcs;
}
const { details: X } = assert;
const { details: X, note: annotateError } = globalThis.assert;

hiddenCurrentEvent += 1;
const sendingError = Error(
`Event: ${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
if (hiddenPriorError !== undefined) {
assert.note(sendingError, X`Caused by: ${hiddenPriorError}`);
annotateError(sendingError, X`Caused by: ${hiddenPriorError}`);
}

return /** @type {T} */ (
Expand Down
5 changes: 3 additions & 2 deletions packages/ses/demos/challenge/main.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global window, document */
/* global globalThis, window, document */
/* eslint-disable no-plusplus */

// two approaches:
Expand Down Expand Up @@ -152,7 +152,8 @@ lockdown();
harden(guess);
const compartment = new Compartment({
console,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
guess,
...dateEndowment,
});
Expand Down
8 changes: 6 additions & 2 deletions packages/ses/demos/console/main.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* globals document */
/* globals globalThis, document */

lockdown();
{
Expand All @@ -13,7 +13,11 @@ lockdown();

// Under the default `lockdown` settings, it is safe enough
// to endow with the safe `console`.
const compartment = new Compartment({ console, assert });
const compartment = new Compartment({
console,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
});

execute.addEventListener('click', () => {
const sourceText = input.value;
Expand Down

0 comments on commit 8b2bedb

Please sign in to comment.