Skip to content

feat(block-producer): fix dangling review comments#522

Merged
Mirko-von-Leipzig merged 4 commits intonext-block-producerfrom
mirko-dangling-review-comments
Oct 24, 2024
Merged

feat(block-producer): fix dangling review comments#522
Mirko-von-Leipzig merged 4 commits intonext-block-producerfrom
mirko-dangling-review-comments

Conversation

@Mirko-von-Leipzig
Copy link
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig commented Oct 23, 2024

This PR

Specifically, addresses this, this and this comment.

Proving times are distributed normally from within a configurable range.

Failure rates are also similarly configurable, which completes one task from #519.

@Mirko-von-Leipzig Mirko-von-Leipzig added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Oct 23, 2024
@Mirko-von-Leipzig Mirko-von-Leipzig marked this pull request as ready for review October 23, 2024 08:23
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you! I left just 2 nits inline.

Question: currently BatchProducer and BlockProducer structs are unused, right?

Comment on lines +233 to +235
/// Used to simulate batch proving by sleeping for
/// a random duration selected from this range.
pub simulated_proof_time: Range<Duration>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we usually wrap comments around 100 chars.

Comment on lines +158 to +160
/// Used to simulate block proving by sleeping for
/// a random duration selected from this range.
pub simulated_proof_time: Range<Duration>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same nit as above.

@Mirko-von-Leipzig
Copy link
Collaborator Author

Question: currently BatchProducer and BlockProducer structs are unused, right?

Correct; one of the next PRs would replace the current production producers with these ones.

@Mirko-von-Leipzig Mirko-von-Leipzig merged commit 90311b3 into next-block-producer Oct 24, 2024
@Mirko-von-Leipzig Mirko-von-Leipzig deleted the mirko-dangling-review-comments branch October 24, 2024 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants