Update lagging validators on blob reads

[andresilva91]

Motivation

We need to update lagging validators if they're not aware of blobs that have already been published.

Proposal

A few things had to be done here:

• Write the actual code to update the lagging validators. This is initially used in two places: when we're first staging block execution, and when we're synchronizing the chain state from the validators.
• Created test only ReadBlob SystemOperation to be able to test this without creating or modifying existing applications.
• Added hashed_certificate_values and hashed_blobs to process_validated_block. This was an existing bug because we check for missing blobs there, but didn't pass the information along on a retry.
• We had to move synchronize_chain_state and try_synchronize_chain_state_from to the client, so that we could properly call the lagging validator code on chain synchronization as well.
• Fixed a bug where our test LocalValidatorClient wasn't properly handling certificates: if NoConfirm and we're trying to handle a validated certificate, it wouldn't actually call the handle code like it should.

Test Plan

3 tests were written (more to come in following PRs)

[andresilva91]

• Make BlobId into enum #2231
• Update lagging validators on blob reads #2220 👈
• read_blob system API #2193
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @andresilva91 and the rest of your teammates on Graphite

[afck]

Nice! But so far this addresses only a part of the lagging validator situation: We also need to be able to re-propose another owner's ValidatedBlock proposal in a later round, even if neither we nor a quorum of validators have the blob. (Maybe that's a separate issue, for a different PR.)

[afck]

We should probably also try to fetch the missing blobs in the ChainExecutionContext::IncomingMessage(index) case, and reject that message only after this has failed. (Doesn't apply in this PR yet, because messages can't read blobs yet.)

So we could leave stage_block_execution_and_discard_failing_messages as it is, and move this logic into an inner self.stage_block_execution(block) call (which in turn calls self.client.local_node.stage_block_execution(block.clone())). But as discussed, we will ultimately want to fetch the blobs without restarting execution from the beginning.

[andresilva91]

Is this all then for a separate PR after messages are able to read blobs then? 🤔

[afck]

If you prefer, but we have to make sure we remember it. The new system operation is meant to simulate what in production will actually be user operations and incoming messages. So I'm a bit worried about any code that only makes the operation work, but not messages, because then our tests give us a false sense of security.

[graphite-app]

Graphite Automations

"Assign reviewers" took an action on this PR • (07/10/24)

6 reviewers were added to this PR based on Andre da Silva's automation.

[afck]

Why do we need to update the lagging validators if our own local node is missing the blob? It would be clearer if synchronize_from_validators really only synchronized from, not to them.

[andresilva91]

I see your point, but AFAIU in our test we try to confirm a block that was validated by a different client, from a client without the blob, and also with lagging validators. How should we deal with this case if we're not updating the lagging validators and getting the blob while processing the certificate? I just couldn't find an alternative, but I'm probably missing something 🤔

[afck]

I'd expect the second client to only update the lagging validators when they send back an error about the missing blob.

[andresilva91]

I think this test_re_propose_validated test has only been passing because of the NoConfirm bug 🤔

[afck]

You mean the existing one on main? If I only apply your test_utils changes, test_re_propose_validated still passes for me on main.