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.

Sign up for GitHub

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

Jump to bottom

feat(babe, epoch): retrieve first slot number correctly #3841

Merged
merged 50 commits into from
Apr 15, 2024
Merged

feat(babe, epoch): retrieve first slot number correctly #3841

merged 50 commits into from
Apr 15, 2024

Conversation

EclesioMeloJunior
Copy link
Member

@EclesioMeloJunior EclesioMeloJunior commented Apr 1, 2024 •
edited
Loading

Changes

  • Introduced retrieveFirstNonOriginBlockSlot that retrieves the block #1 slot number, if there is more than one them select the correct one based on ancestry
  • Changed BABE initialize epoch to check if epoch was skipped
  • Change node initialization createStateService method that now instantiates the genesis runtime and retrieve BabeConfigurations as it is required for the epoch state
  • This PR will make the code extensible for a full node that does not keep the entire chain history

Tests

go test -tags integration github.com/ChainSafe/gossamer

Issues

Primary Reviewer

@kishansagathiya

@EclesioMeloJunior EclesioMeloJunior mentioned this pull request Apr 2, 2024
Closed
5 tasks
@EclesioMeloJunior EclesioMeloJunior changed the title feat: handling epoch skipped before initializing epoch feat(babe, epoch): retrieving correctly very first slot number Apr 4, 2024
@EclesioMeloJunior EclesioMeloJunior marked this pull request as ready for review April 4, 2024 22:54
@EclesioMeloJunior EclesioMeloJunior mentioned this pull request Apr 9, 2024
Closed
kishansagathiya
kishansagathiya reviewed Apr 10, 2024
View reviewed changes
dot/state/epoch.go Show resolved Hide resolved
dot/state/epoch.go Outdated Show resolved Hide resolved
dot/state/epoch.go Outdated Show resolved Hide resolved
dot/state/epoch.go Show resolved Hide resolved
dot/state/epoch.go Show resolved Hide resolved
dot/import.go Outdated Show resolved Hide resolved
dot/import_integration_test.go Outdated Show resolved Hide resolved
dot/state/epoch_test.go Outdated Show resolved Hide resolved
dot/state/epoch_test.go Outdated Show resolved Hide resolved
dot/state/service_integration_test.go Outdated Show resolved Hide resolved
timwu20
timwu20 approved these changes Apr 12, 2024
View reviewed changes
Copy link
Contributor

@timwu20 timwu20 left a comment

Choose a reason for hiding this comment

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

Can you look at the deepsource issues and ensure that you haven't introduced anything.

@dimartiro dimartiro added this to the Block production. BABE milestone Apr 12, 2024
axaysagathiya
axaysagathiya reviewed Apr 15, 2024
View reviewed changes
dot/state/epoch.go Outdated Show resolved Hide resolved
lib/babe/verify_test.go Show resolved Hide resolved
EclesioMeloJunior and others added 2 commits April 15, 2024 11:20
@EclesioMeloJunior @axaysagathiya
remove unneeded blank line
c5cc046 
Co-authored-by: Axay Sagathiya <axaysagathiya@gmail.com>
@EclesioMeloJunior
place t.Parallel() back
f5d0b7a
axaysagathiya
axaysagathiya approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@axaysagathiya axaysagathiya left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Fix the lint problem. Otherwise, LGTM

edwardmack
edwardmack approved these changes Apr 15, 2024
View reviewed changes
Copy link
Member

@edwardmack edwardmack left a comment

Choose a reason for hiding this comment

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

Great work.

dimartiro
dimartiro approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@dimartiro dimartiro left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM did you test it on staging?

@EclesioMeloJunior EclesioMeloJunior merged commit b9168a2 into development Apr 15, 2024
24 checks passed
@EclesioMeloJunior EclesioMeloJunior deleted the eclesio/move-slot-to-header branch April 15, 2024 20:41
timwu20 pushed a commit that referenced this pull request Apr 19, 2024
@EclesioMeloJunior @axaysagathiya @timwu20
feat(babe, epoch): retrieve first slot number correctly (#3841)
652887f 
Co-authored-by: Axay Sagathiya <axaysagathiya@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimjbrettj jimjbrettj jimjbrettj approved these changes

@axaysagathiya axaysagathiya axaysagathiya approved these changes

@kishansagathiya kishansagathiya kishansagathiya approved these changes

@edwardmack edwardmack edwardmack approved these changes

@timwu20 timwu20 timwu20 approved these changes

@dimartiro dimartiro dimartiro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Block production. BABE
Development

Successfully merging this pull request may close these issues.

fix(lib/epoch): use correctly very first slot number
7 participants
@EclesioMeloJunior @dimartiro @timwu20 @edwardmack @kishansagathiya @axaysagathiya @jimjbrettj