-
Notifications
You must be signed in to change notification settings - Fork 1.1k
aura/slot_based: Reduce authoring duration of the last produced block #10154
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
base: lexnv/es-westend
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
|
/cmd prdoc --audience node_dev --bump patch |
…e_dev --bump patch'
| // The authoring of the last block that fits into the relay chain is reduced to fit into the | ||
| // slot timing. For example, when the block is full we need sufficient time to | ||
| // propagate and import it before the next slot starts. | ||
| if blocks_produced_so_far >= blocks_produced_per_relay_slot { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not always work if we skip building some block. Since we know the block production interval, we can easily compute the time index when the last block production begins.
| let blocks_produced_per_relay_slot = (self.relay_slot_duration.as_millis() / | ||
| authoring_duration.as_millis() as u128) as usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have slots that are longer than one relay chain slot.
Signed-off-by: Alexandru Vasile <[email protected]>
commit b6646fe Merge: d0260de dd3b145 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 15:00:28 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit d0260de Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 14:56:07 2025 +0000 parachain-system: Revert check skipping Signed-off-by: Alexandru Vasile <[email protected]> commit 72d1831 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 14:52:32 2025 +0000 xcm-emulator: Mock 2 relay descendants to pass verify_relay_parent_descendants Signed-off-by: Alexandru Vasile <[email protected]> commit f0d2851 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 14:51:13 2025 +0000 xcm-emulator: Add consensus-babe dep Signed-off-by: Alexandru Vasile <[email protected]> commit 554f39d Author: Alexandru Vasile <[email protected]> Date: Tue Oct 28 16:09:59 2025 +0000 test: Ensure we run on 3 cores only, not 4 Signed-off-by: Alexandru Vasile <[email protected]> commit 2f88b59 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 28 16:09:24 2025 +0000 cumulus: Add more trace logs Signed-off-by: Alexandru Vasile <[email protected]> commit b6ace96 Merge: 71bffb4 74ac323 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 28 13:53:35 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 71bffb4 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 16:24:01 2025 +0000 Revert "asset-hub-westend: Update test for 1.5s blocks" This reverts commit 9b346a2. commit 7fcf6da Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 16:23:49 2025 +0000 Revert "asset-hub-westend: Reduce block times to 1.5s from 2s" This reverts commit c412c96. commit 361b35e Merge: 5b7f32b f6858bf Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 14:30:41 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 5b7f32b Merge: 9b346a2 ca0d539 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 14:30:35 2025 +0000 Merge remote-tracking branch 'origin/lexnv/es-westend' into lexnv/es-westend commit 9b346a2 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 14:26:15 2025 +0000 asset-hub-westend: Update test for 1.5s blocks Signed-off-by: Alexandru Vasile <[email protected]> commit c412c96 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 11:42:53 2025 +0000 asset-hub-westend: Reduce block times to 1.5s from 2s Signed-off-by: Alexandru Vasile <[email protected]> commit ffbd59a Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 11:28:24 2025 +0000 tests: Increase validator count and add more scheduled cores Signed-off-by: Alexandru Vasile <[email protected]> commit e75164d Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 11:28:06 2025 +0000 zombienet-sdk-helpers: Add more logs Signed-off-by: Alexandru Vasile <[email protected]> commit ca0d539 Merge: cf83236 0bde5c8 Author: Alexandru Vasile <[email protected]> Date: Fri Oct 24 18:06:51 2025 +0300 Merge branch 'master' into lexnv/es-westend commit cf83236 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:57:49 2025 +0000 zombinet: Enable elastic_scaling_asset_hub_westend test Signed-off-by: Alexandru Vasile <[email protected]> commit 4a49c97 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:57:04 2025 +0000 zombinet: Remove old 0020 elastic scaling test Signed-off-by: Alexandru Vasile <[email protected]> commit bc8582f Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:55:55 2025 +0000 zombienet: Enable extra debug logs Signed-off-by: Alexandru Vasile <[email protected]> commit 5c437bd Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:54:13 2025 +0000 zombinet: Ensure ES works for AHWestend Signed-off-by: Alexandru Vasile <[email protected]> commit 4f68c9d Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 10:25:40 2025 +0000 para_system: Temp fix check verify_relay_parent_descendants iff offset > 1 Signed-off-by: Alexandru Vasile <[email protected]> commit e795f16 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 09:33:38 2025 +0000 asset-hub-westend/genesis: Add dev stakers to enable zombinet tests Signed-off-by: Alexandru Vasile <[email protected]> commit 0035100 Merge: 3427c67 6d64746 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 21 14:28:02 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 3427c67 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 14 07:30:35 2025 +0000 cargo: Remove experimental-ump-signals feature Signed-off-by: Alexandru Vasile <[email protected]> commit 369ca30 Merge: 366fe39 2e716e1 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 14 07:29:15 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 366fe39 Author: Alexandru Vasile <[email protected]> Date: Wed Oct 8 10:07:55 2025 +0000 ci: Fix zombienet test Signed-off-by: Alexandru Vasile <[email protected]> commit d7ea31c Author: Alexandru Vasile <[email protected]> Date: Tue Oct 7 16:01:22 2025 +0000 ci: Add missing bracket Signed-off-by: Alexandru Vasile <[email protected]> commit 0acde44 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 7 15:54:41 2025 +0000 ci/tests: Double check 0020 panics in CI Signed-off-by: Alexandru Vasile <[email protected]> commit 3d53a69 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 7 15:37:53 2025 +0000 zombinet/0020: Add asset hub westend with elastic scaling Signed-off-by: Alexandru Vasile <[email protected]> commit 9f47d7e Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:56:47 2025 +0000 ah-westend: Elastic Scaling with 3 cores on AssetHub Westend Signed-off-by: Alexandru Vasile <[email protected]> commit 63f697b Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:54:46 2025 +0000 ah-westend: Implement runtime api for parent offset Signed-off-by: Alexandru Vasile <[email protected]> commit 84fb876 Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:53:59 2025 +0000 ah-westend: Build into relay chain with offset of 1 Signed-off-by: Alexandru Vasile <[email protected]> commit 04f534a Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:51:54 2025 +0000 ah-westend: Enable experimental-ump-signals feature for systems Signed-off-by: Alexandru Vasile <[email protected]> Signed-off-by: Alexandru Vasile <[email protected]>
commit 26a6317 Author: Alexandru Vasile <[email protected]> Date: Fri Oct 31 12:55:36 2025 +0000 para-system: Check verify_relay_parent_descendants only if sufficent blocks Signed-off-by: Alexandru Vasile <[email protected]> commit b6646fe Merge: d0260de dd3b145 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 15:00:28 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit d0260de Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 14:56:07 2025 +0000 parachain-system: Revert check skipping Signed-off-by: Alexandru Vasile <[email protected]> commit 72d1831 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 14:52:32 2025 +0000 xcm-emulator: Mock 2 relay descendants to pass verify_relay_parent_descendants Signed-off-by: Alexandru Vasile <[email protected]> commit f0d2851 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 30 14:51:13 2025 +0000 xcm-emulator: Add consensus-babe dep Signed-off-by: Alexandru Vasile <[email protected]> commit 554f39d Author: Alexandru Vasile <[email protected]> Date: Tue Oct 28 16:09:59 2025 +0000 test: Ensure we run on 3 cores only, not 4 Signed-off-by: Alexandru Vasile <[email protected]> commit 2f88b59 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 28 16:09:24 2025 +0000 cumulus: Add more trace logs Signed-off-by: Alexandru Vasile <[email protected]> commit b6ace96 Merge: 71bffb4 74ac323 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 28 13:53:35 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 71bffb4 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 16:24:01 2025 +0000 Revert "asset-hub-westend: Update test for 1.5s blocks" This reverts commit 9b346a2. commit 7fcf6da Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 16:23:49 2025 +0000 Revert "asset-hub-westend: Reduce block times to 1.5s from 2s" This reverts commit c412c96. commit 361b35e Merge: 5b7f32b f6858bf Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 14:30:41 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 5b7f32b Merge: 9b346a2 ca0d539 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 14:30:35 2025 +0000 Merge remote-tracking branch 'origin/lexnv/es-westend' into lexnv/es-westend commit 9b346a2 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 14:26:15 2025 +0000 asset-hub-westend: Update test for 1.5s blocks Signed-off-by: Alexandru Vasile <[email protected]> commit c412c96 Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 11:42:53 2025 +0000 asset-hub-westend: Reduce block times to 1.5s from 2s Signed-off-by: Alexandru Vasile <[email protected]> commit ffbd59a Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 11:28:24 2025 +0000 tests: Increase validator count and add more scheduled cores Signed-off-by: Alexandru Vasile <[email protected]> commit e75164d Author: Alexandru Vasile <[email protected]> Date: Mon Oct 27 11:28:06 2025 +0000 zombienet-sdk-helpers: Add more logs Signed-off-by: Alexandru Vasile <[email protected]> commit ca0d539 Merge: cf83236 0bde5c8 Author: Alexandru Vasile <[email protected]> Date: Fri Oct 24 18:06:51 2025 +0300 Merge branch 'master' into lexnv/es-westend commit cf83236 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:57:49 2025 +0000 zombinet: Enable elastic_scaling_asset_hub_westend test Signed-off-by: Alexandru Vasile <[email protected]> commit 4a49c97 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:57:04 2025 +0000 zombinet: Remove old 0020 elastic scaling test Signed-off-by: Alexandru Vasile <[email protected]> commit bc8582f Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:55:55 2025 +0000 zombienet: Enable extra debug logs Signed-off-by: Alexandru Vasile <[email protected]> commit 5c437bd Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 14:54:13 2025 +0000 zombinet: Ensure ES works for AHWestend Signed-off-by: Alexandru Vasile <[email protected]> commit 4f68c9d Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 10:25:40 2025 +0000 para_system: Temp fix check verify_relay_parent_descendants iff offset > 1 Signed-off-by: Alexandru Vasile <[email protected]> commit e795f16 Author: Alexandru Vasile <[email protected]> Date: Thu Oct 23 09:33:38 2025 +0000 asset-hub-westend/genesis: Add dev stakers to enable zombinet tests Signed-off-by: Alexandru Vasile <[email protected]> commit 0035100 Merge: 3427c67 6d64746 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 21 14:28:02 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 3427c67 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 14 07:30:35 2025 +0000 cargo: Remove experimental-ump-signals feature Signed-off-by: Alexandru Vasile <[email protected]> commit 369ca30 Merge: 366fe39 2e716e1 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 14 07:29:15 2025 +0000 Merge remote-tracking branch 'origin/master' into lexnv/es-westend commit 366fe39 Author: Alexandru Vasile <[email protected]> Date: Wed Oct 8 10:07:55 2025 +0000 ci: Fix zombienet test Signed-off-by: Alexandru Vasile <[email protected]> commit d7ea31c Author: Alexandru Vasile <[email protected]> Date: Tue Oct 7 16:01:22 2025 +0000 ci: Add missing bracket Signed-off-by: Alexandru Vasile <[email protected]> commit 0acde44 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 7 15:54:41 2025 +0000 ci/tests: Double check 0020 panics in CI Signed-off-by: Alexandru Vasile <[email protected]> commit 3d53a69 Author: Alexandru Vasile <[email protected]> Date: Tue Oct 7 15:37:53 2025 +0000 zombinet/0020: Add asset hub westend with elastic scaling Signed-off-by: Alexandru Vasile <[email protected]> commit 9f47d7e Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:56:47 2025 +0000 ah-westend: Elastic Scaling with 3 cores on AssetHub Westend Signed-off-by: Alexandru Vasile <[email protected]> commit 63f697b Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:54:46 2025 +0000 ah-westend: Implement runtime api for parent offset Signed-off-by: Alexandru Vasile <[email protected]> commit 84fb876 Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:53:59 2025 +0000 ah-westend: Build into relay chain with offset of 1 Signed-off-by: Alexandru Vasile <[email protected]> commit 04f534a Author: Alexandru Vasile <[email protected]> Date: Tue Sep 30 11:51:54 2025 +0000 ah-westend: Enable experimental-ump-signals feature for systems Signed-off-by: Alexandru Vasile <[email protected]> Signed-off-by: Alexandru Vasile <[email protected]>
…ust-aura-auth-duration
| let last_reported_slot = self.last_reported_slot.unwrap_or_default(); | ||
| if last_reported_slot != next_slot { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sandreim Let me know if I interpreted correctly the #10154 (comment) 🙏
For 3 cores, this removes the wrong reliance on the relay slot duration and produces the desired output:
// First block
Adjusted proposal duration. duration=1.987s
// Second block
Adjusted proposal duration. duration=1.992s
// Last block
Adjusted the last block production attempt for the slot. last_reported_slot=Slot(293653272) next_slot=Slot(293653273) authoring_duration=1.488s
Adjusted proposal duration. duration=1.488s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but I don't think this works in practice if blocks are fuller. If you have a 1.5s block, even if let's say there isn't much latency, say 50ms to the next guy, this means that after the announcement it takes 150ms to fetch it and another 1500ms to execute, assuming ref hw. So the next guy would have been already built it's own block at same height.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that is worst case scenario (full blocks). Otherwise I'd cut it to 1s just to leave more room for latency.
Happy to hear some more opinions on how much a safe cut should be.
cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs
Outdated
Show resolved
Hide resolved
|
|
||
| /// The amount of time the authoring duration of the last block production attempt | ||
| /// should be reduced by to fit into the slot timing. | ||
| const BLOCK_PRODUCTION_ADJUSTMENT_MS: Duration = Duration::from_millis(500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not work for 500ms blocks. Shouldn't this actually be computed based on some average expected latency ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets consider the case of 500ms blocks (be it ES or @bkchr blocks). Let's say we'd have an authorship duration for last block in slot of just 50ms (not possible with this PR) and latency between current author and next author is 200ms. The timeline is as follows:
- last block X production at height N begins at T
- block is announced at T + 50ms
- next guy receives announcement and requests the block at T + 250ms
- author receives the request at T + 450ms
- next guy begins producing it's first block Y of the slot at height N at T + 500ms
- next guy receives block X at T + 650ms
So this won't really work because it takes 3 x latency to even receive the block . I think we should consider reducing authoring over a larger period of time to account for the latency and maximum weight of the last block authored in a slot.
For the example above not building any blocks in the last 1s of the slot would do it.
@skunert WDYT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this makes sense to me! Since we can have quite large ~250ms latencies, the last 1s should be conservative enough to capture most cases 🙏
Have adjusted the code a bit to skip building the last 1s, relying on the time of the "next slot"
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general the logic looks good, just some pedantic nits :D.
I would like to see some additional test cases in the slot timer tests for this though.
| \ / default authoring time`\n- `SlotInfo` is adjusted to account for the number\ | ||
| \ of para blocks produced so far\n- While at it, have added a few extra trace\ | ||
| \ logs\n\n### Testing Done\n\nTested on top of:\n- https://github.com/paritytech/polkadot-sdk/pull/9880\n\ | ||
| \n```\nDEBUG tokio-runtime-worker aura::cumulus: [Parachain] New block production\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like some logs snuck in here? :D
| &mut self, | ||
| mut authoring_duration: Duration, | ||
| ) -> Option<Duration> { | ||
| let Ok((duration, next_slot)) = self.time_until_next_slot() else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should rename time_until_next_slot, because its not really reflecting the next slot, but more the next block timepoint.
| let now = duration_now(); | ||
| let now = now.as_millis().saturating_sub(self.time_offset.as_millis()); | ||
|
|
||
| let last_reported_slot = self.last_reported_slot.unwrap_or_default(); | ||
| let Some(last_slot_timestamp) = last_reported_slot.timestamp(slot_duration) else { | ||
| tracing::error!(target: LOG_TARGET, "Failed to obtain the last slot timestamp"); | ||
| return Err(()) | ||
| }; | ||
|
|
||
| // Compute when the next different slot starts. | ||
| let next_different_slot_time = | ||
| last_slot_timestamp.as_millis() + slot_duration.as_millis() as u64; | ||
| let remaining_millis = next_different_slot_time.saturating_sub(now as u64); | ||
| let next_aura_slot = | ||
| Slot::from_timestamp(Timestamp::from(next_different_slot_time as u64), slot_duration); | ||
|
|
||
| Ok((Duration::from_millis(remaining_millis as u64), next_aura_slot)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I was thinking whether we can simplify this a bit with less type fiddling. Keeping Duration as much as possible seems to help at least a bit. But it never gets super nice.
| let now = duration_now(); | |
| let now = now.as_millis().saturating_sub(self.time_offset.as_millis()); | |
| let last_reported_slot = self.last_reported_slot.unwrap_or_default(); | |
| let Some(last_slot_timestamp) = last_reported_slot.timestamp(slot_duration) else { | |
| tracing::error!(target: LOG_TARGET, "Failed to obtain the last slot timestamp"); | |
| return Err(()) | |
| }; | |
| // Compute when the next different slot starts. | |
| let next_different_slot_time = | |
| last_slot_timestamp.as_millis() + slot_duration.as_millis() as u64; | |
| let remaining_millis = next_different_slot_time.saturating_sub(now as u64); | |
| let next_aura_slot = | |
| Slot::from_timestamp(Timestamp::from(next_different_slot_time as u64), slot_duration); | |
| Ok((Duration::from_millis(remaining_millis as u64), next_aura_slot)) | |
| let now = duration_now(); | |
| let now = now.saturating_sub(self.time_offset); | |
| let last_reported_slot = self.last_reported_slot.unwrap_or_default(); | |
| let next_slot = last_reported_slot + Slot::from(1); | |
| let Some(next_slot_timestamp) = next_slot.timestamp(slot_duration) else { return Err(()) }; | |
| let remaining_time = next_slot_timestamp.as_duration().saturating_sub(now); | |
| Ok((remaining_time, next_slot)) |
| return Some(authoring_duration); | ||
| }; | ||
|
|
||
| let Ok((next_duration_change, next_slot_change)) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| let Ok((next_duration_change, next_slot_change)) = | |
| let Ok((duration_until_next_slot, next_slot)) = |
| }; | ||
|
|
||
| // The authoring of blocks must stop 1 second before the slot ends. | ||
| let deadline = next_duration_change.saturating_sub(BLOCK_PRODUCTION_ADJUSTMENT_MS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| let deadline = next_duration_change.saturating_sub(BLOCK_PRODUCTION_ADJUSTMENT_MS); | |
| let duration_until_deadline = next_duration_change.saturating_sub(BLOCK_PRODUCTION_ADJUSTMENT_MS); |
| // - Block 11: next slot change in 0.993s - skipped by the deadline | ||
| // - Block 12: next slot change in 0.493s - skipped by the deadline | ||
| if authoring_duration < | ||
| BLOCK_PRODUCTION_MINIMUM_INTERVAL_MS.saturating_sub(BLOCK_PRODUCTION_THRESHOLD_MS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can be more generous and increase BLOCK_PRODUCTION_THRESHOLD_MS to 100ms or something. I introduced the original BLOCK_PRODUCTION_THRESHOLD_MS not because blocks below 500ms are not viable at all, but because I wanted to prevent that someone assigns 36 cores to their chain and we arrives at <200ms blocks. So I chose this arbitrary value that we only produce blocks every 500ms for now.
In our case here, producing a 400ms block is also okay.
| }; | ||
|
|
||
| // The authoring of blocks must stop 1 second before the slot ends. | ||
| let deadline = next_duration_change.saturating_sub(BLOCK_PRODUCTION_ADJUSTMENT_MS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, everything here currently assumes that every slot changes brings a different author. Someone could run a single-collator parachain which does not need to respect these offsets. We would break that use-case.
This PR adjusts the block authoring to stop producing blocks 1 second before the scheduled slot change.
This introduces a safety buffer to prevent blocks from being authored too late for inclusion.
Testing Done
Tested on top of:
3 cores 2s blocks
12 cores 500ms blocks
Part of: #9848