-
Notifications
You must be signed in to change notification settings - Fork 707
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
define block hash provider and default impl using frame_system #4080
Conversation
The CI pipeline was cancelled due to failure one of the required jobs. |
f7a9d65
to
bb88cde
Compare
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.
The PR makes sense for MMR. I was also wondering if in order to make it more generic it would make sense to change frame_system::Config::BlockHashCount
to a method or something more generic. Not sure. But we could explore this in a separate issue. And related to this I was curious how your BlockHashProvider
implementation looks like. Are you keeping a separate item in the storage with more block_num -> block_hash
items then the frame_system
? And how do you decide when to prune that ?
I'm not quiet sure how this would work for chains that do not have quick finality. Subspace for example, Runtime would not know at which point finality comes so our
That is fair. Thanks!
At the moment, we have not decided on the approach of how to fetch yet but we have some options.
Overall, this PR should allow us to impl |
@serban300 seems like check is failing, IIUC, is unrelated to this PR. Is this expected ? |
Yes, the failing checks are unrelated to the PR so far |
@acatangiu @serban300 I'm not sure why the the |
|
5b513cc
This PR introduces
BlockHashProvider
intopallet_mmr::Config
This type is used to get
block_hash
for a givenblock_number
rather than directly usingframe_system::Pallet::block_hash
The
DefaultBlockHashProvider
usesframe_system::Pallet::block_hash
to get theblock_hash
Closes: #4062