Skip to content

Conversation

@4meta5
Copy link
Contributor

@4meta5 4meta5 commented Jan 29, 2021

What does it do?

Removes panic in on_finalize if Author storage item isn't set. The requirement should still be enforced by the fact that the inherent which sets this storage item is required to be included in every block afaik paritytech/substrate#8002

I moved Author::kill() to on_initialize so that every block is finalized with the author set (assuming the inherent requirement enforces setting it correctly). I think this is better than clearing the author storage item before the block is finalized.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

Checklist

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

@4meta5 4meta5 marked this pull request as ready for review January 29, 2021 15:46
@4meta5 4meta5 added the A0-pleasereview Pull request needs code review. label Jan 29, 2021
Copy link
Contributor

@JoshOrndorff JoshOrndorff left a comment

Choose a reason for hiding this comment

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

Thanks!

Still needs a spec version bump.

@JoshOrndorff JoshOrndorff added A5-grumble Pull request has minor issues that must be addressed before merging. B0-silent Changes should not be mentioned in any release notes C1-low Does not elevate a release containing this beyond "low priority". labels Jan 29, 2021
@JoshOrndorff JoshOrndorff added A8-mergeoncegreen Pull request is reviewed well. and removed A5-grumble Pull request has minor issues that must be addressed before merging. labels Jan 29, 2021
@4meta5 4meta5 merged commit f21b048 into master Jan 29, 2021
@4meta5 4meta5 deleted the rm-redundant-author-requirement branch January 29, 2021 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A0-pleasereview Pull request needs code review. A8-mergeoncegreen Pull request is reviewed well. B0-silent Changes should not be mentioned in any release notes C1-low Does not elevate a release containing this beyond "low priority".

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants