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

Enforce source chain query ordering #1004

Merged
merged 5 commits into from
Oct 27, 2021
Merged

Enforce source chain query ordering #1004

merged 5 commits into from
Oct 27, 2021

Conversation

freesig
Copy link
Contributor

@freesig freesig commented Sep 29, 2021

INCLUDED CHANGES:

  • This enforces the source chain query order as header sequence ascending.
    I'm open to it being the other way around but it was probably defaulting to this but wasn't actually enforced.

TODO:

  • CHANGELOG(s) updated with appropriate info

@timotree3
Copy link
Collaborator

We are doing oldest to newest because that's the natural order in the databases as that's the way the chain grows.

Art has responded

It's true that the db grows oldest to newest, bit the pointers point from newer to old, so if you are verifying anything about chain integrity or header pointers, I think newest to oldest is what makes the most sense, and matches the documented behavior.

Just including some discussion for posterity. I think Art has a fair point but I don't want to bikeshed

@freesig
Copy link
Contributor Author

freesig commented Oct 12, 2021

I'm not sure what to do with this. Should we keep the current ordering or flip it to the way it used to work in lmdb?

Copy link
Member

@maackle maackle left a comment

Choose a reason for hiding this comment

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

I approve, and I think this is how the ordering should be by default.

The argument for ordering in reverse ordering is that you might want to follow the backlinks to prev_header. But why would you want to do that, when your source chain is already validated at this point, so that you can be sure that the backlinks are valid?

The other argument for reverse ordering is if you are doing a query of limited results. In that case though, we'd need the ability to specify ordering in either direction.

So for now it makes most sense to keep it in chronological order. The dev can always iterate the results in reverse order if they want to.

@freesig freesig merged commit 672feab into develop Oct 27, 2021
@freesig freesig deleted the sourcechain-order branch October 27, 2021 11:31
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.

4 participants