fix(metering): get all tests working again

Note that classic SES doesn't allow metering of builtins, and @agoric/evaluate has flaky support under ESM.
Agoric · Feb 11, 2020 · f2a3206 · f2a3206
1 parent 15adc38
commit f2a3206
Show file tree

Hide file tree

Showing 15 changed files with 129 additions and 119 deletions.
diff --git a/packages/evaluate/src/index.js b/packages/evaluate/src/index.js
@@ -5,8 +5,7 @@ import makeDefaultEvaluateOptions from '@agoric/default-evaluate-options';
 export const makeEvaluators = (makerOptions = {}) => {
   // Evaluate any shims, globally!
   if (typeof globalThis === 'undefined') {
-    // eslint-disable-next-line no-new-func
-    const myGlobal = Function('return this')();
+    const myGlobal = typeof window === 'undefined' ? global : window;
     myGlobal.globalThis = myGlobal;
   }
   // eslint-disable-next-line no-eval
@@ -94,18 +93,16 @@ export const makeEvaluators = (makerOptions = {}) => {
     // The eval below is direct, so that we have access to the named endowments.
     const scopedEval = `(function() {
       with (arguments[0]) {
-        console.log('endow', arguments[0]);
         return function() {
           'use strict';
-          console.log('src', arguments[0]);
           return eval(arguments[0]);
         };
       }
     })`;
 
     // The eval below is indirect, so that we are only in the global scope.
     // eslint-disable-next-line no-eval
-    return  (1, eval)(scopedEval)(sourceState.endowments)(src);
+    return (1, eval)(scopedEval)(sourceState.endowments)(src);
   };
 
   // We need to make this first so that it is available to the other evaluators.

diff --git a/packages/tame-metering/package.json b/packages/tame-metering/package.json
@@ -2,7 +2,7 @@
   "name": "@agoric/tame-metering",
   "version": "1.0.0",
   "description": "tame-metering",
-  "module": "src/tame.js",
+  "main": "src/tame.js",
   "scripts": {
     "test": "tape -r esm 'test/**/*.js'",
     "lint-fix": "eslint --fix '**/*.{js,jsx}'",

diff --git a/packages/tame-metering/src/tame.js b/packages/tame-metering/src/tame.js
@@ -2,9 +2,9 @@
 import * as c from './constants';
 
 const {
-  defineProperty,
+  defineProperties,
   entries,
-  getOwnPropertyDescriptor,
+  fromEntries,
   getOwnPropertyDescriptors,
 } = Object;
 const { apply, construct, get } = Reflect;
@@ -26,8 +26,8 @@ export default function tameMetering() {
   const getWrapped = (...args) => apply(wmGet, wrapped, args);
 
   /*
-    setWrapped(Error, Error); // FIGME: debugging
-    setWrapped(console, console); // FIGME
+  setWrapped(Error, Error); // FIGME: debugging
+  setWrapped(console, console); // FIGME
   */
   const wrapDescriptor = desc => {
     const newDesc = {};
@@ -48,7 +48,8 @@ export default function tameMetering() {
       return wrapper;
     }
 
-    if (typeof target === 'function') {
+    const isFunction = typeof target === 'function';
+    if (isFunction) {
       // Meter the call to the function/constructor.
       wrapper = function meterFunction(...args) {
         // We're careful not to use the replaceGlobalMeter function as
@@ -63,6 +64,7 @@ export default function tameMetering() {
           // Track the entry of the stack frame.
           globalMeter = null;
           // savedMeter && savedMeter[c.METER_ENTER](undefined, false);
+          savedMeter && savedMeter[c.METER_COMPUTE](undefined, false);
           let ret;
 
           // Reinstall the saved meter for the actual function invocation.
@@ -77,7 +79,6 @@ export default function tameMetering() {
           // Track the allocation of the return value.
           globalMeter = null;
           savedMeter && savedMeter[c.METER_ALLOCATE](ret, false);
-
           return ret;
         } catch (e) {
           // Track the allocation of the exception value.
@@ -87,22 +88,18 @@ export default function tameMetering() {
         } finally {
           // In case a try block consumes stack.
           globalMeter = savedMeter;
+          /*
           try {
             // Declare we left the stack frame.
             globalMeter = null;
-            // savedMeter && savedMeter[c.METER_LEAVE](undefined, false);
+            savedMeter && savedMeter[c.METER_LEAVE](undefined, false);
           } finally {
             // Resume the saved meter, if there was one.
             globalMeter = savedMeter;
           }
+          */
         }
       };
-
-      // Replace the constructor.
-      const proto = get(target, 'prototype');
-      if (proto && getOwnPropertyDescriptor(proto, 'constructor')) {
-        defineProperty(proto, 'constructor', { value: wrapper });
-      }
     } else {
       // Don't redefine the object: mutate in place.
       wrapper = target;
@@ -112,11 +109,21 @@ export default function tameMetering() {
     setWrapped(target, wrapper);
     setWrapped(wrapper, wrapper);
 
-    // Assign the wrapped descriptors to the wrapper.
-    for (const [p, desc] of entries(getOwnPropertyDescriptors(target))) {
-      defineProperty(wrapper, p, wrapDescriptor(desc));
+    if (isFunction) {
+      // Replace the constructor.
+      const proto = get(target, 'prototype');
+      if (proto) {
+        const wproto = wrap(proto);
+        wproto.constructor = wrapper;
+      }
     }
 
+    // Assign the wrapped descriptors to the wrapper.
+    const props = entries(
+      getOwnPropertyDescriptors(target),
+    ).map(([p, desc]) => [p, wrapDescriptor(desc)]);
+    defineProperties(wrapper, fromEntries(props));
+
     return wrapper;
   }
 

diff --git a/packages/transform-metering/src/meter.js b/packages/transform-metering/src/meter.js
@@ -13,7 +13,10 @@ const bigIntZero = bigIntWord && BigInt(0);
 // Stop deducting when we reach a negative number.
 const makeCounter = initBalance => {
   let balance = initBalance;
-  const counter = increment => {
+  const counter = (increment, alwaysDecrement = true) => {
+    if (balance <= 0 && !alwaysDecrement) {
+      return 1;
+    }
     if (balance > 0) {
       balance += increment;
     }
@@ -48,9 +51,9 @@ export function makeComputeMeter(maybeAbort, meter, computeCounter = null) {
     };
   }
   return (cost = 1, throwForever = true) => {
-    const already = maybeAbort(undefined, throwForever);
-    if (!already && computeCounter(-cost) <= 0) {
-      maybeAbort(RangeError(`Compute meter exceeded`), throwForever);
+    maybeAbort(undefined, throwForever);
+    if (computeCounter(-cost, throwForever) <= 0) {
+      throw maybeAbort(RangeError(`Compute meter exceeded`), throwForever);
     }
   };
 }
@@ -63,8 +66,8 @@ export function makeAllocateMeter(maybeAbort, meter, allocateCounter = null) {
     };
   }
   return (value, throwForever = true) => {
+    maybeAbort(undefined, throwForever);
     try {
-      const already = maybeAbort(undefined, throwForever);
       // meter[c.METER_ENTER](undefined, throwForever);
       let cost = 1;
       if (value && ObjectConstructor(value) === value) {
@@ -124,8 +127,8 @@ export function makeAllocateMeter(maybeAbort, meter, allocateCounter = null) {
         }
       }
 
-      if (!already && allocateCounter(-cost, throwForever) <= 0) {
-        maybeAbort(RangeError(`Allocate meter exceeded`), throwForever);
+      if (allocateCounter(-cost, throwForever) <= 0) {
+        throw maybeAbort(RangeError(`Allocate meter exceeded`), throwForever);
       }
       return value;
     } finally {
@@ -143,9 +146,9 @@ export function makeStackMeter(maybeAbort, meter, stackCounter = null) {
   return (cost = 1, throwForever = true) => {
     try {
       meter[c.METER_COMPUTE](undefined, throwForever);
-      const already = maybeAbort(undefined, throwForever);
-      if (!already && stackCounter(-cost, throwForever) <= 0) {
-        maybeAbort(RangeError(`Stack meter exceeded`), throwForever);
+      maybeAbort(undefined, throwForever);
+      if (stackCounter(-cost, throwForever) <= 0) {
+        throw maybeAbort(RangeError(`Stack meter exceeded`), throwForever);
       }
     } catch (e) {
       throw maybeAbort(e, throwForever);

diff --git a/packages/transform-metering/src/transform.js b/packages/transform-metering/src/transform.js
@@ -95,7 +95,7 @@ export function makeMeteringTransformer(
       // Ensure meter identifiers are generated by us, or abort.
       Identifier(path) {
         if (
-          (// FIGME path.node.name === meterId ||
+          (path.node.name === meterId ||
             path.node.name === getMeterId ||
             path.node.name === 'getGlobalMeter' ||
             path.node.name === setMeterId ||

diff --git a/packages/transform-metering/test/test-tame.js b/packages/transform-metering/test/test-tame.js
@@ -9,7 +9,10 @@ import { makeMeter, makeWithMeter } from '../src/index';
 test('meter running', async t => {
   try {
     const { meter, adminFacet } = makeMeter({ budgetCombined: 10 });
-    const { withMeter, withoutMeter } = makeWithMeter(replaceGlobalMeter, meter);
+    const { withMeter, withoutMeter } = makeWithMeter(
+      replaceGlobalMeter,
+      meter,
+    );
     const withMeterFn = (thunk, newMeter = meter) => () =>
       withMeter(thunk, newMeter);
 
@@ -33,17 +36,20 @@ test('meter running', async t => {
       'new Array exhausted',
     );
 
-    adminFacet.combined(20);
-    withMeter(() => {
-      const a = new Array(10);
-      withoutMeter(() =>
-        t.throws(
-          withMeterFn(() => a.map(Object.create)),
-          RangeError,
-          'map to Object create exhausted',
-        ),
-      );
-    });
+    adminFacet.combined(100);
+    t.throws(
+      withMeterFn(() => 'x'.repeat(10000)),
+      RangeError,
+      'long string exhausted',
+    );
+
+    const a = new Array(10);
+    adminFacet.combined(10);
+    t.throws(
+      withMeterFn(() => a.map(Object.create)),
+      RangeError,
+      'map to Object create exhausted',
+    );
   } catch (e) {
     t.isNot(e, e, 'unexpected exception');
   } finally {

diff --git a/packages/transform-metering/test/test-zzz-eval.js b/packages/transform-metering/test/test-zzz-eval.js
@@ -1,42 +1,20 @@
-/* global globalThis */
-/* eslint-disable no-await-in-loop */
+import replaceGlobalMeter from '@agoric/tame-metering/src/install-global-metering';
 
 import test from 'tape-promise/tape';
 import * as babelCore from '@babel/core';
 import SES from 'ses';
-import { makeEvaluators } from '@agoric/evaluate';
 
-import replaceGlobalMeter from '@agoric/tame-metering/src/install-global-metering';
 import { makeMeter, makeMeteredEvaluator } from '../src/index';
 
-export const makeSESEvaluator = opts =>
-  SES.makeSESRootRealm({ ...opts, consoleMode: 'allow' });
-
-export const makeAgoricEvaluator = opts => {
-  const { evaluateProgram } = makeEvaluators(opts);
-  return {
-    evaluate(src, endowments = {}) {
-      return evaluateProgram(src, endowments);
-    },
-  };
-};
-
-export const makeLocalEvaluator = opts => ({
-  evaluate(src, endowments = {}) {
-    // console.log('evaluating src', src);
-    const ss = opts.transforms.reduce(
-      (prior, t) => (t.rewrite ? t.rewrite(prior) : prior),
-      { src, endowments, sourceType: 'script' },
-    );
-
-    globalThis.getGlobalMeter = ss.endowments.getGlobalMeter;
-    // eslint-disable-next-line global-require
-    globalThis.RegExp = require('re2');
+let sesRealm;
+if (!sesRealm) {
+  // FIXME: lockdown() approach is the only way to both secure
+  // this realm and make meters available to evaluators.
+  sesRealm = SES.makeSESRootRealm();
+}
 
-    // eslint-disable-next-line no-eval
-    return (1, eval)(ss.src);
-  },
-});
+export const makeSESEvaluator = opts =>
+  sesRealm.global.Realm.makeCompartment(opts);
 
 test('metering evaluator', async t => {
   const rejectionHandler = (_e, _promise) => {
@@ -45,7 +23,7 @@ test('metering evaluator', async t => {
   try {
     process.on('unhandledRejection', rejectionHandler);
     const { meter, adminFacet } = makeMeter();
-    const makeEvaluator = makeSESEvaluator; // makeSESEvaluator; // ideal
+    const makeEvaluator = makeSESEvaluator; // ideal
     const meteredEval = makeMeteredEvaluator({
       replaceGlobalMeter,
       babelCore,
@@ -89,19 +67,6 @@ test('metering evaluator', async t => {
       return times === 0;
     };
 
-    const src3a = `\
-Promise.resolve().then(
-  () => {
-    while(true) {}
-  });
-0`;
-    expectedExhaustedTimes += 1;
-    await t.rejects(
-      myEval(src3a),
-      /Compute meter exceeded/,
-      'promised infinite loop exhausts',
-    );
-
     const src5a = `\
 ('x'.repeat(1e8), 0)
 `;
@@ -155,6 +120,34 @@ while (true) {}
       'nested loop exhausts',
     );
 
+    const src3a = `\
+Promise.resolve().then(
+  () => {
+    while(true) {}
+  });
+0
+`;
+    expectedExhaustedTimes += 1;
+    await t.rejects(
+      myEval(src3a),
+      /Compute meter exceeded/,
+      'promised infinite loop exhausts',
+    );
+
+    const src3c = `\
+function f() {
+  Promise.resolve().then(f);
+}
+f();
+0
+`;
+    expectedExhaustedTimes += 1;
+    await t.rejects(
+      myEval(src3c),
+      /Compute meter exceeded/,
+      'promise loop exhausts',
+    );
+
     const src4 = `\
 /(x+x+)+y/.test('x'.repeat(10000));
 `;
@@ -168,7 +161,11 @@ while (true) {}
 new Array(1e8).map(Object.create); 0
 `;
     failedToRejectUnderSES() ||
-    await t.rejects(myEval(src6), /Allocate meter exceeded/, 'long map exhausts');
+      (await t.rejects(
+        myEval(src6),
+        /Allocate meter exceeded/,
+        'long map exhausts',
+      ));
 
     t.equals(
       exhaustedTimes,

diff --git a/packages/transform-metering/testdata/arrow-function-block/rewrite.js b/packages/transform-metering/testdata/arrow-function-block/rewrite.js
@@ -1,3 +1,3 @@
-const $m=$h‍_enterMeter();try{() => {const $m = $h‍_enterMeter();try {
-    f();} finally {$m.l();}};
-}finally{$m.l()}
+const $h‍_meter_get=getGlobalMeter;const $m=$h‍_meter_set(true);$m&&$m.e();try{() => {const $m = $h‍_meter_get();$m && $m.e();try {
+    f();} finally {$m && $m.l();}};
+}finally{$h‍_meter_set(false);$m && $m.l();}
diff --git a/packages/transform-metering/testdata/arrow-function-expression/rewrite.js b/packages/transform-metering/testdata/arrow-function-expression/rewrite.js
@@ -1,3 +1,3 @@
-const $m=$h‍_enterMeter();try{() => {const $m = $h‍_enterMeter();try {return (
-      f());} finally {$m.l();}};
-}finally{$m.l()}
+const $h‍_meter_get=getGlobalMeter;const $m=$h‍_meter_set(true);$m&&$m.e();try{() => {const $m = $h‍_meter_get();$m && $m.e();try {return (
+      f());} finally {$m && $m.l();}};
+}finally{$h‍_meter_set(false);$m && $m.l();}