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

store: Head of store needs to be the *contiguous* head of the store #201

Closed
renaynay opened this issue Jun 24, 2024 · 2 comments · Fixed by #207
Closed

store: Head of store needs to be the *contiguous* head of the store #201

renaynay opened this issue Jun 24, 2024 · 2 comments · Fixed by #207
Assignees
Labels
bug Something isn't working store

Comments

@renaynay
Copy link
Member

renaynay commented Jun 24, 2024

Currently in main, the head of the store is updated when an Append is done (via #186) because the assumption was that the syncer only does contiguous Appends to the store. This is actually not true because the syncer can write to the store via Append inside incomingNetHead whereby a header that comes in through headersub (or via p2p.Exchange request) can be verified non-adjacently and then applied to the store if it passes the non-adjacent verification.

We need to fix this by ensuring that the store knows of its own highest header that is contiguous (meaning all headers below the head are adjacent).

Note: once the Tail() method is implemented, the head of the store should be the highest header that is contiguous all the way down to the Tail().Height()

@renaynay renaynay added bug Something isn't working store labels Jun 24, 2024
@renaynay
Copy link
Member Author

Needs a test on the store to ensure that an Append that occurs that is non-adjacent to the highest head written to disk would still be successfully written to the disk but NOT reflected in Head() method until all headers below that height have also been written to the disk.

@renaynay
Copy link
Member Author

renaynay commented Oct 4, 2024

#207

cristaloleg added a commit that referenced this issue Oct 22, 2024
## Overview

The main idea of this PR is to make `Store[H].Head` working properly, to
be precise: returning head that was written to the disk (*). Along with
that `heightSub.height` is increased monotonically to prevent bugs when
we have store appends out of order.

To test everything I'm adding 2 new tests: one that verifies out of
order appends and another when which does this concurrently. Which
helped to find 2 or even 3 edge cases during coding.

Fixes #201
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working store
Projects
None yet
2 participants