Test framework for restartability

[jbearer]

Test framework

Set up some Rust automation for tests that spin up a sequencer network and restart various combinations of nodes, checking that we recover liveness. Instantiate the framework with several combinations of nodes as outlined in https://www.notion.so/espressosys/Persistence-catchup-and-restartability-cf4ddb79df2e41a993e60e3beaa28992.

There are many things left to test here, including:

• Test all nodes restarting at the same time (this is the one case that doesn't work yet, requires all nodes to store state which depends on [MACRO] non-DA state storage hotshot-query-service#664)
• Checking integrity of the DA/query service during and after restart

These can be done in follow-up work

I considered doing this with something more dynamic like Bash or Python scripting, leaning on our existing docker-compose or process-compose infrastructure to spin up a network. I avoided this for a few reasons:

• process-compose is annoying to script and in particular has limited capabilities for shutting down and starting up processes
• both docker-compose and process-compose make it hard to dynamically choose the network topology
• once the basic test infrastructure is out of the way, Rust is far easier to work with for writing new checks and assertions. For example, checking for progress is way easier when we can plug directly into the HotShot event stream, vs subscribing to some stream via HTTP and parsing responses with jq

Storage refactor

The tests where all nodes restart together exposed an issue. There was a race condition where DA nodes may decide on a block, but shut down before they have updated their query storage. After the restart, they will have lost the corresponding block payload and VID info from the in-memory HotShot data structures, and they will never get it back.

This PR refactors the way query storage is updated: instead of having separate, unsynchronized event handling tasks for updating consensus storage (e.g. storing DA proposals) and updating query storage, we now have just a single task for populating consensus storage, and query storage is populated from consensus storage. This means we no longer rely at all on in-memory payload storage in order to populate our query service. This makes DA certs much more meaningful, since DA votes are conditioned on successfully storing the DA proposal in consensus storage. We can also now remove the PayloadStore from HotShot.

For the SQL backend, query service population is coupled with consensus storage garbage collection, so that we can delete old data and collect it/move it to archival storage in an atomic transaction.

Key places to review

• New test suite: sequencer/src/restart_tests.rs
• Storage refactor: primarily in types/src/v0/traits.rs, sequencer/src/persistence.rs, sequencer/src/persistence/sql.rs

[jbearer]

Alright, I've found a potential deadlock that might explain the CI timeouts.

During the process of initializing a node, we (sometimes) call out to the API data source. Basically, this happens when there were unprocessed leaves from before the shutdown, that we now try to process. The event consumer then tries to get an exclusive lock to update the data source.

Unfortunately, there are certain API endpoints that also lock the data source (for reading) which then block on initialization completing so they can read from the sequencer context.

Thus, the potential deadlock is:

• Node starts API server
• Another node sends a catchup request (for chain config, frontier, or fee account)
• Request handler locks API state and then awaits future for context initialization
• Context initialization gets to the point where it has to process old leaves and blocks on a write lock on API state

I believe a clean solution to this is to separate the back-processing of old persisted leaves from the loading of consensus state, and move it instead to the beginning of the asynchronous task which processes future leaves. This means that node initialization will no longer block on the data source lock. This will also make consensus state loading read-only, which the fact that it wasn't was a code smell anyways

[jbearer]

Fix for the above deadlock in 7bda75e, and tests are now passing!

[tbro]

There is a lot here, but I've done as thorough of a review as I am able. I think the extensive test coverage should make up the difference. I'm approving. I've made a couple of minor comments, but I leave it up to you if should continue discussing them in another venue.

I considered doing this with something more dynamic like Bash or Python scripting

I'm glad you didn't do that.

[tbro]

Do we absolutely need to run restart-ability on every PR? It adds significant overhead and it feels like something ppl making minor changes shouldn't have to worry about. Can we filter out restart-ability tests here, and add run them in a separate workflow that runs on merge to main?

[tbro]

I think doc comments in selective places can help us understand what some of the less straightforward commands are doing. Then we don't have to think some much when we get to explanations of how it works in the inline comments.

Suggested change

	/// Restart indicated number of nodes and check that remaining
	/// nodes can still make progress.

[jbearer]

Superseded by #1985