conductor: replace gossipnet by a subscription to cometbft/sequencer #383
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
conductor
SuperFluffy
changed the base branch from
main
to
superfluffy/expand-sequencer-client
github-actions
bot
removed
sequencer
SuperFluffy
force-pushed
the
superfluffy/conductor-remove-gossipnet
branch
from
5b7a78c
to
6bbdeb2
Compare
SuperFluffy
commented
Sep 18, 2023
sambukowski
approved these changes
Sep 18, 2023
SuperFluffy
force-pushed
the
superfluffy/expand-sequencer-client
branch
from
a3ff06e
to
016f21d
Compare
SuperFluffy
force-pushed
the
superfluffy/conductor-remove-gossipnet
branch
from
9399c90
to
7158db2
Compare
noot
reviewed
Sep 19, 2023
noot
approved these changes
Sep 19, 2023
Base automatically changed from
superfluffy/expand-sequencer-client
to
main
September 20, 2023 11:44
This patch removes gossipnet from conductor, relying on the new extension trait introduced in recent patches of sequencer-client.
SuperFluffy
force-pushed
the
superfluffy/conductor-remove-gossipnet
branch
from
98de494
to
a2e0805
Compare
github-actions
bot
removed
the
sequencer
Closed
I'm still finding the word 'gossip' in docs and comments when I search |
This was referenced Sep 20, 2023
Merged
steezeburger
added a commit
that referenced
this pull request
Sep 20, 2023
…egration * main: conductor: infallibly convert verification key to tendermint public key (#371) Updating sequencer config implementation (#330) conductor: refactor with a single `Conductor` type entrypoint. (#392) conductor: replace gossipnet by a subscription to cometbft/sequencer (#383) sequencer-client: add extension trait to subscriber to new sequencer blocks (#382) update conductor docs after validation changes (#322)
joroshiba
reviewed
Sep 20, 2023
|
|
||
| is_shutdown: Mutex<bool>, | ||
| } | ||
|
|
||
| struct SequencerClient { |
There was a problem hiding this comment.
Again on descriptive nature, should this be SequencerWsClient?
There was a problem hiding this comment.
IMO that's too descriptive, especially when adding abbreviations. I can see the need if we were to use both http and ws clients, but not if it's only one in this context.
The underlying transport doesn't matter at this level of abstraction
SuperFluffy
added a commit
that referenced
this pull request
Sep 20, 2023
## Summary Remove the gossipnet crate and mentions of it. ## Background Patches #383, #384 removed gossipnet from sequencer-relayer and conductor. This patch removes the `astria-gossipnet` crate altogether because it is no longer used. This PR does not update the conductor spec but leaves it as a follow-up. ## Changes - Remove `astria-gossipnet`, update README, github workflow, workspace Cargo.toml to reflect that - Remove `gossipnet` mentions from astria-conductor ## Testing not applicable ## Related Issues closes #334
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This patch replaces gossipnet by a websocket stream of new blocks directly from sequencer.
Background
Adding a p2p layer adds a lot of complexity to astria services. With this patch conductor reads new blocks directly off of cometbft/sequencer, making it much simpler. For background reading, see this issue: #325
Changes
Testing
We are relying on the rust tendermint-rpc crate functioning properly. As a follow-up it makes sense to create blackbox tests for conductor, which are currently absent.
Breaking Changelist
Related Issues
Precursor PRs unblocking this work:
#375
#376
#382
closes #332