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

runtime: remove randomness collective flip pallet #3347

Merged
2 commits merged into from
Jun 22, 2021
Merged

runtime: remove randomness collective flip pallet #3347

2 commits merged into from
Jun 22, 2021

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Jun 22, 2021 •
edited
Loading

This is insecure and should not be used, in particular as we have BABE as a better source of randomness.

@andresilva andresilva added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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 22, 2021
@andresilva
Copy link
Contributor Author

andresilva commented Jun 22, 2021 •
edited
Loading

This pallet stored the last 81 block hashes in its storage, should we write a runtime migration to clean that up?

@andresilva andresilva requested a review from shawntabrizi June 22, 2021 21:57
@ordian ordian added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Jun 22, 2021
@shawntabrizi
Copy link
Member

shawntabrizi commented Jun 22, 2021 •
edited
Loading

This pallet stored the last 81 block hashes in its storage, should we write a runtime migration to clean that up?

Yes, probably. Or we can do governance motion.

rphmeier
rphmeier approved these changes Jun 22, 2021
View reviewed changes
Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

bless up

@shawntabrizi
add storage migration to remove key
29bde59 
Co-Authored-By: parity-processbot <>
@shawntabrizi
Copy link
Member

shawntabrizi commented Jun 22, 2021 •
edited
Loading

@andresilva take a look at the migration. if all looks okay, you can merge

we should take a look at this migration on westend when that happens

@andresilva
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Jun 22, 2021

Trying merge.

@ghost ghost merged commit 869a130 into master Jun 22, 2021
@ghost ghost deleted the andre/remove-randomness-collective-flip branch June 22, 2021 23:36
@s3krit s3krit mentioned this pull request Jun 30, 2021
Merged
@apopiak apopiak mentioned this pull request Oct 19, 2021
Closed
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ordian ordian ordian left review comments

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@rphmeier rphmeier rphmeier approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andresilva @shawntabrizi @ordian @rphmeier