remove gossipnet from conductor #332
Comments
joroshiba
added
gossipnet
joroshiba
added
the
needs-scope
|
before updating events cometbft validates the block. this means blocks received over the web socket always point to the canonical chain, that is their parent block is the soft block. apply block tries to validate block and last publishes events validation checks our equivalent to head block's parent block hash with our equivalent to soft block hash the state where last block hash is taken from is block applied before this one so event publisher publishes validated block this constrains the way to handle incoming blocks. it means, assuming we trust the validator we are connected to via web socket, all blocks that are received at head height point to the soft block. further this implies, if a child to any of the head blocks is received it will certainly finalise the soft block. also by single slot finality it implies that if head block N+1 is lost but we see its child N+2 and grandchild N+3, we can deduce that the parent block hash in N+2 is the final head. also we know that any blocks that will become part of the canonical chain, will pass conversion from tendermint block response to https://github.com/astriaorg/astria/blob/main/specs/sequencer-app.md#processproposal |
|
...trust the validator (or sequencer without validator), and trust the connection! add TLS to web socket connection if conductor and sequencer not intended to run on localhost if? |
|
can we divide this issue into two parts: (i) removing gossipnet and (ii) adding web socket ? I'd start working on adding a web socket to conductor and encrypting the messages with this library https://crates.io/crates/tokio-native-tls or some other library, or at least in a away that makes it possible to implement sessions on the connection in the future. |
## Summary Replace all untyped errors in sequencer-validation by typed errors. ## Background `sequencer-validation` has to be made a dependency of `sequencer-client`, which only uses typed errors. Therefore `sequencer-validation` must have typed errors (because otherwise `sequencer-client` would be "infected" by eyre, which would require changing all of sequencer-client to also use eyre :-/ ). ## Changes - Replace `eyre` reports by typed errors - Fix `MerkleTree::prove_inclusion` to actually return an error: the docs said it would error, but it actually panicked if `index` was out of bounds. - Add eyre error context to sequencer-relayer: because the error in it wasn't properly wrapped, changing the sequencer-validation types caused it to no longer compile ## Testing Added a test to check for out of bounds inclusion proof index. The other error type is just a wrapper. ## Related Issues This is done as part of #332, which now requires extra trait methods in the `SequencerClientExt` and `SequencerSubscriptionClientExt` extension traits.
## Summary Replace all implicit eyre errors by context specific typed errors ## Background Same reasoning as for #375 as part of the work for #332: `sequencer-types` has to be made a dependency of `sequencer-client`, which only uses typed errors. Therefore `sequencer-types` must have typed errors (because otherwise `sequencer-client` would be "infected" by eyre, which would require changing all of sequencer-client to also use eyre. ## Changes - Introduced one error variant per distinct eyre error. ## Testing N/A. Nothing changed on the code level. Testing that the errors are actually triggered could be done in a follow-up PR, but big parts of the code will be replaced by the work in #355 and follow-up PRs to it, moving the types exchanged over RPCs to the astria-proto crate. ## Breaking Changelist - No observable breaking changes in the astria services. Breaking changes in the library crates because their public API changed (errors), but we are not considering them right now ## Related Issues #12 #86 #366 #332
…blocks (#382) ## Summary Add a `SequencerSubscriptionClientExt` trait with a `subscribe_new_block_data` method. ## Background Removing gossipnet from conductor as part of #332 requires it to read new blocks directly from cometbft / sequencer. This patch extends the sequencer-client crate to provide that functionality using a convenience function so that conductor need not know how to convert comertbft new-block events to sequencer block data. ## Changes - Add a `SequencerSubscriptionClientExt` trait to the sequencer-client crate. - Reorganize the sequencer-client crate to export items under the appropriate features. - warn users if they set neither the `http` nor `websocket` features. ## Testing Testing this client is out of scope for this PR. We are relying on the underlying tendermint-rpc WebSocketClient being correctly implemented. It would be great to extend wiremock or a similar crate to provide a quick way to test websocket APIs. An alternative is to derive a jsonrpc server with jsonrpsee, but that's a lot more work. ## Breaking Changelist None, only functionality was added crates depending on sequencer-client were not touched. ## Related Issues Part of #332 #375 and #376 are precursor PRs unblocking this patch. --------- Co-authored-by: noot <36753753+noot@users.noreply.github.com>
…383) ## 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 - Replace gossipnet directly by a stream of new blocks using the cometbft subscrition RPC. - No other changes in regard to how blocks are further processed has been made. ## 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 - Reading over p2p is no longer supported. Deployments must be updated and wired to read off of cometbft/sequenceer. ## Related Issues Precursor PRs unblocking this work: #375 #376 #382 closes #332 --------- Co-authored-by: Emilia Hane <emiliaha95@gmail.com>
Based on discussion in #325, where we have opted for option 1.
We want to strip out the gossipnet, and instead rely on a full node of the sequencer to read blocks. This is similar to what we are doing in
sequencer-relayerto read new blocks off of proposing chains. We should do this using a cometbft websocket subscription.Note: we may need to handle network retry logic in here in case a packet goes missing, and ws connection loses a block.
The text was updated successfully, but these errors were encountered: