Skip to content

Fix chunksBetween, which was always either empty or singleton#1872

Closed
vreuter wants to merge 3 commits intoIntersectMBO:mainfrom
tweag:vince_erin/fix-chunksBetween
Closed

Fix chunksBetween, which was always either empty or singleton#1872
vreuter wants to merge 3 commits intoIntersectMBO:mainfrom
tweag:vince_erin/fix-chunksBetween

Conversation

@vreuter
Copy link
Copy Markdown
Contributor

@vreuter vreuter commented Feb 10, 2026

This change fixes a bug in the chunksBetween function in the Ouroboros.Consensus.Storage.ImmutableDB.Chunks.Internal, whereby the result was always either empty (if the bounds of the range were unequal) or a single-item list (if the bounds were equal).

@vreuter vreuter changed the title Vince erin/fix chunks between Fix chunksBetween, which was always either empty or singleton Feb 10, 2026
@dnadales dnadales self-assigned this Feb 10, 2026
@dnadales dnadales moved this to 👀 In review in Consensus Team Backlog Feb 10, 2026
Copy link
Copy Markdown
Member

@dnadales dnadales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thank you for submitting the patch and for adding the tests!

@dnadales
Copy link
Copy Markdown
Member

dnadales commented Mar 6, 2026

@vreuter, the branch has some conflicts that need to be resolved. Do you want to do it, or should I clone your fork and do it myself. I have no problem doing it, but I wanted to check with you before cloning your fork 👍

@vreuter vreuter force-pushed the vince_erin/fix-chunksBetween branch from 451031c to 5750908 Compare March 8, 2026 14:12
@vreuter
Copy link
Copy Markdown
Contributor Author

vreuter commented Mar 8, 2026

Hey thanks for checking @dnadales , I went ahead and resolved the conflicts now.

<!--
### Patch

- A bullet item for the Patch category.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a bullet explaining the patch change? See the other changelog entries for inspiration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've updated the entry here 7213be4

@dnadales
Copy link
Copy Markdown
Member

dnadales commented Mar 9, 2026

Hey thanks for checking @dnadales , I went ahead and resolved the conflicts now.

Thank you @vreuter. One last request, could you add a "Patch" changelog entry? Then I'll kick off the CI 🙏

vreuter added 2 commits March 9, 2026 20:23
…6d53

fix description of main subject of Immutable DB chunks tests modules
explain the change that is occurring
@vreuter vreuter force-pushed the vince_erin/fix-chunksBetween branch from 5750908 to 7213be4 Compare March 9, 2026 19:25
@vreuter
Copy link
Copy Markdown
Contributor Author

vreuter commented Mar 9, 2026

OK @dnadales , I've described the change with a bulleted item in the Patch subsection.

@dnadales
Copy link
Copy Markdown
Member

Thank you @vreuter!

@dnadales
Copy link
Copy Markdown
Member

@vreuter see #1923

I hope this is ok, but I think it's the path of least resistance.

dnadales added a commit that referenced this pull request Mar 13, 2026
Copied from
#1872. This was
necessary since all the commits need to be signed.
Supersedes:
#1872

Original authors: [vreuter](https://github.com/vreuter) and
[ErinvanderVeen](https://github.com/ErinvanderVeen)

Co-authored-by: Erin van der Veen <erin.vanderveen@tweag.io>
Co-authored-by: Vince Reuter <vince.reuter@tweag.io>
@dnadales
Copy link
Copy Markdown
Member

Merged here #1923

@dnadales dnadales closed this Mar 30, 2026
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Consensus Team Backlog Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants