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

fix(lib/blocktree): improve blocktree.GetHashesAtNumber #3799

Merged
merged 9 commits into from
Mar 22, 2024

Conversation

EclesioMeloJunior
Copy link
Member

Changes

  • Change the name from GetBlocksAtNumber to GetHashesAtNumber given the fact that this function returns a slice of hashes and not a slice of blocks

  • Improve the function to run based on the actual number and not based on the parent hash of the desired number?

  • The function first try to search the slice of hashes with the same number in the in-memory block tree first if results are found then we return the hashes, if nothing returns then try to retrieve the hash from database (we map the number -> hash in the db)

  • These changes are needed to complete issue BABE: added reusing last epoch data in case of skipped epochs #3430. Gossamer should be aware of the very first slot number (slot number for the first non genesis block) and might exist a case where more than 1 block is the first non genesis block, then we should use GetHashesAtNumber to check if there is more than one block #1 and in this case use the correct first slot

Tests

go test -tags integration github.com/ChainSafe/gossamer/lib/blocktree
go test -tags integration github.com/ChainSafe/gossamer/dot/state

Issues

Primary Reviewer

@

@EclesioMeloJunior EclesioMeloJunior added S-state issues related to dot/state package. T-enhancement this issue/pr covers improvement of existing functionality. C-simple Minor changes changes, no additional research needed. Good first issue/review. labels Mar 14, 2024
Copy link
Contributor

@jimjbrettj jimjbrettj left a comment

Choose a reason for hiding this comment

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

few comments, but lgtm

dot/state/grandpa_test.go Show resolved Hide resolved
lib/blocktree/blocktree.go Show resolved Hide resolved
lib/blocktree/blocktree.go Show resolved Hide resolved
lib/blocktree/blocktree.go Show resolved Hide resolved
lib/blocktree/node.go Show resolved Hide resolved
@EclesioMeloJunior EclesioMeloJunior merged commit f9ab505 into development Mar 22, 2024
24 checks passed
@EclesioMeloJunior EclesioMeloJunior deleted the eclesio/move-slot-to-header branch March 22, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-simple Minor changes changes, no additional research needed. Good first issue/review. S-state issues related to dot/state package. T-enhancement this issue/pr covers improvement of existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants