Add QuorumCertificate method to Snapshot

[jordanschalm]

This PR adds QuorumCertificate method to protocol.Snapshot. This isn't strictly necessary for serializable snapshots, however we needed at least the signature from the QC in order to satisfy the Seed method, and we wanted to make this change anyway (see https://github.com/dapperlabs/flow-go/issues/4892).

This PR does not handle the case where the quorum certificate is requested for the root snapshot, but should handle all other cases. Storing the root QC will be handled as part of the Bootstrap changes in https://github.com/dapperlabs/flow-go/issues/4890.

[zhangchiqing]

Looks good 👍

[AlexHentschel]

Looks great.

Something not directly related to your PR, which I noticed:

• Snapshot.Seed(...) and Snapshot.QuorumCertificate() directly work with the data base. There is no caching layer in between. This concerns me, as we had previously significant issues with directly accessing the data base:
  • it cost us two days to benchmark until Max Wolter narrowed the problem down to the protocol state taking way too long when it was always directly accessing the DB
  • As a result, Max introduced caching layers for all storage access in the protocol state.
• I feel it would be a relatively moderate time investment now, to also cache a block's valid child (e.g. in the mutator as part of the MakeValid call). In contrast, if we don't do it now and it becomes a performance problem, it will likely cost us significantly more time to only identify that finding a valid child is taking too long through benchmarking.

Totally happy, if we do this in a separate PR. Just wanted to flag it.

[jordanschalm]

Snapshot.Seed(...) and Snapshot.QuorumCertificate() directly work with the data base. There is no caching layer in between.

It's true. On the other hand, retrieving the latest finalized or sealed block, an operation we do very often currently, also goes directly to Badger (twice actually!):

flow-go/state/protocol/badger/state.go

Lines 91 to 99 in 9d69ff9

	func (s *State) Final() protocol.Snapshot {
	// retrieve the latest finalized height
	var finalized uint64
	err := s.db.View(operation.RetrieveFinalizedHeight(&finalized))
	if err != nil {
	return NewInvalidSnapshot(fmt.Errorf("could not retrieve finalized height: %w", err))
	}
	return s.AtHeight(finalized)
	}

Badger does have a built-in cache, but the cache contains the serialized data. My understanding is that for some larger models (and for the special case of BLS keys, where deserialization involved expensive cryptographic checks), deserialization was expensive enough to warrant another caching layer. I assumed for small DB values like heights and indexes the deserialization cost was small enough we got most of the benefits from Badger's built-in cache.

However, it turns out that Badger disabled caching by default in v2.0.3, which we switched to back in June (here as far as I can tell). Apparently having the cache turned on is slower than having it turned it off for the default configuration 🤔.

For now I made an issue for adding the cache as you suggested. I also think we should add tracing to a few of these paths that hit the database to get a sense for the current cost. https://github.com/dapperlabs/flow-go/issues/5115