feat(starknet_consensus_orchestrator): use a struct to collect arguments for validate_proposal #4301
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions
crates/starknet_consensus_orchestrator/src/sequencer_consensus_context.rs line 232 at r1 (raw file):
block_info_validation: BlockInfoValidation, proposal_id: ProposalId, batcher: &'a dyn BatcherClient,
Avoid lifetimes in structs whenever feasible
Suggestion:
batcher: Arc<dyn BatcherClient>,crates/starknet_consensus_orchestrator/src/sequencer_consensus_context.rs line 815 at r1 (raw file):
// TODO(Arni): Remove the clippy when switch to ProposalInit. #[allow(clippy::too_many_arguments)] async fn validate_proposal<'a>(mut args: ProposalValidateArguments<'a>) {
Suggestion:
async fn validate_proposal(mut args: ProposalValidateArguments<'a>) {
guy-starkware
force-pushed
the
guyn/proposals/validate_argument_struct
branch
from
32afe5b to
25315a9
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @matan-starkware)
crates/starknet_consensus_orchestrator/src/sequencer_consensus_context.rs line 232 at r1 (raw file):
Previously, matan-starkware wrote…
Avoid lifetimes in structs whenever feasible
Done.
crates/starknet_consensus_orchestrator/src/sequencer_consensus_context.rs line 815 at r1 (raw file):
// TODO(Arni): Remove the clippy when switch to ProposalInit. #[allow(clippy::too_many_arguments)] async fn validate_proposal<'a>(mut args: ProposalValidateArguments<'a>) {
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware)
crates/starknet_consensus_orchestrator/src/sequencer_consensus_context.rs line 815 at r1 (raw file):
Previously, guy-starkware wrote…
Done.
should also be able to remove the clippy
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matan-starkware)
crates/starknet_consensus_orchestrator/src/sequencer_consensus_context.rs line 815 at r1 (raw file):
Previously, matan-starkware wrote…
should also be able to remove the clippy
Ah, yes, I was going to do that. Thanks!
…nts for validate_proposal
guy-starkware
force-pushed
the
guyn/proposals/validate_argument_struct
branch
from
25315a9 to
c4cdda5
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @guy-starkware)
No description provided.