Warn for remotables not declared Remotable or Far (#3504)

* fix: Declare more remotables. Deprecate and warn on undeclared ones.

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

@@ -494,7 +494,7 @@ export function makeNetworkProtocol(protocolHandler) {
   E(protocolHandler).onCreate(protocolImpl, protocolHandler);
 
   // Return the user-facing protocol.
-  return harden({ bind });
+  return Far('binder', { bind });
 }
 
 /**

packages/SwingSet/src/vats/vat-timerWrapper.js

@@ -34,7 +34,7 @@ export function buildRootObject(vatPowers) {
 
         const index = D(timerNode).makeRepeater(delaySecs, interval);
 
-        const vatRepeater = harden({
+        const vatRepeater = Far('vatRepeater', {
           schedule(h) {
             return D(timerNode).schedule(index, h);
           },
@@ -55,7 +55,7 @@ export function buildRootObject(vatPowers) {
 
         const index = D(timerNode).makeRepeater(delaySecs, interval);
         const { notifier, updater } = makeNotifierKit();
-        const updateHandler = harden({
+        const updateHandler = Far('updateHandler', {
           wake: updater.updateState,
         });
         D(timerNode).schedule(index, updateHandler);

packages/SwingSet/test/test-network.js

@@ -1,7 +1,8 @@
+// eslint-disable-next-line import/order
 import { test } from '../tools/prepare-test-env-ava.js';
 
-// eslint-disable-next-line import/order
 import { makePromiseKit } from '@agoric/promise-kit';
+import { Far } from '@agoric/marshal';
 
 import {
   parse,
@@ -26,7 +27,7 @@ const makeProtocolHandler = t => {
   let l;
   let lp;
   let nonce = 0;
-  return harden({
+  return Far('ProtocolHandler', {
     async onCreate(_protocol, _impl) {
       log('created', _protocol, _impl);
     },
@@ -80,7 +81,7 @@ test('handled protocol', async t => {
   const port = await protocol.bind('/ibc/*/ordered');
   await port.connect(
     '/ibc/*/ordered/echo',
-    harden({
+    Far('ProtocolHandler', {
       async onOpen(connection, localAddr, remoteAddr) {
         t.is(localAddr, '/ibc/*/ordered');
         t.is(remoteAddr, '/ibc/*/ordered/echo');
@@ -113,7 +114,7 @@ test('protocol connection listen', async t => {
   /**
    * @type {import('../src/vats/network').ListenHandler}
    */
-  const listener = harden({
+  const listener = Far('listener', {
     async onListen(p, listenHandler) {
       t.is(p, port, `port is tracked in onListen`);
       t.assert(listenHandler, `listenHandler is tracked in onListen`);
@@ -199,7 +200,7 @@ test('loopback protocol', async t => {
   /**
    * @type {import('../src/vats/network').ListenHandler}
    */
-  const listener = harden({
+  const listener = Far('listener', {
     async onAccept(_p, _localAddr, _remoteAddr, _listenHandler) {
       return harden({
         async onReceive(c, packet, _connectionHandler) {
@@ -214,7 +215,7 @@ test('loopback protocol', async t => {
   const port2 = await protocol.bind('/loopback/bar');
   await port2.connect(
     port.getLocalAddress(),
-    harden({
+    Far('opener', {
       async onOpen(c, localAddr, remoteAddr, _connectionHandler) {
         t.is(localAddr, '/loopback/bar/nonce/1');
         t.is(remoteAddr, '/loopback/foo/nonce/2');

packages/SwingSet/test/test-timer-device.js

@@ -1,5 +1,7 @@
 import { test } from '../tools/prepare-test-env-ava.js';
 
+// eslint-disable-next-line import/order
+import { Far } from '@agoric/marshal';
 import { makeTimerMap, curryPollFn } from '../src/devices/timer-src.js';
 
 test('multiMap multi store', t => {
@@ -39,17 +41,17 @@ test('multiMap remove key', t => {
 });
 
 function fakeSO(o) {
-  return {
+  return Far('fake SO', {
     wake(arg = null) {
       o.wake(arg);
     },
-  };
+  });
 }
 
 function makeHandler() {
   let calls = 0;
   const args = [];
-  return {
+  return Far('wake handler', {
     getCalls() {
       return calls;
     },
@@ -60,19 +62,19 @@ function makeHandler() {
       args.push(arg);
       calls += 1;
     },
-  };
+  });
 }
 
 function makeFakeTimer(initialVal) {
   let fakeTime = initialVal;
-  return {
+  return Far('fake timer', {
     getLastPolled() {
       return fakeTime;
     },
     setTime(t) {
       fakeTime = t;
     },
-  };
+  });
 }
 
 test('Timer schedule single event', t => {

packages/dapp-svelte-wallet/api/src/lib-wallet.js

@@ -1130,7 +1130,7 @@ export function makeWallet({
       brand,
       issuer: undefined,
       status: undefined,
-      actions: {
+      actions: Far('payment actions', {
         async deposit(purseOrPetname = undefined) {
           /** @type {Purse} */
           let purse;
@@ -1226,7 +1226,7 @@ export function makeWallet({
           updatePaymentRecord(paymentRecord);
           return true;
         },
-      },
+      }),
     };
 
     payments.init(payment, harden(paymentRecord));

packages/dapp-svelte-wallet/api/src/wallet.js

@@ -122,7 +122,7 @@ export function buildRootObject(_vatPowers) {
     }
 
     /** @type {WalletBridge} */
-    const bridge = {
+    const bridge = Far('bridge', {
       async getPursesNotifier() {
         await approve();
         const pursesNotifier = walletAdmin.getAttenuatedPursesNotifier();
@@ -209,8 +209,8 @@ export function buildRootObject(_vatPowers) {
         await approve();
         return walletAdmin.getBrandPetnames(brands);
       },
-    };
-    return harden(bridge);
+    });
+    return bridge;
   };
 
   /**
@@ -220,7 +220,7 @@ export function buildRootObject(_vatPowers) {
    *
    * @type {WalletBridge}
    */
-  const preapprovedBridge = {
+  const preapprovedBridge = Far('preapprovedBridge', {
     addOffer(offer) {
       return walletAdmin.addOffer(offer);
     },
@@ -266,7 +266,7 @@ export function buildRootObject(_vatPowers) {
     async getBrandPetnames(brands) {
       return walletAdmin.getBrandPetnames(brands);
     },
-  };
+  });
   harden(preapprovedBridge);
 
   async function getWallet(bank) {
@@ -281,7 +281,7 @@ export function buildRootObject(_vatPowers) {
      *
      * @type {WalletUser & { getAdminFacet: () => WalletAdminFacet }}
      */
-    const wallet = {
+    const wallet = Far('wallet', {
       addPayment: walletAdmin.addPayment,
       async getScopedBridge(suggestedDappPetname, dappOrigin) {
         const approve = async () => {
@@ -298,13 +298,13 @@ export function buildRootObject(_vatPowers) {
       },
       getDepositFacetId: walletAdmin.getDepositFacetId,
       getAdminFacet() {
-        return harden({ ...walletAdmin, ...notifiers });
+        return Far('adminFacet', { ...walletAdmin, ...notifiers });
       },
       getIssuer: walletAdmin.getIssuer,
       getIssuers: walletAdmin.getIssuers,
       getPurse: walletAdmin.getPurse,
       getPurses: walletAdmin.getPurses,
-    };
+    });
     return harden(wallet);
   }
 
@@ -342,7 +342,7 @@ export function buildRootObject(_vatPowers) {
   }
 
   function getBridgeURLHandler() {
-    return harden({
+    return Far('bridgeURLHandler', {
       /**
        * @typedef {Object} WalletOtherSide the callbacks from a CapTP wallet
        * client.
@@ -402,7 +402,7 @@ export function buildRootObject(_vatPowers) {
        * methods directly.  Then we would like to deprecate this handler.
        */
       getCommandHandler() {
-        return harden({
+        return Far('commandHandler', {
           onOpen(_obj, meta) {
             bridgeHandles.add(meta.channelHandle);
           },

packages/dapp-svelte-wallet/api/test/test-getPursesNotifier.js

@@ -8,18 +8,19 @@ import fakeVatAdmin from '@agoric/zoe/tools/fakeVatAdmin';
 import { makeBoard } from '@agoric/vats/src/lib-board';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { makeNameHubKit } from '@agoric/vats/src/nameHub';
+import { Far } from '@agoric/marshal';
 import { makeWallet } from '../src/lib-wallet';
 
 import '../src/types';
 
 function makeFakeMyAddressNameAdmin() {
   const { nameAdmin: rawMyAddressNameAdmin } = makeNameHubKit();
-  return {
+  return Far('fakeMyAddressNameAdmin', {
     ...rawMyAddressNameAdmin,
     getMyAddress() {
       return 'agoric1test1';
     },
-  };
+  });
 }
 
 const setup = async () => {

packages/dapp-svelte-wallet/api/test/test-lib-wallet.js

@@ -15,6 +15,7 @@ import { E } from '@agoric/eventual-send';
 import { makeBoard } from '@agoric/vats/src/lib-board';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { makeNameHubKit } from '@agoric/vats/src/nameHub';
+import { Far } from '@agoric/marshal';
 import { makeWallet } from '../src/lib-wallet';
 
 import '../src/types';
@@ -23,12 +24,12 @@ const ZOE_INVITE_PURSE_PETNAME = 'Default Zoe invite purse';
 
 function makeFakeMyAddressNameAdmin() {
   const { nameAdmin: rawMyAddressNameAdmin } = makeNameHubKit();
-  return {
+  return Far('fakeMyAddressNameAdmin', {
     ...rawMyAddressNameAdmin,
     getMyAddress() {
       return 'agoric1test1';
     },
-  };
+  });
 }
 
 async function setupTest() {

packages/marshal/src/passStyleOf.js

@@ -16,7 +16,9 @@ import '@agoric/assert/exported.js';
 //
 // TODO: once the policy changes to force remotables to be explicit, remove this
 // flag entirely and fix code that uses it (as if it were always `false`).
-const ALLOW_IMPLICIT_REMOTABLES = true;
+//
+// Exported only for testing during the transition
+export const ALLOW_IMPLICIT_REMOTABLES = true;
 
 const {
   getPrototypeOf,
@@ -146,7 +148,7 @@ function isPassByCopyArray(val) {
  */
 function isPassByCopyRecord(val) {
   const proto = getPrototypeOf(val);
-  if (proto !== objectPrototype) {
+  if (proto !== objectPrototype && proto !== null) {
     return false;
   }
   const descs = getOwnPropertyDescriptors(val);
@@ -236,17 +238,29 @@ export { assertIface };
  *   [Symbol.toStringTag]: string,
  *   toString: () => void }} val the value to verify
  * @param {Checker} [check]
+ * @param {any} original for better diagnostics
  * @returns {boolean}
  */
-const checkRemotableProto = (val, check = x => x) => {
+const checkRemotableProto = (val, check = x => x, original = undefined) => {
   if (
     !(
       check(
         typeof val === 'object',
         X`cannot serialize non-objects like ${val}`,
       ) &&
       check(!Array.isArray(val), X`Arrays cannot be pass-by-remote`) &&
-      check(val !== null, X`null cannot be pass-by-remote`)
+      check(val !== null, X`null cannot be pass-by-remote`) &&
+      check(
+        // Since we're working with TypeScript's unsound type system, mostly
+        // to catch accidents and to provide IDE support, we type arguments
+        // like `val` according to what they are supposed to be. The following
+        // tests for a particular violation. However, TypeScript complains
+        // because *if the declared type were accurate*, then the condition
+        // would always return true.
+        // @ts-ignore TypeScript assumes what we're trying to check
+        val !== Object.prototype,
+        X`Remotables must now be explicitly declared: ${q(original)}`,
+      )
     )
   ) {
     return false;
@@ -362,9 +376,13 @@ function checkRemotable(val, check = x => x) {
   const p = getPrototypeOf(val);
 
   if (ALLOW_IMPLICIT_REMOTABLES && (p === null || p === objectPrototype)) {
+    const err = assert.error(
+      X`Remotables should be explicitly declared: ${q(val)}`,
+    );
+    console.warn('Missing Far:', err);
     return true;
   }
-  return checkRemotableProto(p, check);
+  return checkRemotableProto(p, check, val);
 }
 
 /**
@@ -390,12 +408,11 @@ harden(getInterfaceOf);
 export { getInterfaceOf };
 
 /**
- * objects can only be passed in one of two/three forms:
+ * objects can only be passed in one of two forms:
  * 1: pass-by-remote: all properties (own and inherited) are methods,
  *    the object itself is of type object, not function
  * 2: pass-by-copy: all string-named own properties are data, not methods
  *    the object must inherit from objectPrototype or null
- * 3: the empty object is pass-by-remote, for identity comparison
  *
  * all objects must be frozen
  *

packages/marshal/test/test-marshal-stringify.js

@@ -37,9 +37,6 @@ test('marshal stringify errors', t => {
   t.throws(() => stringify(harden(Promise.resolve(8))), {
     message: /Marshal's stringify rejects presences and promises .*/,
   });
-  t.throws(() => stringify(harden({ foo: () => {} })), {
-    message: /Marshal's stringify rejects presences and promises .*/,
-  });
   t.throws(() => stringify(Far('x', { foo: () => {} })), {
     message: /Marshal's stringify rejects presences and promises .*/,
   });