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

test: Add more thorough test for dbwrapper iterators #7956

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@laanwj
Member

laanwj commented Apr 27, 2016

I made a silly mistake in an experimental database wrapper where keys were sorted by char instead of uint8_t. As x86 char is signed the sorting (and thus seeking) for the block index database was messed up, resulting in a segfault due to missing records. This should be caught by the tests.

Add a test to catch:

  • Wrong sorting
  • Seeking errors
  • Iteration result not complete
  • Wrong keys/values from iterator

Also add an assertion to CBlockIndex::GetAncestor, as this it the place it will segfault when block index records are missing. An assertion error is easier to diagnose than a pointer crash (although what we really need is a start-up check whether the database is complete otherwise suggest a reindex).

laanwj added some commits Apr 26, 2016

test: Add more thorough test for dbwrapper iterators
I made a silly mistake in a database wrapper where keys
were sorted by char instead of uint8_t. As x86 char is signed
the sorting for the block index database was messed up, resulting
in a segfault due to missing records.

Add a test to catch:
- Wrong sorting
- Seeking errors
- Iteration result not complete
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli
Member

jonasschnelli commented Apr 27, 2016

utACK 6030625

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 28, 2016

Member

I think this needs another check: that shorter keys get sorted before longer keys if they're otherwise the same. I don't think that we use variable-length keys anywhere, at least not within the same key prefix, but it's good to have the sorting be well-defined as lexicographic.

Member

laanwj commented Apr 28, 2016

I think this needs another check: that shorter keys get sorted before longer keys if they're otherwise the same. I don't think that we use variable-length keys anywhere, at least not within the same key prefix, but it's good to have the sorting be well-defined as lexicographic.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 3, 2016

Member

Closing in favor of #7992

Member

laanwj commented May 3, 2016

Closing in favor of #7992

@laanwj laanwj closed this May 3, 2016

sipa added a commit that referenced this pull request Jun 2, 2016

Merge #7992: Extend #7956 with one more test.
269a440 Add test for dbwrapper iterators with same-prefix keys. (Matt Corallo)
6030625 test: Add more thorough test for dbwrapper iterators (Wladimir J. van der Laan)
84c13e7 chain: Add assertion in case of missing records in index db (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment