[consensus/application/marshaled] Migrate to Asynchronous Verification by Default

[patrick-ogrady]

Related: #2565

Instead of waiting for verification of the block to finish during consensus::verify(), we should immediately return verified and only use the verification result in consensus::certify().

This better streamlines block verification with no real downside (faster views in the happy path with no slower views if timeout). The only "net negative" is that we won't immediately vote nullify if verification fails (would have to wait for timeout or for certify to be called).

[patrick-ogrady]

This shouldn't require modifying the block format (and I think we can just cache the verify context in metadata)?

[clabby]

This shouldn't require modifying the block format (and I think we can just cache the verify context in metadata)?

I'm not sure if there's a clean way around altering the payload format to enable async verification without some deeper modifications. It's possible for a node to receive notarization certs from the network without ever having verified the payload themselves, and in that case, they won't have the consensus context from Automaton::verify cached to complete verification within Automaton::certify.

monorepo/consensus/src/simplex/actors/voter/actor.rs

Lines 1019 to 1025 in 85d7d8f

	Certificate::Notarization(notarization) => {
	trace!(%view, from_resolver, "received notarization");
	self.handle_notarization(notarization).await;
	if from_resolver {
	resolved = Resolved::Notarization;
	}
	}

monorepo/consensus/src/simplex/actors/voter/actor.rs

Lines 425 to 428 in 85d7d8f

	if added {
	self.certification_candidates.insert(view);
	self.append_journal(view, artifact).await;
	}

[andresilva]

How about changing CertifiableAutomaton to pass the context directly:

  fn certify(
      &mut self,
      context: Self::Context,
      payload: Self::Digest,
  ) -> impl Future<Output = oneshot::Receiver<bool>> + Send

The voter already has access to the proposal (which contains round and parent), and can derive leader from the elector. It should be able to construct and pass the context when calling certify()?

[clabby]

How about changing CertifiableAutomaton to pass the context directly

If this works, would solve all of our problems (and make the integration for coding in marshal much easier.) IIRC, this was avoided intentionally, though. cc @patrick-ogrady @BrendanChou

[andresilva]

What I suggested above doesn't actually work. The issue is that certify() can be called for any view that has a notarization, not just the current view (unlike verify()).

[andresilva]

Can't we just abstain from certifying in this case? If a notarization exists, at least 2f+1 nodes voted to notarize, which means at least 2f+1 nodes called verify() and cached the context. So normally at most f nodes could be in the "no context" state, these would be nodes catching up, observers, or nodes that received the notarization before the proposal. The 2f+1 nodes with context will eventually certify and vote to finalize. If for some reason >f eventually get in this situation we will timeout and nullify the round?

[patrick-ogrady]

Can't we just abstain from certifying in this case? If a notarization exists, at least 2f+1 nodes voted to notarize, which means at least 2f+1 nodes called verify() and cached the context. So normally at most f nodes could be in the "no context" state, these would be nodes catching up, observers, or nodes that received the notarization before the proposal.

Interesting claim. I think you're right (and like always the nullification is our saving grace). On the happy path, almost anyone presented with certify will have seen verify.

The 2f+1 nodes with context will eventually certify and vote to finalize. If for some reason >f eventually get in this situation we will timeout and nullify the round?

This isn't exactly true as that 2f+1 quorum is only guaranteed to have f+1 honest (not enough to see the finalization through), however, I don't think it affects the validity of the claim.