fix: remove obsolete "unwrap" (#1360)

Agoric · Aug 8, 2020 · 5796e0e · 5796e0e
1 parent 184c912
commit 5796e0e
Show file tree

Hide file tree

Showing 7 changed files with 5 additions and 151 deletions.
diff --git a/packages/eventual-send/NEWS.md b/packages/eventual-send/NEWS.md
@@ -1,5 +1,9 @@
 User-visible changes in eventual-send:
 
+## Release ???
+
+Removed obsolete `HandledPromise.unwrap` and `E.unwrap` functions.
+
 ## Release 0.5.0 (17-Dec-2019)
 
 Implement updated `HandledPromise` interface (#6):
@@ -13,4 +17,3 @@ Implement updated `HandledPromise` interface (#6):
 
 Moved from https://github.com/Agoric/eventual-send into the monorepo at
 https://github.com/Agoric/agoric-sdk .
-
diff --git a/packages/eventual-send/src/E.js b/packages/eventual-send/src/E.js
@@ -67,7 +67,6 @@ export default function makeE(HandledPromise) {
 
   E.G = makeEGetterProxy;
   E.resolve = HandledPromise.resolve;
-  E.unwrap = HandledPromise.unwrap;
 
   E.when = (x, onfulfilled = undefined, onrejected = undefined) =>
     HandledPromise.resolve(x).then(onfulfilled, onrejected);

diff --git a/packages/eventual-send/src/index.js b/packages/eventual-send/src/index.js
@@ -51,7 +51,7 @@ export function makeHandledPromise(Promise) {
   let presenceToHandler;
   let presenceToPromise;
   let promiseToUnsettledHandler;
-  let promiseToPresence; // only for HandledPromise.unwrap
+  let promiseToPresence;
   let forwardedPromiseToPromise; // forwarding, union-find-ish
   function ensureMaps() {
     if (!presenceToHandler) {
@@ -353,43 +353,6 @@ export function makeHandledPromise(Promise) {
         promiseResolve().then(_ => new HandledPromise(executeThen)),
       );
     },
-    // TODO verify that this is safe to provide universally, i.e.,
-    // that by itself it doesn't provide access to mutable state in
-    // ways that violate normal ocap module purity rules. The claim
-    // that it does not rests on the handled promise itself being
-    // necessary to perceive this mutable state. In that sense, we
-    // can think of the right to perceive it, and of access to the
-    // target, as being in the handled promise. Note that a .then on
-    // the handled promise will already provide async access to the
-    // target, so the only additional authorities are: 1)
-    // synchronous access for handled promises only, and thus 2) the
-    // ability to tell, from the client side, whether a promise is
-    // handled. Or, at least, the ability to tell given that the
-    // promise is already fulfilled.
-    unwrap(value) {
-      // This check for Thenable is safe, since in a remote-object
-      // environment, our comms system will defend against remote
-      // objects being represented as a tricky local Proxy, otherwise
-      // it is guaranteed to be local and therefore synchronous enough.
-      if (Object(value) !== value || !('then' in value)) {
-        // Not a Thenable, so return it.
-        // This means that local objects will pass through without error.
-        return value;
-      }
-
-      // Try to look up the HandledPromise.
-      ensureMaps();
-      const pr = presenceToPromise.get(value) || value;
-
-      // Find the fulfilled presence for that HandledPromise.
-      const presence = promiseToPresence.get(pr);
-      if (!presence) {
-        throw TypeError(
-          `Value is a Thenble but not a HandledPromise fulfilled to a presence`,
-        );
-      }
-      return presence;
-    },
   });
 
   defineProperties(HandledPromise, getOwnPropertyDescriptors(staticMethods));

diff --git a/packages/eventual-send/test/test-e.js b/packages/eventual-send/test/test-e.js
@@ -5,7 +5,6 @@ import { E, HandledPromise } from '../src/index';
 test('E reexports', async t => {
   try {
     t.equals(E.resolve, HandledPromise.resolve, 'E reexports resolve');
-    t.equals(E.unwrap, HandledPromise.unwrap, 'E reexports unwrap');
   } catch (e) {
     t.isNot(e, e, 'unexpected exception');
   } finally {

diff --git a/packages/eventual-send/test/test-hp.js b/packages/eventual-send/test/test-hp.js
@@ -46,112 +46,6 @@ test('chained properties', async t => {
   }
 });
 
-test('HandledPromise.unwrap', async t => {
-  try {
-    for (const [val, desc] of [
-      [{}, 'object'],
-      [true, 'true'],
-      [false, 'false'],
-      [undefined, 'undefined'],
-      [null, 'null'],
-      [123, 'number'],
-      ['hello', 'string'],
-    ]) {
-      t.equal(HandledPromise.unwrap(val), val, `unwrapped ${desc} is equal`);
-    }
-    const t0 = {
-      then() {},
-    };
-    t.throws(
-      () => HandledPromise.unwrap(t0),
-      TypeError,
-      `unwrapped thenable object throws`,
-    );
-    const t1 = () => {};
-    t1.then = () => {};
-    t.throws(
-      () => HandledPromise.unwrap(t1),
-      TypeError,
-      `unwrapped thenable function throws`,
-    );
-    const p0 = new Promise(_ => {});
-    t.throws(
-      () => HandledPromise.unwrap(p0),
-      TypeError,
-      `unwrapped unfulfilled Promise throws`,
-    );
-    const p1 = new Promise(resolve => {
-      resolve({});
-    });
-    t.throws(
-      () => HandledPromise.unwrap(p1),
-      TypeError,
-      `unwrapped resolved Promise throws`,
-    );
-    const p2 = new Promise((_, reject) => {
-      reject(Error('p2'));
-    });
-    // Prevent unhandled promise rejection.
-    p2.catch(_ => {});
-    t.throws(
-      () => HandledPromise.unwrap(p2),
-      TypeError,
-      `unwrapped rejected Promise throws`,
-    );
-    const hp0 = new HandledPromise(_ => {});
-    t.throws(
-      () => HandledPromise.unwrap(hp0),
-      TypeError,
-      'unfulfilled HandledPromise throws',
-    );
-    const hp1 = new HandledPromise(resolve => {
-      resolve({});
-    });
-    t.throws(
-      () => HandledPromise.unwrap(hp1),
-      TypeError,
-      'resolved HandledPromise throws',
-    );
-    const hp2 = new HandledPromise((_, reject) => {
-      reject(Error('hp2'));
-    });
-    // Prevent unhandled promise rejection.
-    hp2.catch(_ => {});
-    t.throws(
-      () => HandledPromise.unwrap(hp2),
-      TypeError,
-      'rejected HandledPromise throws',
-    );
-    let presence;
-    const hp3 = new HandledPromise((_res, _rej, resolveWithPresence) => {
-      presence = resolveWithPresence({});
-    });
-    t.equals(typeof presence, 'object', `typeof presence is object`);
-    t.equals(
-      HandledPromise.unwrap(hp3),
-      presence,
-      `unwrapped HandledPromise is presence`,
-    );
-    t.equals(
-      HandledPromise.unwrap(presence),
-      presence,
-      `unwrapped presence is presence`,
-    );
-    const hp4 = new HandledPromise(resolve => {
-      resolve(hp3);
-    });
-    t.equals(
-      HandledPromise.unwrap(hp4),
-      presence,
-      `unwrapped forwarded HandledPromise is presence`,
-    );
-  } catch (e) {
-    t.isNot(e, e, 'unexpected exception');
-  } finally {
-    t.end();
-  }
-});
-
 test('no local stalls', async t => {
   const log = [];
   const target = {

diff --git a/packages/produce-promise/src/producePromise.js b/packages/produce-promise/src/producePromise.js
@@ -34,8 +34,6 @@ export function producePromise() {
   /** @type {(reason: any) => void} */
   let rej = NOOP_INITIALIZER;
 
-  // We use a HandledPromise so that we can run HandledPromise.unwrap(p)
-  // even if p doesn't travel through a comms system (like SwingSet's).
   const p = new HandledPromise((resolve, reject) => {
     res = resolve;
     rej = reject;

diff --git a/packages/promise-kit/src/promiseKit.js b/packages/promise-kit/src/promiseKit.js
@@ -42,8 +42,6 @@ export function makePromiseKit() {
   /** @type {(reason: any) => void} */
   let rej = NOOP_INITIALIZER;
 
-  // We use a HandledPromise so that we can run HandledPromise.unwrap(p)
-  // even if p doesn't travel through a comms system (like SwingSet's).
   const p = new HandledPromise((resolve, reject) => {
     res = resolve;
     rej = reject;