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

Bump trie-db to 0.27.1 (fix storage iteration bug + change seek behavior with prefix) #13630

Conversation

koute
Copy link
Contributor

@koute koute commented Mar 17, 2023

This PR bumps trie-db to version 0.27.1 (see this PR for details) which mainly changes two things:

  1. It fixes a bug with storage iteration where in certain cases the iterator would return keys outside of the specified prefix (see this issue for details)

  2. It changes how the iterator works when both a prefix and a key from which to start iteration from is specified. This cleans up some corner cases and solidifies the behavior of how the iteration works which, AFAIK, previously was entirely accidental. The iteration now always follows the following two rules:

    • All of the keys returned by the iterator are guaranteed to start with the specified prefix (if present).
    • All of the keys returned by the iterator are guaranteed to be equal or bigger lexicographically to the starting key (if present).

Tagging this as B1-note_worthy and C3-medium for RPC nodes; everyone else can most likely treat this as C3-low.

Fixes polkadot-js/apps#9103

Side-note

As far as I can see this shouldn't affect runtime behavior yet as the host functions (clear_prefix and storage_kill) to access this from within the runtime are not actually used (they're marked as register_only). That said, after merging this PR we will have to wait until the nodes are upgraded before we can consider removing the register_only flag from those host functions. (Since the host function is technically exposed; it's just not used by the runtime. This is why I've tagged this E3-host_functions even though it shouldn't affect the current runtime.)

@koute koute added A0-please_review Pull request needs code review. I3-bug The node fails to follow expected behavior. C3-medium PR touches the given topic and has a medium impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit B1-note_worthy Changes should be noted in the release notes E3-host_functions PR adds new host functions which requires a node release before a runtime upgrade. T0-node This PR/Issue is related to the topic “node”. T2-API This PR/Issue is related to APIs. labels Mar 17, 2023
@koute koute requested review from cheme and a team March 17, 2023 14:11
@koute koute removed the T0-node This PR/Issue is related to the topic “node”. label Mar 17, 2023
@cheme
Copy link
Contributor

cheme commented Mar 17, 2023

Certainly should be deployed as soon as possible, but medium sounds right to me. If another pr is running a burn in, can be an idea to include this one, but I don t think it is really needed.

@koute
Copy link
Contributor Author

koute commented Mar 17, 2023

Certainly should be deployed as soon as possible, but medium sounds right to me. If another pr is running a burn in, can be an idea to include this one, but I don t think it is really needed.

Hmm... well, it should be fine, but I guess I can probably leave a full burn-in + partial burn-in (from the current chain head) with this applied over the weekend and see how it goes, just in case.

@cheme
Copy link
Contributor

cheme commented Mar 17, 2023

Certainly should be deployed as soon as possible, but medium sounds right to me. If another pr is running a burn in, can be an idea to include this one, but I don t think it is really needed.

Hmm... well, it should be fine, but I guess I can probably leave a full burn-in + partial burn-in (from the current chain head) with this applied over the weekend and see how it goes, just in case.

For me it is not essential, let's get others opinion :)

@koute
Copy link
Contributor Author

koute commented Mar 17, 2023

For me it is not essential, let's get others opinion :)

Essential or not I'll do it anyway purely just for my own peace of mind. (: You can never have enough testing.

@koute
Copy link
Contributor Author

koute commented Mar 17, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit bab32f8 into paritytech:master Mar 17, 2023
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
crystalin pushed a commit to moonbeam-foundation/substrate that referenced this pull request May 3, 2023
Neopallium pushed a commit to PolymeshAssociation/substrate that referenced this pull request Jun 5, 2023
Neopallium pushed a commit to PolymeshAssociation/substrate that referenced this pull request Jun 7, 2023
Neopallium pushed a commit to PolymeshAssociation/substrate that referenced this pull request Jun 7, 2023
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C3-medium PR touches the given topic and has a medium impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit E3-host_functions PR adds new host functions which requires a node release before a runtime upgrade. I3-bug The node fails to follow expected behavior. T2-API This PR/Issue is related to APIs.
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

"Phantom" multisig approvals appearing on Accounts page
3 participants