Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(swingset): commit crank-buffer after every c.step/c.run
We observed a consensus failure on the chain that was triggered by a validator getting restarted. The root cause seems to be that device calls (specifically `poll()` on the timer device) cause state changes, which are accumulated in the crankhasher and linger in the crank buffer until something causes them to be committed. This commit doesn't happen until processQueueMessage() finishes, which only happens if the run-queue had some work to do. This occurs in a subset of the times that `kernel.run()` is invoked. On the chain, each block causes a `poll()` followed by a `kernel.run()`. If the `poll()` did not cause any timers to fire, the timer device will not enqueue any messages to the timer vat, the run-queue remains empty, and `run()` has no work to do. Therefore the crank buffer is not committed. Imagine a chain in which some delivery causes everything to be committed in block 1, then blocks 2-9 have a `poll()` but no actual deliveries, then something happens to make block 10 have a delivery (causing a commit). Now follow validator A (which is not restarted): when the block 10 delivery happens, the crank buffer (and crankhasher) has accumulated all the state changes from blocks 2-10, including all the timer device state udpates from the `poll()` calls. Suppose validator B is restarted after block 5. When it resumes, the crank buffer is empty, and the crankhasher is at the starting state. Blocks 6-10 add changes to both, and at block 10 the `commit()` causes the crankhasher to be finalized. The total data in the crankhasher is different (the input is smaller) on B than on A, and the crankhash thus diverges. This causes the activityhash to diverge, which causes a consensus failure. The invariant we must maintain is that the crank buffer and crankhasher must be empty at any time the host application might commit the block buffer. There must be no lingering changes in the crank buffer, because that buffer is ephemeral (it lives only in RAM). The simplest way to achieve this is to add a `commitCrank()` to the exit paths of `kernel.step()` and `kernel.run()`, so that the invariant is established before the host application regains control. We must also rearrange the way that `incrementCrankNumber` is called. Previously, it was called at the end of `start()`, leaving the "current" crank number in the crank buffer (but not committed to the block buffer yet). Then, at end-of-crank, the sequence was `commitCrank()` followed by `incrementCrankNumber()`. In this approach, the crank buffer was never empty: except for the brief moment between these two calls, it always had a pending update. In the new approach, we remove the call from `start()`, and we increment the crank number just *before* the end-of-crank commit. This way, the crank buffer remains empty between cranks. One additional piece remains: we should really commit the crank buffer after every device call. The concern is that an e.g. timer device `poll()` causes some state changes but no delivery, and then the next delivery fails (i.e. it causes the target vat to terminate). We don't want the unrelated device changes to be discarded along with the vat's half-completed activity. This would be a good job for the #720 kernel input queue. refs #3720
- Loading branch information
