Skip to content

Commit

Permalink
fix: endow with original unstructured assert (#9514)
Browse files Browse the repository at this point in the history
closes: #9515 
refs: #5672 #8332 #9513  endojs/endo#2323

## Description

To address #5672 , we should change all uses of `assert` to obtain `assert` by importing it from the `@endo/errors` package. However, attempts to do so (#8332 #9513) ran into the symptoms reported at #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 #5672 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 #5672 , 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 #9513 still passes CI once rebased on this PR.

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

### Upgrade Considerations
This PR by itself does not have any dependence on @endo/errors existing, and so should be compatible even when linked against fairly ancient versions of endo.
  • Loading branch information
erights authored Jun 17, 2024
1 parent 24665a9 commit f908f89
Show file tree
Hide file tree
Showing 14 changed files with 47 additions and 16 deletions.
3 changes: 2 additions & 1 deletion packages/SwingSet/src/controller/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ export async function makeSwingsetController(
filePrefix: 'kernel/...',
endowments: {
console: makeConsole(`${debugPrefix}SwingSet:kernel`),
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
require: kernelRequire,
URL: globalThis.Base64, // Unavailable only on XSnap
Base64: globalThis.Base64, // Available only on XSnap
Expand Down
9 changes: 8 additions & 1 deletion packages/SwingSet/src/kernel/kernel.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global globalThis */

import { assert, Fail } from '@agoric/assert';
import { isNat } from '@endo/nat';
import { importBundle } from '@endo/import-bundle';
Expand Down Expand Up @@ -1644,7 +1646,12 @@ export default function buildKernel(
assert(bundle);
const NS = await importBundle(bundle, {
filePrefix: `dev-${name}/...`,
endowments: harden({ ...vatEndowments, console: devConsole, assert }),
endowments: harden({
...vatEndowments,
console: devConsole,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
}),
});

if (deviceEndowments[name] || unendowed) {
Expand Down
3 changes: 2 additions & 1 deletion packages/SwingSet/src/kernel/vat-loader/manager-local.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ export function makeLocalVatManagerFactory({
const workerEndowments = harden({
...vatEndowments,
console: makeVatConsole(makeLogMaker('vat')),
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
TextEncoder,
TextDecoder,
Base64: globalThis.Base64, // Available only on XSnap
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global WeakRef, FinalizationRegistry */
/* global globalThis, WeakRef, FinalizationRegistry */

// this file is loaded at the start of a new subprocess
import '@endo/init';
Expand Down Expand Up @@ -139,7 +139,8 @@ function handleSetBundle(margs) {
// Enable or disable the console accordingly.
const workerEndowments = {
console: makeVatConsole(makeLogMaker(`SwingSet:vat:${vatID}`)),
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
};

async function buildVatNamespace(lsEndowments, inescapableGlobalProperties) {
Expand Down
10 changes: 7 additions & 3 deletions packages/SwingSet/test/zcf-ish-upgrade/pseudo-zcf.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// @ts-nocheck
/* global VatData */
/* global globalThis, VatData */
/* eslint-disable no-unused-vars */

import { Far } from '@endo/far';
import { importBundle } from '@endo/import-bundle';
import { defineDurableKind } from '@agoric/vat-data';
import { assert } from '@agoric/assert';
import {
provideHandle,
provideBaggageSubset as provideBaggageSubTree,
Expand All @@ -24,7 +23,12 @@ export const buildRootObject = async (vatPowers, vatParameters, baggage) => {
const { D } = vatPowers;
const { contractBundleCap } = vatParameters;
const contractBundle = D(contractBundleCap).getBundle();
const endowments = { console, assert, VatData };
const endowments = {
console,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
VatData,
};
const contractNS = await importBundle(contractBundle, { endowments });
const { setupInstallation, setup: _ } = contractNS;
if (!setupInstallation) {
Expand Down
5 changes: 4 additions & 1 deletion packages/cosmic-swingset/scripts/clean-core-eval.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#! /usr/bin/env node
/* global globalThis */

import '@endo/init/debug.js';
import * as farExports from '@endo/far';
import { isEntrypoint } from '../src/helpers/is-entrypoint.js';
Expand All @@ -12,7 +14,8 @@ export const compartmentEvaluate = code => {
const globals = harden({
...modules,
...farExports,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
console: {
// Ensure we don't pollute stdout.
debug: console.warn,
Expand Down
3 changes: 2 additions & 1 deletion packages/spawner/src/vat-spawned.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { Far } from '@endo/marshal';
const endowments = {
VatData,
console,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
Base64: globalThis.Base64, // Present only on XSnap
URL: globalThis.URL, // Absent only on XSnap
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ function makeWorker(port) {

const workerEndowments = {
console: makeVatConsole(makeLogMaker('vat')),
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
// bootstrap provides HandledPromise
HandledPromise: globalThis.HandledPromise,
TextEncoder,
Expand Down
3 changes: 2 additions & 1 deletion packages/vats/src/core/chain-behaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export const bridgeCoreEval = async allPowers => {
const endowments = {
VatData: globalThis.VatData,
console,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
Base64: globalThis.Base64, // Present only on XSnap
URL: globalThis.URL, // Absent only on XSnap
};
Expand Down
2 changes: 2 additions & 0 deletions packages/vats/src/core/core-eval-env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ declare global {

// endowments
var VatData: VatData;
// This is correctly the `Assert` type from `'ses'`, not from @endo/errors
// See https://github.com/Agoric/agoric-sdk/issues/9515
var assert: Assert;

// console is a VirtualConsole but this directive fails to override the extant global `console`
Expand Down
5 changes: 4 additions & 1 deletion packages/vats/src/repl.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global globalThis */

import { isPromise } from '@endo/promise-kit';
import { Far } from '@endo/far';
import { V as E } from '@agoric/vow/vat.js';
Expand Down Expand Up @@ -194,7 +196,8 @@ export function getReplHandler(replObjects, send) {
...farExports,
...vowExports,
E,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
console: replConsole,
commands,
history,
Expand Down
2 changes: 2 additions & 0 deletions packages/xsnap/src/avaAssertXS.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ function makeTester(htest, out) {
fail(message) {
assert(false, message);
},
// Not the SES or @endo/errors `assert`
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert,
truthy,
/**
Expand Down
6 changes: 5 additions & 1 deletion packages/xsnap/test/boot-lockdown.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global globalThis */

import test from 'ava';

import * as proc from 'child_process';
Expand Down Expand Up @@ -173,7 +175,9 @@ test('console - objects should include detail', async t => {
Error('oops!'),
];

const { Fail } = assert;
// Using `globalThis.assert` because of delayed SES lockdown
// See https://github.com/Agoric/agoric-sdk/issues/9515
const { Fail } = globalThis.assert;

try {
Fail`assertion text ${richStructure}`;
Expand Down
4 changes: 2 additions & 2 deletions packages/zoe/src/contractFacet/evalContractCode.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
/* global globalThis */

import { importBundle } from '@endo/import-bundle';
import { assert } from '@agoric/assert';
import { handlePWarning } from '../handleWarning.js';

const evalContractBundle = (bundle, additionalEndowments = {}) => {
Expand All @@ -16,7 +15,8 @@ const evalContractBundle = (bundle, additionalEndowments = {}) => {

const defaultEndowments = {
console: louderConsole,
assert,
// See https://github.com/Agoric/agoric-sdk/issues/9515
assert: globalThis.assert,
VatData: globalThis.VatData,
};

Expand Down

0 comments on commit f908f89

Please sign in to comment.