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

Avoid calls to contains_key on small value. #23

Open
2 tasks done
cheme opened this issue Feb 13, 2023 · 2 comments
Open
2 tasks done

Avoid calls to contains_key on small value. #23

cheme opened this issue Feb 13, 2023 · 2 comments
Labels
I5-enhancement An additional feature request.

Comments

@cheme
Copy link
Contributor

cheme commented Feb 13, 2023 •
edited
Loading

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

Calls as https://github.com/paritytech/substrate/blob/1cc2a007290ce32314a17e7d03af7b986e0630dc/frame/system/src/lib.rs#L1016 are accessing the merkle trie with get_hash function.

This allows to avoid fetching a big value, but in case the value size is less than 33 it brings the overhead of calculating the hash instead.

Generally in frame calls to contains_key or exists or storage_hash should only be when the value can be bigger than 32 bytes in length.

There may be multiple place where we can switch to a simple get. Proper fix would be to have the size limit info in the storage map type and do it automatically in these cases.

Steps to reproduce

No response
@bkchr
Copy link
Member

bkchr commented Feb 13, 2023

This allows to avoid fetching a big value, but in case the value size is less than 33 it brings the overhead of calculating the hash instead.

Hashes are cached as well in the trie cache, so this shouldn't be such a huge problem.

Proper fix would be to have the size limit info in the storage map type and do it automatically in these cases.

A proper fix would be to have a proper exists api in the trie crate.

@cheme
Copy link
Contributor Author

cheme commented Feb 14, 2023

A proper fix would be to have a proper exists api in the trie crate.

Sounds right 👍

@the-right-joyce the-right-joyce transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. and removed J0-enhancement labels Aug 25, 2023
liuchengxu added a commit to subcoin-project/polkadot-sdk that referenced this issue Sep 20, 2024
@liuchengxu
Check more consensus rules in block verification (paritytech#23)
4097995 
* Check max_sigops and past_median_time

* Add --major-sync-confirmation-depth

* Separate out header_verifier module

* refactor: extract start() in RunCmd

* Rename to calculate_median_time_past()

* Check size limits

* Check output value

* Panic if an error occurred in the block import

* Check MTP when CSV is activated

* Check if tx is final

* Nit

* Log too many blocks in the queue every 5 seconds

It was printed way too frequent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I5-enhancement An additional feature request.
Projects
SDK Node
Status: backlog
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cheme @juangirini @bkchr @the-right-joyce