-
Notifications
You must be signed in to change notification settings - Fork 666
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
Pallet-MMR should use custom block_hash
getter instead of using frame_system::block_hash
for non-canonicalised blocks
#4062
Comments
IIUC this
So instead of
Sounds ok to me. I would not call it |
Yes, this is causing the issue. I hope this makes sense |
Ok, sounds good to me. I can help review the PR and push it through once you post it. |
@acatangiu PR for the above proposed solution path is here - #4080 |
This PR introduces `BlockHashProvider` into `pallet_mmr::Config` This type is used to get `block_hash` for a given `block_number` rather than directly using `frame_system::Pallet::block_hash` The `DefaultBlockHashProvider` uses `frame_system::Pallet::block_hash` to get the `block_hash` Closes: #4062
Is there an existing issue?
Experiencing problems? Have you tried our Stack Exchange first?
Motivation
Currently
pallet_mmr
usesframe_system::block_hash
to get the hash of a given block number while constructing the non-canonicalised keys. This works fine with Polkadot and other blockchains where finality is quicker. Currently, polkadot stores 4096 number of block hashes on the runtime and before the block_hashes are pruned, the blocks are already finalized.At subspace, we do not have a quick finality and we cannot put a static number at which we can gaurantee block finalization happens. With this context, when an MMR proof is being verified, if the particular block is not canonicalised yet, it will look at
frame_system::block_hashes
but by then our runtime already prunes the block hash due toframe_system::Config::BlockHashCount
and the storage returns an Default value for the Hash since theframe_system::block_hash
is a ValueQuery. With the default value given, the MMR proof verification fails on our end.Request
pallet_mmr
should ideally define a custom getter to fetch the block_hash instead of directly using theframe_system
. This will provide the projects using thepallet_mmr
can impl the custom getter to fetch the saidblock_hash
.For polkadot and likes, the impl will just wrap
frame_system::block_hashes
function and should not change anything while Subspace and other project can impl custom logic to fetch the correctblock_hash
for a given non-canonicalised block number.Solution
Solution would be introduce a new type for
pallet_mmr::Config
that can fetchblock_hash
for a given block_numberAre you willing to help with this request?
Yes!
The text was updated successfully, but these errors were encountered: