Skip to content

Conversation

@lexnv
Copy link
Contributor

@lexnv lexnv commented Sep 12, 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/subxt-team

@lexnv lexnv requested review from arkpar, bkchr, cheme and jsdw September 12, 2023 16:59
@lexnv lexnv self-assigned this Sep 12, 2023
@lexnv lexnv added A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). R0-no-crate-publish-required The change does not require any crates to be re-published. labels Sep 12, 2023
@paritytech-ci paritytech-ci requested a review from a team September 12, 2023 17:00
// that the value doesn't exist in the trie.
self.update_recorded_keys(full_key, RecordedForKey::Value);
},
TrieAccess::InlineValue { full_key } => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bkchr Would like to double check this with you 🙏

(introduced by: paritytech/trie#194)

// Non-existing access means we recorded all trie nodes up to the value.
// Not the actual value, as it doesn't exist, but all trie nodes to know
// that the value doesn't exist in the trie.
self.update_recorded_keys(full_key, RecordedForKey::Value);
Copy link
Contributor

Choose a reason for hiding this comment

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

this should switch to RecordedForKey::None I think

Copy link
Member

Choose a reason for hiding this comment

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

No the current value is correct. The comment also says why we pass there Value.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand it (this did make sense to me).

Copy link
Member

Choose a reason for hiding this comment

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

Good find 🙈

paritytech/trie#202

See this docs: https://github.com/paritytech/trie/blob/692ee41a6bd0df36252663d2f7974ab1c368d9a0/trie-db/src/lib.rs#L193-L220

Maybe I should have given None a different name, but the docs are explaining it.

// Non-existing access means we recorded all trie nodes up to the value.
// Not the actual value, as it doesn't exist, but all trie nodes to know
// that the value doesn't exist in the trie.
self.update_recorded_keys(full_key, RecordedForKey::Value);
Copy link
Member

Choose a reason for hiding this comment

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

No the current value is correct. The comment also says why we pass there Value.

Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv lexnv merged commit 0bebc8a into master Sep 13, 2023
@lexnv lexnv deleted the lexnv/update_trie_db branch September 13, 2023 11:18
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 <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). R0-no-crate-publish-required The change does not require any crates to be re-published.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants