This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
pallet-mmr: Change runtime API and RPC API to work with block numbers instead of leaf indices #12339
Description
Issue
This issue was triggered by a good observation made by @serban300 in #12324 (comment):
Current pallet-mmr APIs work with leaf indices, leaving the burden of translating block numbers to leaf indices to the users of the pallet.
Light clients might have an easier experience dealing only with block numbers, and avoiding managing leaf indices and the conversion from block numbers => leaf indices.
Suggested solution
pallet-mmr could expose APIs based on block numbers and internally map those to leaf indices.
Implications
- Breaking API change, both for Runtime API and RPC API.
- Pallet currently only deployed on Rococo so breaking change only for that. Kusama and Polkadot will directly deploy new API.
Feedback requests
cc @vgeddes and @seunlanlege as current (light-client) users of this pallet
Activity
Szegoo commentedon Sep 24, 2022
I will give this a try.
acatangiu commentedon Sep 26, 2022
As shown in #12345 there is also the option to change only RPC API to use block numbers, while leaving existing Runtime API unchanged.
seunlanlege commentedon Sep 27, 2022
And while this change is harmless, it's actually unneccessary. As light clients need to be aware of the
leaf_indexwhich they can use to derive theleaf_postionwhich is needed when verifying mmr proofs with the ckb-merkle-mountain-range libvgeddes commentedon Sep 27, 2022
I'm in favour of the change. It does simplify our offchain relayers that call
mmr_generateProof. They have needed to manually calculate the leaf indices. To compute the correct leaf indices on Kusama and Polkadot, they will need to know the block height when MMR leafs started being generated, and this is extra configuration and deployment hassle.Our system also requires the MMR leaf index for a block. But since the output of
mmr_generateProof(blockNumber)includes the leaf index we're happy. And we'd rather rely on that instead of calculating our own leaf indices.acatangiu commentedon Sep 28, 2022
Seeing how
pallet-mmrhas only been deployed onRococoso far, I'm leaning heavily to just break the API without backwards compatibility if that's acceptable to you.vgeddes commentedon Sep 28, 2022
sounds fine with us 👍
Szegoo commentedon Sep 28, 2022
So is the final decision to have this implemented? If so should #12345 also change the runtime API? Initially, I didn't change the runtime API in the PR, but I don't actually see a reason why we wouldn't change that also.
acatangiu commentedon Sep 28, 2022
Yes, decision is to implement full breaking change. Let's change Runtime API too in #12345