fix(swingset): rewrite comms, probably add third-party forwarding

[warner]

This overhauls the comms vat to use a new reference-tracking model. It should
fix a number of gaps where the previous code would throw an error:

• pass an already-resolved Promise through any message, this would make
  the comms vat re-use a retired VPID, which is a vat-fatal error
• resolve a remotely-sourced promise to a local object (rather than a remote
  one), then pipeline a message to it (so the message should be reflected back
  into the kernel)
• resolve a local promise to a remote object, then the remote pipelines a
  message to it (so the message should be reflected back out to the remote)
• passing one remote's references to a different remote

The last case is the "three-party handoff". The full solution (with
shortening, grant-matching, and automatic connection establishment) is quite
a ways off, but this change ought to implement "three-party forwarding". In
this form, when A sends B a reference to C, what B gets is effectively an
object on A that automatically forwards all messages off to C. This "object"
is hidden inside the comms vat and is not reified as a real object.

Three-party forwarding is not tested yet.

refs #1535
refs #1404

[warner]

This is a big overhaul of a tricky part of swingset. I don't know how to best approach the review, but I imagine we should get together with a virtual whiteboard and at least spend some time walking through the code together.

I have a diagram to add to the tree (which I'll copy here to get us started) that shows the flow in delivery.js and the different namespaces involved, which might be a useful reference.

@michaelfig I'll work on testing it tomorrow, but this branch should (in theory) enable the third-party forwarding that the IBC demo requires. Try building your branch on top and see how it goes.

[warner]

[erights]

See #1636

[warner]

That test failure looks like AVA was overenthusiastic about parallelizing the message-pattern tests, running 20 at a time. Each test would take maybe 2-3s by itself, but something (I think rollup) is sufficiently intensive that running more than a handful causes everything to slow down by one or two orders of magnitude. I think AVA is confused by the CI's virtual machine and is counting the number of CPU cores incorrectly. I've seen reference to this in some AVA docs (some environment variables that it uses to sense common CI infrastructure), so there may be something we can tweak. I might also mark just the message-pattern tests to be run serially, to inhibit that misbehavior.

[michaelfig]

@michaelfig I'll work on testing it tomorrow, but this branch should (in theory) enable the third-party forwarding that the IBC demo requires. Try building your branch on top and see how it goes.

🎉 It passes the #259 local test just fine! Trying next with actual IBC, and (fingers-crossed) we are done!

[michaelfig]

@michaelfig I'll work on testing it tomorrow, but this branch should (in theory) enable the third-party forwarding that the IBC demo requires. Try building your branch on top and see how it goes.

🎉 It passes the #259 local test just fine! Trying next with actual IBC, and (fingers-crossed) we are done!

🚀

With just a few minor fixes of bitrot in the ag-nchainz script (in #1639), VatTP over IBC demo works!

[michaelfig]

I don't have enough understanding to approve the totality of this change, but I did give it a cursory look.

[michaelfig]

The following is too dramatic a change to make now, but I should put it on your radar for future code you write:

This is @dtribble's pet peeve, and something we have discussed on eng-quality. The preference is defining only closed-over state and helper functions at the top of a function.
When possible, define the returned object with concise methods (rather than indirection via a separately-defined function), like:

  return harden({
    provideLocalForRemote(remoteID, rref) {
      // ...
    },
    getLocalForRemote(remoteID, rref) {
      // ...
     },
     // ...
  });
}

[warner]

I hear you, and I certainly like having a visible split between internal helper functions and exported API functions.

Is there a pattern (maybe some typescript thing) to let someone see the full set of what's being returned? Those function definitions are 4-20 lines each, so the full set doesn't fit on a single page, so that define-as-we-return pattern means I can't see the full list anywhere.

[michaelfig]

You can define the return type of the function. Or you could use a folding editor and collapse the definitions you're not interested in.

[warner]

We had a review session, I think @FUDCo said this was clearly better than before, and it's passed @michaelfig 's manual IBC tests, and it's survived the integration tests. Landing now.

[michaelfig]

LGTM, based on the in-person walkthrough we just had.