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

Fix panic when asking for a salted hash of key with len != 32 #18

Closed
wants to merge 1 commit into from

Conversation

gnunicorn
Copy link
Contributor

@gnunicorn gnunicorn commented Feb 4, 2021

[When upgrading substrate to latest [parity-db 2.0.0](https://github.com/paritytech/substrate/pull/8015), we [saw a panic in the benchmarking](https://gitlab.parity.io/parity/substrate/-/jobs/801118#L2252) of code we haven't touched. A short investigation revealed that the [recently added key salting feature](https://github.com/paritytech/parity-db/commit/64389057d3db2a72f92e43259db1a0ad65c66632) doesn't properly deal with keys of length other than 32`. This PR fixes that behavior for both circumstances of the key being smaller then (our case) but also larger than 32 character.

I wanted to add tests, but noticed there aren't any and that a general infrastructure for them is missing.. (e.g. easily create a testable Column)

@arkpar may I ask you to release this as 2.0.1 soon, so I can merge the dependencies PR?!?

@arkpar
Copy link
Member

arkpar commented Feb 4, 2021

This only uses first 32 bytes of the key. If two long keys share the same 32 byte prefix there will be a collision. I've implemented a better fix in 8ba14c7 and published it as 0.2.1

@arkpar arkpar closed this Feb 4, 2021
@arkpar arkpar deleted the ben-fix-salted-shorter-keys branch February 4, 2021 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants