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

No minimum block time in Runtime #250

Merged
merged 8 commits into from
Feb 25, 2021
Merged

No minimum block time in Runtime #250

merged 8 commits into from
Feb 25, 2021

Conversation

JoshOrndorff
Copy link
Contributor

@JoshOrndorff JoshOrndorff commented Feb 16, 2021

What does it do?

This PR removes the requirement from the runtime that blocks be at least 3 seconds apart. With that requirement gone, we are free to remove the mock timestamp provider.

Solves https://purestake.atlassian.net/browse/MOON-318 (@albertov19)

What important points reviewers should know?

The mock timestamp was introduced because our use of Aura in the standalone node required a minimum time stamp on the order of the block time. Now that the standalone node is removed, there is no reason need to mock the timestamp, or even make arbitrary requirements on a minimum block duration.

Drawback?

One potential drawback of this change is that our runtime logic no longer requires blocks to be any particular amount of time apart anymore. In practice they should always be at least six seconds apart because of the relay chain. And further, correct nodes should ensure that each block's timestamp is withing 30 seconds of its own. Currently the relay chain calidators will not check these timestamps, but this is planned: paritytech/cumulus#343

What value does it bring to the blockchain users?

The block timestamp in will reflect the "real" wall clock time even when running the dev service.

Checklist

  • ❌ Does it require a purge of the network?
  • ✔️ You bumped the runtime version if there are breaking changes in the runtime ?
  • ❌ Does it require changes in documentation/tutorials ?

@JoshOrndorff JoshOrndorff changed the title No minimum block time in Runtimer No minimum block time in Runtime Feb 16, 2021
@crystalin
Copy link
Collaborator

We won't merge this into v6, it will be merged for the tutorial-v6 however

@JoshOrndorff
Copy link
Contributor Author

We won't merge this into v6

Sounds good 👍

it will be merged for the tutorial-v6 however

I don't understand this. Why do we want a separate tutorials tag? That means someone who completes a tutorial, and then wants to sync a node to alphanet will have to re-checkout and rebuild a different node (or pull a different docker image). It also makes it impossible to have a tutorial about running an alphanet node.

Can we just make a new tag v0.6.1 when this goes in and then make the tutorials and the alphanet recommend that version?

@crystalin
Copy link
Collaborator

@JoshOrndorff I believe the tutorial tags are not meant to run an alphanet node. They are designed for developers to run it locally.
@albertov19 correct me if I'm wrong

@JoshOrndorff
Copy link
Contributor Author

the tutorial tags are not meant to run an alphanet node

Yes I understand they aren't to run an alphanet node. But they should run the same version of the software. They just use --dev instead of --chain alphanet.

@tgmichel
Copy link
Contributor

Looks good to me!

@JoshOrndorff JoshOrndorff merged commit 1f9999c into master Feb 25, 2021
@JoshOrndorff JoshOrndorff deleted the joshy-min-block-time branch February 25, 2021 18:06
@tgmichel tgmichel mentioned this pull request Jun 3, 2021
1 task
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.

3 participants