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

Expose MMR root through runtime API - use it in BEEFY client #11183

Merged
paritytech-processbot merged 13 commits into from
Apr 13, 2022
Merged

Expose MMR root through runtime API - use it in BEEFY client #11183

paritytech-processbot merged 13 commits into from
Apr 13, 2022

Conversation

@acatangiu
Copy link
Contributor

@acatangiu acatangiu commented Apr 7, 2022 •
edited
Loading

Adds new mmr_root() API to pallet-mmr ; doesn't change runtime storage.
BEEFY client then uses this API as a backup to MmrRoot header digest logs.
Enables removal of MmrRoot header digest entries in the future => lighter headers.

  • beefy-gadget: allow custom runtime api provider
  • beefy-gadget: use mock runtime api in tests
  • pallet-mmr: expose MMR root from state through runtime API
  • beefy-gadget: get MMR root from runtime state
  • pallet-beefy-mmr: remove MMR root from header digests

Moves MMR primitives from frame/merkle-mountain-range/primitives to primitives/merkle-mountain-range so MmrApi can be used by client code.

Fixes paritytech/grandpa-bridge-gadget#42
Fixes paritytech/grandpa-bridge-gadget#406

Polkadot companion: paritytech/polkadot#5276
Cumulus companion: paritytech/cumulus#1161

@acatangiu acatangiu added 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Apr 7, 2022
@acatangiu acatangiu self-assigned this Apr 7, 2022
@acatangiu acatangiu mentioned this pull request Apr 7, 2022
Merged
@acatangiu acatangiu marked this pull request as ready for review April 7, 2022 10:49
acatangiu
acatangiu commented Apr 7, 2022
View reviewed changes
svyatonik
svyatonik approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

Looks good!

@acatangiu
Copy link
Contributor Author

acatangiu commented Apr 8, 2022

Moved MMR primitives from frame/merkle-mountain-range/primitives to primitives/merkle-mountain-range so MmrApi can be used by client code.

@acatangiu acatangiu requested review from a team and shawntabrizi as code owners April 8, 2022 11:07
@acatangiu acatangiu removed request for a team and shawntabrizi April 8, 2022 12:43
@acatangiu acatangiu force-pushed the beefy-mmr-from-state branch from f178cf0 to 449a204 Compare April 8, 2022 13:12
@acatangiu acatangiu mentioned this pull request Apr 11, 2022
Merged
andresilva
andresilva reviewed Apr 11, 2022
View reviewed changes
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

IMO the worker should still rely on digests for fetching the MMR root. Doing it with a runtime call will make it harder for any future light client to verify BEEFY (since it requires a runtime call which a light client can't do), and this would also require changes to state pruning since validators will prune state on finality in which case the runtime call to fetch the MMR root will fail.

@andresilva
Copy link
Contributor

andresilva commented Apr 11, 2022

To be clear, I think adding the runtime API and all the other changes in this PR are good, but due to the reasons above I think it would be safer to keep using the digest.

@joao-paulo-parity joao-paulo-parity mentioned this pull request Apr 11, 2022
Merged
andresilva
andresilva approved these changes Apr 11, 2022
View reviewed changes
@acatangiu
Copy link
Contributor Author

acatangiu commented Apr 11, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 11, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@acatangiu
Copy link
Contributor Author

acatangiu commented Apr 12, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 12, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@acatangiu
Copy link
Contributor Author

acatangiu commented Apr 12, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 12, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@TriplEight
Copy link
Contributor

TriplEight commented Apr 12, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 12, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@TriplEight
Copy link
Contributor

TriplEight commented Apr 12, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 12, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@acatangiu acatangiu mentioned this pull request Apr 12, 2022
Merged
@bkchr
Copy link
Member

bkchr commented Apr 13, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 13, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@bkchr
Copy link
Member

bkchr commented Apr 13, 2022

bot merge force

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 13, 2022

Error: pr-custom-review is not passing for paritytech/polkadot#5276

@acatangiu
Copy link
Contributor Author

acatangiu commented Apr 13, 2022

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 2b4818a into paritytech:master Apr 13, 2022
This was referenced Apr 13, 2022
Closed
Closed
@joao-paulo-parity joao-paulo-parity mentioned this pull request Apr 13, 2022
Merged
girazoki pushed a commit to moonbeam-foundation/substrate that referenced this pull request May 3, 2022
@acatangiu @girazoki
Expose MMR root through runtime API - use it in BEEFY client (parityt…
81546d1 
…ech#11183)

* beefy-gadget: allow custom runtime api provider

* beefy-gadget: use mock runtime api in tests

* pallet-mmr: expose mmr root from state through runtime API

* beefy-gadget: get mmr root from runtime state

* pallet-beefy-mmr: remove MmrRoot from header digests

* frame/mmr: move mmr primitives out of frame

* frame/mmr: completely move primitives out of frame

* address review comments

* beefy-mmr: bring back mmr root from header digest

* clippy fixes for rustc 1.60

* address review comments
@acatangiu acatangiu deleted the beefy-mmr-from-state branch August 22, 2022 13:44
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
@acatangiu @DaviRain-Su
Expose MMR root through runtime API - use it in BEEFY client (parityt…
f223107 
…ech#11183)

* beefy-gadget: allow custom runtime api provider

* beefy-gadget: use mock runtime api in tests

* pallet-mmr: expose mmr root from state through runtime API

* beefy-gadget: get mmr root from runtime state

* pallet-beefy-mmr: remove MmrRoot from header digests

* frame/mmr: move mmr primitives out of frame

* frame/mmr: completely move primitives out of frame

* address review comments

* beefy-mmr: bring back mmr root from header digest

* clippy fixes for rustc 1.60

* address review comments
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@acatangiu @ark0f
Expose MMR root through runtime API - use it in BEEFY client (parityt…
8a9e69a 
…ech#11183)

* beefy-gadget: allow custom runtime api provider

* beefy-gadget: use mock runtime api in tests

* pallet-mmr: expose mmr root from state through runtime API

* beefy-gadget: get mmr root from runtime state

* pallet-beefy-mmr: remove MmrRoot from header digests

* frame/mmr: move mmr primitives out of frame

* frame/mmr: completely move primitives out of frame

* address review comments

* beefy-mmr: bring back mmr root from header digest

* clippy fixes for rustc 1.60

* address review comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tomusdrw tomusdrw Awaiting requested review from tomusdrw

2 more reviewers

@andresilva andresilva andresilva approved these changes

@svyatonik svyatonik svyatonik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@acatangiu acatangiu

Labels

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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add customizable providers for validator-set and payload (for tests) Customize payload extraction

5 participants

@acatangiu @andresilva @TriplEight @bkchr @svyatonik