IBC puts Set in Store, Store rejects, chain cannot launch

[warner]

Describe the bug

While trying to run a loadgen test against current trunk (198e5c3), I found the chain process could not start. The root cause turned out to be code in the IBC vat that was trying to put a Set into a Store:

agoric-sdk/packages/vats/src/ibc.js

Lines 240 to 243 in 198e5c3

	/**
	* @type {Store<Port, Set<PromiseRecord<ConnectionHandler>>>}
	*/
	const portToPendingConns = makeStore('Port');

and

agoric-sdk/packages/vats/src/ibc.js

Line 263 in 198e5c3

portToPendingConns.init(port, new Set());

That fails, now that #3570 has landed, because Stores only accept Passables, and Set is not Passable.

This wasn't caught by CI because the IBC vat has no unit tests.

It was probably caught by the dailyperf test (which does the same steps as the loadgen test I was attempting manually), but we don't have any UI surface on which the dailyperf results are displayed. I'm hoping @mhofman and @michaelfig can think of a place for those to be surfaced.

The fix is to replace this Store with a LegacyMap.

The other thing I wanted to highlight for @erights was what the problem looked like. We had another bug getting in the way (the XS console.log fails when it tries to print a Symbol), but once that's fixed, when the chain startup (agoric start local-chain --verbose) fails with the following:

Logging sent error stack (Error#1)
Error#1: Symbol(passStyle) property expected: [object Set]
Error: "[Symbol(passStyle)]" property expected: (an object)
 at makeError ()
 at fail ()
 at baseAssert ()
 at assertChecker ()
 at checkTagRecord ()
 at checkRemotableProtoOf ()
 at assertValid ()
 at passStyleOfInternal ()
 at passStyleOfRecur ()
 at assertValue ()
 at init ()
 at onBind ()
 at onBind ()
 at ()
 at win ()
 at ()

followed by another 100 lines which are remote echoes of the same error, followed by the kernel halting because the bootstrap() message's Promise was rejected:

2021-08-07T00:11:59.779Z SwingSet: kernel: ##### KERNEL PANIC: kp40.policy panic: rejected {"body":"{\"@qclass\":\"error\",\"errorId\":\"error:liveSlots:v11#70001\",\"message\":\"\\\"[Symbol(passStyle)]\\\" property expected: (an object)\",\"name\":\"Error\"}","slots":[]} #####
portHandler threw (Error#1)
Error#1: kernel panic kp40.policy panic: rejected {"body":"{\"@qclass\":\"error\",\"errorId\":\"error:liveSlots:v11#70001\",\"message\":\"\\\"[Symbol(passStyle)]\\\" property expected: (an object)\",\"name\":\"Error\"}","slots":[]}

  at panic (packages/SwingSet/src/kernel/kernel.js:281:26)
  at doResolve (packages/SwingSet/src/kernel/kernel.js:331:9)
  at resolve (packages/SwingSet/src/kernel/kernelSyscall.js:114:5)
  at Object.doKernelSyscall (packages/SwingSet/src/kernel/kernelSyscall.js:155:16)
  at vatSyscallHandler (packages/SwingSet/src/kernel/kernel.js:880:43)
  at Object.syscallFromWorker (packages/SwingSet/src/kernel/vatManager/manager-helper.js:231:18)
  at handleUpstream (packages/SwingSet/src/kernel/vatManager/manager-subprocess-xsnap.js:88:23)
  at handleCommand (packages/SwingSet/src/kernel/vatManager/manager-subprocess-xsnap.js:110:22)
  at runToIdle (packages/xsnap/src/xsnap.js:195:42)
  at runMicrotasks (<anonymous>)

panic: Error: kernel panic kp40.policy panic: rejected {"body":"{\"@qclass\":\"error\",\"errorId\":\"error:liveSlots:v11#70001\",\"message\":\"\\\"[Symbol(passStyle)]\\\" property expected: (an object)\",\"name\":\"Error\"}","slots":[]}

followed by 43 lines of a Go stack trace.

The specific assertion that failed was in passStyleHelpers.js checkNormalProperty(), which has been called with the prototype of a Set, and asked about a propertyName of Symbol(passStyle). This took us two hours to discover, partially because of the XS bug that thwarted assert's attempt to print the symbol. The error message made it look like checkNormalProperty was being handed a Set, but that's only because the prototype of a Set is stringified very much like a Set itself. It was not obvious to us that this function was being given the prototype of something.

The request I have for @agoric/marshal is some better error logging to help track down this sort of thing. Eventually @michaelfig spotted the onBind on the track trace, and recognized it as part of the IBC code (which he wrote). The assertion that fired was called on such a narrow portion of the data that it didn't have a lot of context to display: once we grepped for property expected to find where the check was being made, we added a console.log(candidate) to see what was being tested, it looked like it was a Set, we added a log of .keys().join(',') to see the contents, and we got a this is no Set instance error, which turned out to be in the XS C code (in retrospect, it was because we were invoking Set.prototype.keys() directly, so this was indeed not a Set). It took a while to figure out that checkNormalProperty was being called on something's prototype.

It'd be nice if, somehow, the Passable assertions could describe the original object in their error messages, rather than the tiny fragment of that object which they find offensive.