-
Notifications
You must be signed in to change notification settings - Fork 87
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
ChainDB.Iterator: fix EBB bug #1475
Conversation
We were using the very simple `Test.Util.TestBlock` test block, which was easier to generate, but could not represent EBBs. Now by using the `Test.Storage.TestBlock` test block, we can test with EBBs.
In the past, `ImmDB.getPointAtTip` used to be more expensive (it had to read a block from disk to get its hash). That's why we had `itGetImmDBTip`, implemented using the in-memory chain fragment, in `IteratorEnv`. The ImmDB (and ImmutableDB) keeps the hash of the block at the tip in memory, so we can use `itImmDB` directly and no longer need `itGetImmDBTip`. While at it, add `getTipInfo`, which also includes `IsEBB`, and use that to inspect the tip of the `ImmDB`.
Many thanks to @nfrisby for providing such a detailed bug report! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the thorough tests! I only selected "Request changes" because I'm not sure the fallbackTo
is required, and if it is, then the comments as-is didn't address my confusion about that.
ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl/Iterator.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl/Iterator.hs
Outdated
Show resolved
Hide resolved
Closes #1435. See the comment for more information.
e2dd054
to
652dac5
Compare
You're right. I have rewritten the logic and made all 6 cases explicit (in the tests too). There was indeed no fallback to the VolatileDB needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
bors r+ |
1475: ChainDB.Iterator: fix EBB bug r=mrBliss a=mrBliss Closes #1435. See the comment for more information. Co-authored-by: Thomas Winant <[email protected]>
Closes #1435.
See the comment for more information.