Skip to content
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

Remove pallet::without_storage_info from bridge GRANDPA pallet #1478

Merged
merged 14 commits into from
Oct 10, 2022

Conversation

svyatonik
Copy link
Contributor

@svyatonik svyatonik commented Jun 27, 2022

related to #1433
it'll stay draft until the similar issue with system pallet is resolved (see explanation below)

I've tried to implement that, but looks like there are some more general problems that need to be solved first. Right now the main issue is that the generic header is not implementing the MaxEncodedLen. And it is what we're storing in the runtime storage. The similar case is with the system pallet, which is storing recent headers - so let's see how frame guys will solve it there. I assume that we'll have some upper bound on header size? Then we'll also need to do similar thing here. I've been wrong obviously - system pallet is only storing block hashes. But there's still problem with paras pallet (https://github.com/paritytech/polkadot/blob/master/runtime/parachains/src/paras/mod.rs), which doesn't generate storage info too.

With parachains finality pallet it is even more complex. because we deal with multiple parachains there. So we shall use max(max-of-parachain-1, max-of-parachain-2, ...).

The fix for messages pallet looks easier - we already are limiting the message size. The only thing is that it is missing from the pallet configuration (it is configured through the message verifier trait).

UPD: I doubt that the parachains pallet will be fixed, because it is deployed at relay chain

@svyatonik svyatonik added the A-chores Something that has to be done, as part of regular maintenance label Jun 27, 2022
@svyatonik svyatonik marked this pull request as ready for review October 7, 2022 13:24
modules/grandpa/src/mock.rs Show resolved Hide resolved
modules/grandpa/src/storage_types.rs Outdated Show resolved Hide resolved
modules/grandpa/src/storage_types.rs Outdated Show resolved Hide resolved
bin/rialto-parachain/runtime/src/lib.rs Outdated Show resolved Hide resolved
modules/grandpa/src/storage_types.rs Outdated Show resolved Hide resolved
@svyatonik svyatonik enabled auto-merge (squash) October 10, 2022 07:41
@svyatonik svyatonik enabled auto-merge (squash) October 10, 2022 08:08
Copy link
Contributor

@bkontur bkontur left a comment

Choose a reason for hiding this comment

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

lgtm

@svyatonik svyatonik merged commit b7918c8 into master Oct 10, 2022
@svyatonik svyatonik deleted the max-encoded-len-for-grandpa-pallet branch October 10, 2022 08:32
@svyatonik svyatonik added the PR-audit-needed A PR has to be audited before going live. label Oct 10, 2022
jiguantong added a commit to darwinia-network/darwinia-messages-substrate that referenced this pull request Nov 17, 2022
hackfisher pushed a commit to darwinia-network/darwinia-messages-substrate that referenced this pull request Nov 21, 2022
* update substrate dep master > v0.9.30

* Companion for paritytech/parity-bridges-common#1589

* Companion for paritytech/parity-bridges-common#1593

* Companion for paritytech/parity-bridges-common#1478

* Companion for paritytech/parity-bridges-common#1596

* Companion for paritytech/parity-bridges-common#1600

* Companion for paritytech/parity-bridges-common#1603

* Companion for paritytech/parity-bridges-common#1604

* Companion for paritytech/parity-bridges-common#1597 part.1

* Companion for paritytech/parity-bridges-common#1597 part.2 update weights

* Companion for paritytech/parity-bridges-common#1597 part.3

* Companion for paritytech/parity-bridges-common#1597 part.4 fix compile

* clear unused imports

* Companion for paritytech/parity-bridges-common#1613 part.1 undone

* fix tests
jiguantong added a commit to darwinia-network/darwinia-messages-substrate that referenced this pull request Nov 21, 2022
* update substrate dep master > v0.9.30

* Companion for paritytech/parity-bridges-common#1589

* Companion for paritytech/parity-bridges-common#1593

* Companion for paritytech/parity-bridges-common#1478

* Companion for paritytech/parity-bridges-common#1596

* Companion for paritytech/parity-bridges-common#1600

* Companion for paritytech/parity-bridges-common#1603

* Companion for paritytech/parity-bridges-common#1604

* Companion for paritytech/parity-bridges-common#1597 part.1

* Companion for paritytech/parity-bridges-common#1597 part.2 update weights

* Companion for paritytech/parity-bridges-common#1597 part.3

* Companion for paritytech/parity-bridges-common#1597 part.4 fix compile

* clear unused imports

* Companion for paritytech/parity-bridges-common#1613 part.1 undone

* fix tests
boundless-forest added a commit to darwinia-network/darwinia-messages-substrate that referenced this pull request Jan 4, 2023
* Prepare polkadot v0.9.30 for darwinia-2.0 (#226)

* update substrate dep master > v0.9.30

* Companion for paritytech/parity-bridges-common#1589

* Companion for paritytech/parity-bridges-common#1593

* Companion for paritytech/parity-bridges-common#1478

* Companion for paritytech/parity-bridges-common#1596

* Companion for paritytech/parity-bridges-common#1600

* Companion for paritytech/parity-bridges-common#1603

* Companion for paritytech/parity-bridges-common#1604

* Companion for paritytech/parity-bridges-common#1597 part.1

* Companion for paritytech/parity-bridges-common#1597 part.2 update weights

* Companion for paritytech/parity-bridges-common#1597 part.3

* Companion for paritytech/parity-bridges-common#1597 part.4 fix compile

* clear unused imports

* Companion for paritytech/parity-bridges-common#1613 part.1 undone

* fix tests

* 0930 > master

* try fix CI

* fix CI

* Use `H160` as `AccountId` (#230)

* Use H160 in darwinia-core

* H256 > H160

* update moonbeam account

* Fix tests

* remove debug println

* Fix review

* Fix review

* Compatible `derive_account_id` (#234)

* new derive_account_id

* adjust

* add test

* Keep compatible of the derive way

* Fix test

* Update converter

* Self review

Co-authored-by: Aki Wu <aki.wu@itering.com>

* update cargo

* master -> polkadot-v0.9.30

Co-authored-by: bear <boundless.forest@outlook.com>
Co-authored-by: Aki Wu <aki.wu@itering.com>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
…ytech#1478)

* remove pallet::without_storage_info from bridge GRANDPA pallet

* StoredBridgedHeader

* spelling

* fix benchmarks

* MAX_BRIDGED_AUTHORITIES: 256 -> 2048

* Update modules/grandpa/src/storage_types.rs

Co-authored-by: Adrian Catangiu <adrian@parity.io>

* Update modules/grandpa/src/storage_types.rs

Co-authored-by: Adrian Catangiu <adrian@parity.io>

* moved max authorities + header size to chain primitives

* removed unused code

* new -> try_new

* fix benchmarks compilation

Co-authored-by: Adrian Catangiu <adrian@parity.io>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
…ytech#1478)

* remove pallet::without_storage_info from bridge GRANDPA pallet

* StoredBridgedHeader

* spelling

* fix benchmarks

* MAX_BRIDGED_AUTHORITIES: 256 -> 2048

* Update modules/grandpa/src/storage_types.rs

Co-authored-by: Adrian Catangiu <adrian@parity.io>

* Update modules/grandpa/src/storage_types.rs

Co-authored-by: Adrian Catangiu <adrian@parity.io>

* moved max authorities + header size to chain primitives

* removed unused code

* new -> try_new

* fix benchmarks compilation

Co-authored-by: Adrian Catangiu <adrian@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chores Something that has to be done, as part of regular maintenance PR-audit-needed A PR has to be audited before going live.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants