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

Implement request/response protocol #2688

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

rob-maron
Copy link
Contributor

@rob-maron rob-maron commented Feb 26, 2025

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)
  • All the RollCall removal - cc @Ayiga you previously said we didn't need this, and it doesn't look like it breaks anything

I 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

Copy link
Contributor

github-actions bot commented Mar 3, 2025

Unable to build without Cargo.lock file.

This means that after this change 3rd party projects may have
difficulties using crates in this repo as a dependency. If this
isn't easy to fix please open an issue so we can fix it later.

For the first failing build see: https://github.com/EspressoSystems/espresso-sequencer/actions/runs/13634643539

To reproduce locally run

cargo generate-lockfile
cargo check --all-targets

This PR can still be merged.

@sveitser
Copy link
Collaborator

sveitser commented Mar 4, 2025

LGTM. Just some questions.

@rob-maron rob-maron enabled auto-merge (squash) March 5, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Networking] - Expose request-response protocol to application layer
2 participants