Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include vat transcripts in activityhash? #6594

Closed
warner opened this issue Nov 18, 2022 · 3 comments
Closed

include vat transcripts in activityhash? #6594

warner opened this issue Nov 18, 2022 · 3 comments
Labels
enhancement New feature or request needs-design SwingSet package: SwingSet vaults_triage DO NOT USE

Comments

@warner
Copy link
Member

warner commented Nov 18, 2022

What is the Problem Being Solved?

While investigating #6588 , we realized that transcript entries are probably not being included directly in the consensus-verifying activityHash. It's conceivable, but unlikely, that the kernels which suffered consensus problems had done the right thing during replay, and the wrong thing originally (recorded in the transcript), not the other way around.

Once upon a time, we recorded transcript entries in the kvStore. Later, we moved them into a separate DB named the streamStore (backed by SQLite instead of LMDB).

At a different time, we implemented the crankHash, which hashes together all kvStore writes and deletes. At the end of every crank, we fold this hash into the activityHash (e.g. newActivityHash = sha256(oldActivityHash + crankHash)). The activityHash is returned to the host application. In the case of our chain, the activityHash is written into the IAVL tree, which makes it part of consensus: if one validator manages to get a diverging activityHash, they'll fall out of consensus right away, alerting us to some nondeterminism in the kernel or the vats.

If transcripts were still in kvStore, we'd get consensus-enforcing coverage for free. Or when the #3087 work merges kvStore and streamStore into a single SQLite-backed DB, it will be easier to include streamStore changes in the crankHash. But at the moment, I don't think transcript entries are directly included in the hashes.

However, most of their contents probably are covered by that hash.

  • each transcript entry consists of a delivery, and zero or more syscalls
  • the delivery comes from popping something off the run-queue
    • the run-queue is stored in the kvStore as a series of numbered keys and a pair of pointer keys
    • popping the item deletes a numbered key, and rewrites a pointer key, both of which go into the crankHash
      • this doesn't record the contents of the delivery, just the fact of its existence (actually it's sudden loss of existence)
    • however when it was first added to the queue, the full contents when into crankHash
  • each syscall gets executed, and most executions cause specific kvStore changes
    • syscall.send pushes a specific value onto the acceptanceQueue
    • syscall.resolve changes the (kvStore-hosted) kernel promise table
    • syscall.vatstoreSet changes the (kvStore-hosted) per-vat vatStore table

So although we don't hash the syscalls directly, it would be awfully hard to make a divergent syscall while maintaining the same crankHash.

cc @FUDCo , see if you agree with my math

Description of the Design

Security Considerations

Test Plan

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Nov 18, 2022
@mhofman
Copy link
Member

mhofman commented Nov 19, 2022

I have since pretty much confirmed that the transcript had diverged earlier, and had they been included in the activity hash, node would have experienced a consensus failure.

@mhofman
Copy link
Member

mhofman commented Jan 15, 2023

I think we don't need to do this since vat transcripts and snapshots will be hashed and included in consensus state through state-sync. See #6773 and #5542

@mhofman
Copy link
Member

mhofman commented Sep 5, 2023

Per last comment, closing

@mhofman mhofman closed this as not planned Won't fix, can't repro, duplicate, stale Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-design SwingSet package: SwingSet vaults_triage DO NOT USE
Projects
None yet
Development

No branches or pull requests

4 participants