Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

slots: slot lenience must take into account block proposal portion #9138

Merged
6 commits merged into from
Jun 19, 2021

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Jun 17, 2021

The slot lenience calculation was being done on the original slot duration without accounting for the block proposal portion. This PR fixes that and additionally adds a max_block_proposal_slot_portion so that the block proposal duration is never exceeded regardless of the slot lenience.

polkadot companion: paritytech/polkadot#3294

@andresilva andresilva added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 17, 2021
@andresilva andresilva requested a review from bkchr June 17, 2021 23:01
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

LGTM, but I don't understand why SlotLenienceType::Linear is there if not used.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Some nitpicks, otherwise looks good

client/consensus/aura/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/slots/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/slots/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@andresilva andresilva force-pushed the andre/fix-slot-lenience branch from 52a2524 to 7996a77 Compare June 19, 2021 15:17
@andresilva
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Jun 19, 2021

Trying merge.

@ghost ghost merged commit 4d9f03d into master Jun 19, 2021
@ghost ghost deleted the andre/fix-slot-lenience branch June 19, 2021 20:37
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants