comms: provide sendExplicitSeqNums in vatParameters

[warner]

What is the Problem Being Solved?

The comms vat has two initialization parameters:

• identifierBase: this initializes the counter used to allocate object/promise identifiers like roNN and lpNN. We use it during a few unit tests that involve multiple comms vats, so we can tell their messages apart in the debug logs (chore(swingset): localVatManager: give vatParameters to setup() #1432 , add vatParameters to setup() arguments, make comms starting ID configurable #2529).
• sendExplicitSeqNums: If true, transmitted messages include an explicit sequence number. If false, the sequence number is implicit, and messages are slightly smaller (maybe 10 bytes). The receiver can handle either, and it will validate any explicit seqnums it gets. The idea was that we leave it turned on until we're confident in the implicit tracking and/or we stop wanting to debug or interpret comms messages, then turn it off. It currently defaults to true. When true, we do one additional DB read per transmit() (to fetch the current seqnum). We could combine remote.nextSendSeqNum() and remote.advanceSendSeqNum() to avoid the extra read.

identifierBase is only used during initialization of the persistent state, and obviously must be the same for all members of a consensus machine.

sendExplicitSeqNums would tolerate being changed over time, but for any given call, it must be the same for all members of a consensus machine.

To support #4381, I've been moving vatParameters out of setBundle (and therefore setup()) and into the dispatch.startVat message. This delays the availability of both parameters until the startVat message is delivered. identifierBase was straightforward (we don't need to do state.initialize() until startVat), but I ran into a problem with sendExplicitSeqNums. When vatParameters arrived as an argument to setup(), we could hold sendExplicitSeqNums in RAM for the lifetime of the (local) worker, knowing that we'd get the same value after a reboot.

But when they only arrive in startVat, we don't get a copy during the second and subsequent incarnations of the worker. To maintain consistent behavior across reboots, we must either move the parameter back into the setup() args, or store it into the DB during startVat (and read it out of the DB later).

I see the following options:

• 1: create setupParameters (basically a renaming of the old setup()-delivered vatParameters), only it for setup vats, and only for manager-local. Remove vat-selected option for exporting default setup or named buildRootObject, change managerOptions.enableSetup into .useSetup, pass setupParameters.sendExplicitSeqNums. All incarnations get the same value in setup()
• 2: pass it through vatParameters, write into DB, read from DB on every transmit
• 3: pass through vatParameters, write into DB, only read from DB during the first transmit of a new incarnation
• 4: remove the parameter entirely, always send explicit seqnums
• 5: remove the parameter entirely, never send explicit seqnums

I've been looking forward to deleting a lot of vatParameter -handline code, by removing it from managerOptions. Option 1 would only let me do half of that cleanup, and would add a new argument to DynamicVatOptions (we'd have both setupParameters and vatParameters). There would additional code to restrict use of setupParameters to local workers (because we don't currently support setup() anywhere else, and don't see much of a need for it, since the only setup() vat is comms, and maybe vattp some day.

Option 2 would add an extra DB read to every transmit. We could compensate for this by consolidating:

    const seqNum = db.getSendExplicitSeqNums() ? remote.nextSendSeqNum() : ''; // one DB read, maybe one DB read
    remote.advanceSendSeqNum(); // one DB read, one DB write

into something like:

    const sn = remote.advanceSendSeqNum(); // returns old seqnum, one DB read, one DB write
    const seqNum = db.getSendExplicitSeqNums() ? sn : ''; // one DB read

but of course if we stopped making it configurable (options 4 or 5) we could remove both DB reads.

It also causes this really-for-debugging value to show up in the DB, which feels wrong. But, the behavior of the comms vat is changed by this value (the syscall.sends it makes to vattp will contain different strings), so we should accept that this value is a legitimate part of the configuration, and treat it as such.

Option 3 would cause variable read syscalls depending upon how recently the worker had been restarted. @FUDCo points out that this isn't technically a problem: the comms vat doesn't use a transcript, so nobody is tracking the syscalls that it makes, and a DB read won't modify the crank-hash, and the comms vat isn't metered. But I'm still really reluctant to introduce this sort of variation, since we have an incentivized testnet killed by just this sort of thing (#3726, also maybe #3021, #3471 in the mailbox device).

I'm also somewhat reluctant to remove the explicit sequence numbers. I'm no longer worried that the code might misbehave, but It makes studying the comms protocol messages a lot easier, and the space savings doesn't seem to be significant (although the cost does grow, logarithmically, over time, just like all our identifiers get longer over time).

So I'm going to go with option 2 for now: store it in the DB during startVat(), read from the DB during every transmit(). The comms vat runs co-resident with the kernel, so DB reads are much cheaper than they would be in an xsnap process, but if we find we want to squeeze out those reads for performance in the future, I'll lean towards option 4 and always send the seqnum.

Description of the Design

function doStartVat(vatParameters={}) {
  const { sendExplicitSeqNums = true } = vatParameters;
  state.setSendExplicitSeqNums(sendExplicitSeqNums);
}

function transmit() {
  const sendExplicitSeqNums = state.getExplicitSeqNums();
  ..
}

(this also opens up the possibility of adding a message that turns it on or off later)

Security Considerations

consensus considerations, but no security differences

Test Plan

Maybe unit tests, this is used so infrequently that I may be satisfied with coverage provided by the existing tests.

[FUDCo]

I'd note that options 4 and 5 are possibly just kicking the can down the road, as this flavor of issue could very well manifest for other vats that don't have the option to do without some parameter.