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

chainHead: Add support for storage closest merkle descendant #14818 #1153

Merged
merged 21 commits into from
Sep 18, 2023

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Aug 25, 2023

This PR adds support for fetching the closest merkle value of some key.

Builds on top of

Migrates paritytech/substrate#14818 to the monorepo.
Closes: paritytech/substrate#14550
Closes: #1506

// @paritytech/subxt-team

lexnv added 9 commits August 25, 2023 13:30
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
and TrieBackend

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv added A0-needs_burnin Pull request needs to be tested on a live validator node before merge. DevOps is notified via matrix R0-silent Changes should not be mentioned in any release notes labels Aug 25, 2023
@lexnv lexnv self-assigned this Aug 25, 2023
@paritytech-ci paritytech-ci requested review from a team August 28, 2023 09:45
…_db_merkle

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
lexnv added a commit that referenced this pull request Sep 13, 2023
This PR updates:
- trie-db from 0.27.1 to 0.28.0
- trie-bench from 0.37.0 to 0.38.0 (deb-dependency)


While at it, also adapts the recorder to take into account the newly
added `TrieAccess::InlineValue`.

Needed by:
- #1153

@paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
…_db_merkle

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv requested review from jsdw and skunert September 13, 2023 16:11
taqtiqa-mark pushed a commit to subversive-upstream/polkadot-sdk-cumulus that referenced this pull request Sep 14, 2023
This PR updates:
- trie-db from 0.27.1 to 0.28.0
- trie-bench from 0.37.0 to 0.38.0 (deb-dependency)


While at it, also adapts the recorder to take into account the newly
added `TrieAccess::InlineValue`.

Needed by:
- paritytech/polkadot-sdk#1153

@paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

Looks nice!

substrate/client/rpc-spec-v2/src/chain_head/tests.rs Outdated Show resolved Hide resolved
skunert

This comment was marked as duplicate.

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Copy link
Contributor

@tadeohepperle tadeohepperle left a comment

Choose a reason for hiding this comment

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

I don't know too much about how substrate handles storage, but since the changes got merged into sp-trie and this PR mostly forwards to functions from sp-trie I think this should be all good. Good work!

key: &[u8],
) -> Result<Option<MerkleValue<B::Hash>>, Self::Error> {
self.state.closest_merkle_value(key)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: What does it mean here, if an Error is returned, vs. an Option is returned? Under what circumstances can errors happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, the Option tells us if there is descendant merkle value below the provided key, since the key may not be in the database and may not have any descendants.

On the other hand, the error is a way to propagate that something went wrong during this operation. Mainly on the trie-db side:

  • unable to fetch bytes from the database (invalid state root / incomplete data)
  • corrupted data (failing to decode the node)

Here would be the main place from which those errors originate

@lexnv lexnv merged commit 5d34664 into master Sep 18, 2023
9 checks passed
@lexnv lexnv deleted the lexnv/chainhed_trie_db_merkle branch September 18, 2023 10:54
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
This PR updates:
- trie-db from 0.27.1 to 0.28.0
- trie-bench from 0.37.0 to 0.38.0 (deb-dependency)


While at it, also adapts the recorder to take into account the newly
added `TrieAccess::InlineValue`.

Needed by:
- paritytech#1153

@paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
…aritytech#1153)

This PR adds support for fetching the closest merkle value of some key.


Builds on top of
- paritytech/trie#199

Migrates paritytech/substrate#14818 to the
monorepo.
Closes: paritytech/substrate#14550
Closes: paritytech#1506

// @paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
bkchr pushed a commit that referenced this pull request Apr 10, 2024
…nt` (#1153)

* Refactor logic

* Thanks svyatonik help, it compile

* Fix failed unit test

* Remove compile warning

* Rename

* Return result in pay_relayers_rewards

* Fix runtime compile issue

* Use MessageNonce

* Fix review issue

* Missing u64 replacement

* Revert return type changes

* Fix merge issue

* Remove useless clone
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-needs_burnin Pull request needs to be tested on a live validator node before merge. DevOps is notified via matrix R0-silent Changes should not be mentioned in any release notes
Projects
None yet
3 participants