-
Notifications
You must be signed in to change notification settings - Fork 95
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
[encointer] set target blocktime to 6s #462
[encointer] set target blocktime to 6s #462
Conversation
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 good to me.
/// Maximum number of blocks simultaneously accepted by the Runtime, not yet included | ||
/// into the relay chain. | ||
pub const UNINCLUDED_SEGMENT_CAPACITY: u32 = 2; |
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 have seen and implemented 3 for the unincluded segment capacity, but I remember learning that 3 and 2 actually resolve to the same value of 6s blocktime in the end.
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 good.
/merge |
Enabled Available commands
For more information see the documentation |
CI fails out-of-scope: |
@brenzi all the pallets you are using, none of them has scheduled anything based on blocks? Maybe your democracy or similar? You will produce twice as many blocks, meaning that anything that is using blocks for scheduling will happen twice as fast. |
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.
Just removing my review for Basti's comment due to auto-merge.
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Head branch was pushed to by a user without write access
@bkchr I'm pretty sure we're fine:
top-down check: Our testnet Gesell uses the same pallets with 6s block time and cross-checking settings there didn't reveal any issues |
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.
Can you please check if the instructions in https://paritytech.github.io/polkadot-sdk/master/polkadot_sdk_docs/guides/async_backing_guide/index.html are enough to create this PR?
@kianenigma that document encompasses much more than this PR and Encointer benefitted from many other PR's in this repo that did all the prep work around async backing. In this PR I basically just needed to change constants - and there the linked docs are fine |
/merge |
Ty @brenzi. If you are using timestamps, it should all be good :) |
Enabled Available commands
For more information see the documentation |
f542af4
into
polkadot-fellows:main
Encointer communities could benefit from 6s block time because the network is used for IRL point-of-sale or person-to-person transactions
Encointer is unaffected by paritytech/polkadot-sdk#3268 as its pallets have since ever based time on block timestamps
The parameters are copy-paste from people, as introduced by #308