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

Make tips return lca as well #3301

Merged
merged 1 commit into from
Jan 21, 2021
Merged

Make tips return lca as well #3301

merged 1 commit into from
Jan 21, 2021

Conversation

nzpr
Copy link
Collaborator

@nzpr nzpr commented Jan 20, 2021

Overview

Fork choice tips are ranked tips of the DAG. They do not make sense without knowing LCA, which were used to rank tips.
This PR package LCA next to tips returned by estimator.

This is useful for easier management of data and doing less work when preparing casper snapshot.

@nzpr
Copy link
Collaborator Author

nzpr commented Jan 20, 2021

Two tests failing are inherited from block-merge branch, see #3277

@nzpr nzpr added enhancement feature-branch Development parallel to dev branch labels Jan 20, 2021
@nzpr nzpr added this to the v0.11 Block merging milestone Jan 20, 2021
Copy link
Collaborator

@tgrospic tgrospic left a comment

Choose a reason for hiding this comment

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

Very nice!

@9rb
Copy link
Collaborator

9rb commented Jan 20, 2021

Is lca in this context the same as nearest common parent or nearest common state ? It's better to use a more complete and accurate name since l and c in lca can be misinterpreted or interpreted in multiple ways. It slips my mind now, but the last time we discussed this, we added a further clarification / qualification / specificity that was helpful and self-explanatory

@nzpr
Copy link
Collaborator Author

nzpr commented Jan 21, 2021

@9rb I think it is latest common ancestor. But yes, having L in LCA match also lowest, or least, brings some uncertainty.

@nzpr nzpr merged commit dfa88dd into rchain:feature/block-merge Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature-branch Development parallel to dev branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants