Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always Propose when empty_block_timeout is reached and remove redundant proposal conditions #1706

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

86667
Copy link
Contributor

@86667 86667 commented Oct 25, 2024

We now will always broadcast proposals when time_since_last_view_change > empty_block_timeout. This allows us to remove the further check on whether the exponential timeout has yet been reached when proposing (since the latter is always true if the former is true).

This means that if there are no consensus timeouts then we expect blocks to regularly be 1 seconds (+ a few ms) apart.

In timeout() Ive removed the check on has_txns() because transactions will be added to early_proposal when created and when received (if early_proposal exists). Whether there are txs in the mempool at this stage is irrelevant.

The minimum_time_left_for_empty_block variable is useful in a network in which a node is incentivised to add as many txs to their blocks as possible. Since fees are sunk in zq2 I have removed this. Happy to revert if we think it still has use.

@86667 86667 changed the title Remove redundant proposal conditions Always Propose when empty_block_timeout is reached and remove redundant proposal conditions Oct 25, 2024
@86667 86667 force-pushed the rm_redundant_propose_conditions branch from 8b9d622 to c5ceac6 Compare October 25, 2024 15:39
@JamesHinshelwood
Copy link
Contributor

@86667 Did you want to rebase this at some point so we can review and merge it? It looks like a good simplification of the code :)

@86667
Copy link
Contributor Author

86667 commented Dec 11, 2024

@86667 Did you want to rebase this at some point so we can review and merge it? It looks like a good simplification of the code :)

Definitely! It hasn't been forgotten - I am prioritising deposit contract work first so we can get the audit done asap.

@86667 86667 force-pushed the rm_redundant_propose_conditions branch 2 times, most recently from 3fd4bc0 to a1e9edd Compare December 12, 2024 15:57
@86667 86667 self-assigned this Dec 12, 2024
Copy link
Contributor

github-actions bot commented Dec 12, 2024

🐰 Bencher Report

Branchrm_redundant_propose_conditions
Testbedself-hosted
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
process-empty/process-empty📈 view plot
🚷 view threshold
8.41
(-7.96%)
10.40
(80.85%)
produce-full/produce-full📈 view plot
🚷 view threshold
1,730.90
(-16.18%)
2,737.42
(63.23%)
🐰 View full continuous benchmarking report in Bencher

@86667 86667 force-pushed the rm_redundant_propose_conditions branch from a1e9edd to cad3d6b Compare December 23, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants