-
Notifications
You must be signed in to change notification settings - Fork 103
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
Implement request/response protocol #2688
base: main
Are you sure you want to change the base?
Conversation
Unable to build without Cargo.lock file. This means that after this change 3rd party projects may have For the first failing build see: https://github.com/EspressoSystems/espresso-sequencer/actions/runs/13634643539 To reproduce locally run
This PR can still be merged. |
Co-authored-by: Mathis <sveitser@gmail.com>
LGTM. Just some questions. |
Closes EspressoSystems/HotShot#3871
This PR:
Finishes the plumbing for the request/response protocol for use in the confirmation layer. It does this by making use of the preexisting
external
messaging functionality so that the application layer can both send and receive messages.As a reminder, this protocol was introduced in EspressoSystems/HotShot#4071
This PR does not:
Make use of the protocol. These will be introduced in subsequent PRs. I decided not to do a
NetworkConfig
example, since the protocol has a dependency on HotShot (HotShot is not initialized when we need the config)Key places to review:
sequencer/src/context.rs
for protocol defaults (I think these are sane, but others would know more)RollCall
removal - cc @Ayiga you previously said we didn't need this, and it doesn't look like it breaks anythingI tested that this works and each node is able to get a response on startup. I didn't include this since it felt unnecessary in the PR. Perhaps we can add better testing, but unsure if it would be easy to do with the existing integration tests