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

Replace http client by websocket client #393

Closed
SuperFluffy opened this issue Sep 19, 2023 · 0 comments · Fixed by #455
Closed

Replace http client by websocket client #393

SuperFluffy opened this issue Sep 19, 2023 · 0 comments · Fixed by #455
Assignees
Labels
conductor pertaining to the astria-conductor crate

Comments

@SuperFluffy
Copy link
Member

SuperFluffy commented Sep 19, 2023

PR #332 introduces a websocket client to stream new blocks from cometbft/sequencer.

The stil existing http client used to verify blocks should be replaced by the same websocket client to reuse the open tcp socket.

Doing this will require refactoring block verifier and driver to accept websocket client types instead of opening sockets themselves.

@SuperFluffy SuperFluffy added the conductor pertaining to the astria-conductor crate label Sep 19, 2023
@SuperFluffy SuperFluffy self-assigned this Sep 19, 2023
SuperFluffy added a commit that referenced this issue Oct 5, 2023
## Summary
This patch adds sync functionality to conductor. On every startup
conductor will get all blocks up to the latest height from sequencer and
forward them to the executor before submitting blocks it got from its
subscription.

## Background
Because conductor is thought of as stateless sidecar to a rollup node
there needs to be a mechanism to bring the rollup node to the latest
state. This patch adds that functionality by fetching all sequencer
blocks up to the latest sequencer height for the target chain ID and
replaying ("executing") them against the rollup node.

Because the prior implementation of conductor was strictly sequential
(blocking the async loop) this patch required a refactor of conductor
into fully async components.

## Changes
- Replace all http clients by websocket clients.
- Add an object pool of websocket clients with a managing task that
restarts the websocket connection on failure.
- Remove the `Driver` type.
- Simplify Executor by not being generic over some client
- Introduce the `data_availability::Reader` and `sequencer::Reader`
types that are independently reading off celestia and sequencer,
respectively
- Refactor `data_availability::Reader` from a for-loop blocking the
actor's event loop into a chain of futures that are driven by its event
loop
- Perform sequencer namespace data verification in one step:
- the block verifier need not get a validator set for each rollup blob
retrieved from celestia; it is enough to run its inclusion proof + data
against the root in the sequencer data.
- since the sequencer namespace data is verified and rollup processing
does not continue without it this is enough
- Add sync functionality to `sequencer::Reader`: fetch all sequencer
blocks between a configured initial height and the latest height and
submit them to the rollup node for execution before submitting newer
blocks.
- Refactor the `Conductor` entry point to track its constituent tasks in
a `JoinMap`.

## Testing
These tests have to be tested by running conductor in devnet. Conductor
does not currently have black box tests. These must be added in a follow
PR.

The executor mock tests have been uncommented because executor should
not be generic. These too will be replaced by blackbox tests.

## Breaking Changelist
- the `ASTRIA_CONDUCTOR_TENDERMINT_URL` has been removed. All
communication with cometbft/sequencer happens over the URL configured in
`ASTRIA_CONDUCTOR_SEQUENCER_URL`.
- no other breaking changes should be observable.

## Related Issues
closes #381
closes #393
closes #439
closes #451
closes #452

---------

Co-authored-by: Sam Bukowski <sam.c.bukowski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conductor pertaining to the astria-conductor crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant