Merge pull request #7544 from Agoric/swingset-remove-todo-slog-replay

chore(swingset): clean up TODOs, add replay syscall results to slog

packages/SwingSet/src/devices/bridge/bridge.js

@@ -113,12 +113,9 @@ export function buildBridge(outboundCallback) {
   }
 
   function callOutbound(...args) {
-    // TODO: prevent cross-Realm contamination by serializing/deserializing
-    // all the data that crosses this boundary. Inside the device
-    // (device-bridge.js), we know we're immediately serializaing this data, so
-    // we don't need to sanitize it there. But here, we have no idea what
-    // outboundCallback() might do, so we want to avoid confusion. When we
-    // switch to new-SES, we can remove this protection.
+    // TODO: this existed to prevent cross-Realm contamination, but
+    // now that the Start Compartment has tamed globals, we no longer
+    // need it
     return outboundCallback(...sanitize(args));
   }
 

packages/SwingSet/src/devices/command/command.js

@@ -41,8 +41,8 @@ export default function buildCommand(broadcastCallback) {
     const count = Nat(kCount);
     const isReject = Boolean(kIsReject);
     let obj;
-    // TODO: is this safe against kernel-realm trickery? It's awfully handy
-    // to let the kernel-side result be 'undefined'
+    // TODO: Start Compartment globals are tamed, no longer need this
+    // sanitization
     if (kResponseString !== undefined) {
       obj = JSON.parse(`${kResponseString}`);
     }

packages/SwingSet/src/kernel/kernel.js

@@ -1484,11 +1484,11 @@ export default function buildKernel(
 
           // kres is a KernelResult: ['ok', value] or ['error', problem],
           // where 'error' means we want the calling vat's syscall() to
-          // throw. Vats (liveslots) record the response in the transcript
-          // (which is why we use 'null' instead of 'undefined', TODO clean
-          // this up #4390), but otherwise most syscalls ignore it. The one
-          // syscall that pays attention is callNow(), which assumes it's
-          // capdata.
+          // throw. Vats record the response in the transcript (which is why
+          // we use 'null' instead of 'undefined', TODO clean this up
+          // #4390), but otherwise most syscalls ignore it. The two that
+          // pay attention are 'callNow' (which assumes it's capdata), and
+          // 'vatstoreGet' (which assumes string or null).
 
           // this can throw if the translator is buggy
           vres = translators.kernelSyscallResultToVatSyscallResult(

packages/SwingSet/src/kernel/vat-warehouse.js

@@ -10,6 +10,7 @@ import djson from '../lib/djson.js';
  * @typedef {import('@agoric/swingset-liveslots').VatSyscallObject} VatSyscallObject
  * @typedef {import('@agoric/swingset-liveslots').VatSyscallResult} VatSyscallResult
  * @typedef {import('@agoric/swingset-liveslots').VatSyscallHandler} VatSyscallHandler
+ * @typedef {import('../types-external.js').KernelDeliveryObject} KernelDeliveryObject
  * @typedef {import('../types-internal.js').VatManager} VatManager
  * @typedef {import('../types-internal.js').VatID} VatID
  * @typedef {import('../types-internal.js').TranscriptDeliveryInitializeWorkerOptions} TDInitializeWorkerOptions
@@ -43,12 +44,12 @@ function recordSyscalls(origHandler) {
     return vres;
   };
   const getTranscriptEntry = (vd, deliveryResult) => {
-    // TODO add metering computrons to results
     /** @type {TranscriptDeliveryResults} */
     const tdr = { status: deliveryResult[0] };
     if (deliveryResult[0] === 'ok') {
       const usage = deliveryResult[2];
       if (usage) {
+        // add metering computrons to results, if any
         tdr.metering = { computrons: usage.compute };
       }
     }
@@ -136,7 +137,8 @@ export function makeSyscallSimulator(
   };
 
   const syscallHandler = vso => {
-    kernelSlog.syscall(vatID, undefined, vso); // TODO: finish()?
+    // slog entries have no kernel-translated kso/ksr
+    const finish = kernelSlog.syscall(vatID, undefined, vso);
     const expected = syscallsExpected[syscallsMade.length];
     syscallsMade.push(vso);
     if (!expected) {
@@ -152,6 +154,7 @@ export function makeSyscallSimulator(
       throw error;
     }
     syscallStatus.push('ok');
+    finish(undefined, expected.r);
     return expected.r;
   };
 
@@ -590,13 +593,11 @@ export function makeVatWarehouse({
 
   /**
    * @param {string} vatID
-   * @param {unknown[]} kd
+   * @param {KernelDeliveryObject} kd
    * @returns {VatDeliveryObject}
    */
   function kernelDeliveryToVatDelivery(vatID, kd) {
     const translators = provideTranslators(vatID);
-
-    // @ts-expect-error TODO: types for kernelDeliveryToVatDelivery
     return translators.kernelDeliveryToVatDelivery(kd);
   }