fix(SwingSet): remove consensusMode flip-flop

[michaelfig]

refs: #1852
closes: #4510 closes: #4517

Description

A do-nothing console.log gets in the way of legitimate on-chain debugging (especially when the "chain" is just a local node). This PR implements consistent do-something console.log, even when running in consensus.

Security Considerations

The consensus guarantee is that the same code runs on all the chain followers, except for SwingSet host configuration, which is permitted to differ as long as it doesn't affect the behaviour committed in the IAVL tree or kernel DB. Running in $DEBUG or out of $DEBUG mode as in this PR causes no observable consensus difference, but does affect what is displayed on the console.

That change of display should not affect chain security. It was a fault of the earlier consensusMode rollout that caused a divergence of behaviour when running within or without consensus mode.

Documentation Considerations

Testing Considerations

Should help Cosmos chain integration testing.

Being able to debug code running on chain is useful, especially since our sim-chain doesn't have many of the affordances a real Cosmos chain has. Without the debug output, we lacked insight into what would happen when actually running with Cosmos.

[mhofman]

My understanding of this change is that the worker's view of consensusMode is really a "disable console logging".

Since we seem to keep this parameter present, but always set it to false, would it make sense to rename it to something like _disableLogging instead of _consensusMode.

Also curious whether it's worth removing the check itself. And if we are removing the check, why can't we remove the parameter altogether?

[warner]

I'm generally in favor of removing this consensusMode switch. If we do, I'd second Mathieu's recommendation to remove the machinery entirely, instead of leaving a parameter whose fixed value is false.

But I hesitate because I don't know what other changes have landed to support this. The original concern was that we wanted the flexibility to change the behavior of logging (possibly between validators, and/or we didn't trust that the platform's console would behave in a consistent way across tolerable platform upgrades), but userspace could use getters or Proxy to sense that behavior. consensusMode was meant to abandon debugging pleasantries exchange for closing off console-based sources of nondeterminism.

What is the current state of console behavior? Can we commit to its getter/Proxy-visible behavior across xsnap upgrades and changes to debugging modes?

I imagine this might be more possible now than it was back then, because:

• 1: we're committed to using xsnap for all user-written code in vats (for metering)
• 2: we now realize that xsnap does not provide a console object (only print), so there's no platform behavior to depend upon
• 3: we're providing our own console, so we have more control/confidence of its behavior

@michaelfig is that last one accurate? I get lost in the layers of console-like things that xs-worker gets, but I'm willing to believe that it might be deterministic at this point. What's doing the object rendering? What happens if you feed it a cyclic object graph? Is it a library that we're confident we'll never need to upgrade?

[michaelfig]

• 3: we're providing our own console, so we have more control/confidence of its behavior

@michaelfig is that last one accurate?

Yes. The rendering is done in a vetted JS shim. I can vouch for the fact that it doesn't access any sources of nondeterminism.

What happens if you feed it a cyclic object graph?

It reports [Circular].

Is it a library that we're confident we'll never need to upgrade?

It's much less complex than liveslots or SES. For all of these, we currently cannot upgrade them easily. Auditing them gives more confidence, but we probably need plans for what to do if we found an upgrade-necessary problem in any of them.

[dckc]

Is now a good time to document clearly the path from a console.log(...) call in a contract through the various layers, over the pipe and through the woods, to... where? The stdout of agoric start or agd?

Maybe I'm thinking more about stack traces, which is somewhat separate.

Meanwhile, our Hardened JS docs for contract writers say:

... compartments used to load Agoric smart contracts ... have console and assert globals from the ses package. Don't rely on console.log for printing, though; it is for debugging only, and in a blockchain consensus context, it may do nothing at all.

Is that still correct?

[warner]

I skimmed through #4364 (which introduced the vendored copy of object-inspect.js which provides the "stable+deterministic" renderer which backs console.log), as well as our internal #eng_ses_internal discussion on 08-feb-2022. The conversation got cut off for meetings, but I think @erights and @dckc were against using object-inspect.js without a line-by-line review, and there was some proposal to write our own version, possibly use it to replace the import { q } from '@agoric/assert' renderer. I've filed #4814 to discuss this.

Any form of console.log that used a renderer with the safe/deterministic properties we would put into q, or believe/hope we have from object-inspect, would make it safe to remove consensusMode and have console.log unconditionally render its arguments into a string. To send that rendered string to the kernel process over the pipe and not worry about nondeterminism, we'd need to either send it unconditionally, or hand it to a (non-metered) C function which could write(1) it or not as it saw fit. (It might be sufficient to disable metering for the sending decision and JS write calls, that would still have a variable effect on organic GC, but we're supposedly not allowing that to affect syscalls).

I think the decision to lean in to this renderer (and make logging unconditional) is independent of our decision to use (and review) object-inspect.js vs write our own vs go back to using marshal's existing q() renderer.

[warner]

With #4814 to track the review/rewrite, I'm ok with removing consensusMode. Please make some tickets (if you haven't already) for building the alternate controls we talked about today ("start/stop sending console messages to the kernel process", and maybe "start/stop sending console messages to your own local stdout/stderr"). I think those are lower priority but let's not forget what our plan was.