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.

Sign up for GitHub

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

Jump to bottom

chainHead/storage: Fix storage iteration using the query key #1665

Merged
merged 4 commits into from
Sep 26, 2023
Merged

chainHead/storage: Fix storage iteration using the query key #1665

merged 4 commits into from
Sep 26, 2023

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Sep 21, 2023 •
edited

This PR ensures that all storage keys under a prefix are returned by the chainHead_storage method.

Before this PR, the storage_keys was used with just the start_key.
Before the pagination event was generated, the last reported key last_key was saved internally.
When the pagination is resumed, the last_key will serve as the next start_key to the storage_keys API.

However, this behavior does not function properly for non-prefixed storage keys.

Entry keys a, ab, abc share a common prefix and therefore the ab key leads to abc.
However, for a, ab, aB and abc, the aB key does not immediately lead to abc.

To mitigate this, the PR saves the start key of the query, together with the next pagination key.
Improve testing to ensure we have a key entry that doesn't share the prefix with the descendant key.

@paritytech/subxt-team

@lexnv
chainHead/tests: Repro case for non-monotonically storage entries
71bf752 
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv
chainHead/storage: Resume storage iteration using the query key
33109b5 
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv added A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). I2-bug The node fails to follow expected behavior. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Sep 21, 2023
@lexnv lexnv self-assigned this Sep 21, 2023
@lexnv lexnv requested review from skunert and jsdw September 21, 2023 14:46
@lexnv
chainHead/storage: Use key iterator to optimise continue events
a0d1532 
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv
chainHead: Fix build
5291fc7 
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
tadeohepperle
tadeohepperle approved these changes Sep 25, 2023
View reviewed changes
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.

👍

@lexnv lexnv merged commit cc50eda into master Sep 26, 2023
109 checks passed
@lexnv lexnv deleted the lexnv/fix_chainhead_storage branch September 26, 2023 11:36
ordian added a commit that referenced this pull request Sep 27, 2023
@ordian
merge master and resolve conflicts (v8)
6fb4b7f 
* master: (61 commits)
  OpenGov in Westend and Rococo (#1177)
  Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720)
  Migrate polkadot-primitives to v6 (#1543)
  genesis-builder: implemented for all runtimes (#1492)
  `BlockId` removal: `tx-pool` refactor (#1678)
  Bump directories from 4.0.1 to 5.0.1 (#1656)
  Allow debug_assertions in short-benchmarks CI job (#1711)
  chainHead/storage: Fix storage iteration using the query key (#1665)
  Implement more useful traits in `Slot` type (#1595)
  Make downloads in parallel and give more time to complete (#1699)
  Bump actions/checkout from 4.0.0 to 4.1.0 (#1688)
  contracts: Fix incorrect storage alias in mirgration (#1687)
  Fix documentation about justification and `finalized == true` requirement (#1607)
  tweak pallet macro (genesis_config etc) to cater for RA users as well. (#1689)
  Uncoupling pallet-xcm from frame-system's RuntimeCall (#1684)
  Bump aes-gcm from 0.10.2 to 0.10.3 (#1681)
  docs / Update PR template to reflect monorepo (#1674)
  update contributing guide and ui-tests scripts (#1668)
  pallet epm: add `TrimmingStatus` to the mined solution (#1659)
  Update HRMP pallet benchmarking to use benchmarks v2 (#1676)
  ...
ordian added a commit that referenced this pull request Oct 10, 2023
@ordian
Merge branch 'tsv-disabling-node-side' into tsv-disabling-backing
26866fa 
* tsv-disabling-node-side: (69 commits)
  runtime-api: cleanup after v7 stabilization (#1729)
  Move requests-responses and polling from `ChainSync` to `SyncingEngine` (#1650)
  Add custom error message for `StorageNoopGuard` (#1727)
  Clarify docs
  cargo fmt
  add a CAVEAT comment
  implement disabled_validators correctly
  remove unnecessary hash string (#1722)
  OpenGov in Westend and Rococo (#1177)
  Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720)
  Migrate polkadot-primitives to v6 (#1543)
  genesis-builder: implemented for all runtimes (#1492)
  `BlockId` removal: `tx-pool` refactor (#1678)
  Bump directories from 4.0.1 to 5.0.1 (#1656)
  Allow debug_assertions in short-benchmarks CI job (#1711)
  chainHead/storage: Fix storage iteration using the query key (#1665)
  Implement more useful traits in `Slot` type (#1595)
  Make downloads in parallel and give more time to complete (#1699)
  Bump actions/checkout from 4.0.0 to 4.1.0 (#1688)
  contracts: Fix incorrect storage alias in mirgration (#1687)
  ...
@kiltbot kiltbot mentioned this pull request Oct 19, 2023
Closed
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@lexnv
chainHead/storage: Fix storage iteration using the query key (parityt…
022c7cd 
…ech#1665)

This PR ensures that all storage keys under a prefix are returned by the
`chainHead_storage` method.

Before this PR, the `storage_keys` was used with just the `start_key`.
Before the pagination event was generated, the last reported key
`last_key` was saved internally.
When the pagination is resumed, the `last_key` will serve as the next
`start_key` to the `storage_keys` API.

However, this behavior does not function properly for non-prefixed
storage keys.

Entry keys `a`, `ab`, `abc` share a common prefix and therefore the `ab`
key leads to `abc`.
However, for `a`, `ab`, `aB` and `abc`, the `aB` key does not
immediately lead to `abc`.

To mitigate this, the PR saves the start key of the query, together with
the next pagination key.
Improve testing to ensure we have a key entry that doesn't share the
prefix with the descendant key.

@paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 26, 2024
@svyatonik
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
8b50630 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 27, 2024
@svyatonik
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
df3f9d2 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
04ed32f 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
8575f3d 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
1318e2d 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
40336f4 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
d661fb5 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
056582d 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
4026fc8 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
82582c1 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
6d9f1eb 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
fca186c 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
dd82dc7 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
03f186e 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
@svyatonik @serban300
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
345f93e 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
bkchr pushed a commit that referenced this pull request Apr 10, 2024
@svyatonik @bkchr
Per-lane rewards in pallet-bridge-relayers (#1665)
ea98118 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
TomaszWaszczyk pushed a commit to TomaszWaszczyk/polkadot-sdk that referenced this pull request May 27, 2024
@svyatonik @TomaszWaszczyk
Per-lane rewards in pallet-bridge-relayers (paritytech#1665)
99a0c0d 
* per-lane rewards in pallet-bridge-relayers

* add lane id to RewardPaid event

* clippy

* fix benchmarks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skunert skunert skunert approved these changes

@tadeohepperle tadeohepperle tadeohepperle approved these changes

@jsdw jsdw Awaiting requested review from jsdw

Assignees

@lexnv lexnv

Labels
A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I2-bug The node fails to follow expected behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lexnv @skunert @tadeohepperle