Refactors to fit into sequencer TestNetwork #87
Conversation
| "Sending DA proposal to the builder states for view {:?}", | ||
| view_number | ||
| ); | ||
| if !sender.validate(&da_proposal.signature, &encoded_txns_hash) { |
There was a problem hiding this comment.
Is it because the validate function handles the leader == sender check, that we no longer need to explicitly check it here?
There was a problem hiding this comment.
It wasn't necessary from the get go, because HotShot simply won't emit an event if the sender isn't the leader. Technically, it's true for the signature validation as well, but I left it be as a sanity check.
| } | ||
| } | ||
|
|
||
| #[instrument(skip_all, fields(sender, quorum_proposal.data.view_number))] | ||
| async fn handle_qc_event<TYPES: NodeType>( |
There was a problem hiding this comment.
Nit: I think qc should be quorum here. No need to do any renaming in this PR. I'm going to make a separate issue for all qc-to-quorum renaming that we can do in the future.
There was a problem hiding this comment.
Made a low-priority issue: #88.
src/utils.rs
Outdated
There was a problem hiding this comment.
I really like these utility functions!
Nit: Can we add hotshot to function or struct names that are only for HotShot events? E.g., I would think connect_inner is a helper function for connecting to any event service, but it turns out to be for HotShot specifically.
There was a problem hiding this comment.
Sorry, not sure I understand you right. Those are HotShot events in the same sense as the builder-core is HotShot builder-core, i.e. it builds for any HotShot application, so adding HotShot feels redundant, as if we renamed any other type T we have to HotShotT because it's part of HotShot ecosystem
There was a problem hiding this comment.
Was thinking it could be a some other URL, e.g., for solver. But I think what you said also makes sense. No need to change!
Closes #<ISSUE_NUMBER>
This PR:
Modifies
run_non_permissioned_standalone_builder_serviceto accept generic event stream for usage in testing with event service replaced by direct access to hotshot event stream. Logic surrounding event service moved to a separate Stream implementation that can be called in the sequencer binary as an utility.This PR does not:
Key places to review: