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 import db on rc v1.6.0 #5018

Merged
merged 7 commits into from Feb 24, 2023
Merged

Conversation

iulianpascalau
Copy link
Contributor

Reasoning behind the pull request

  • import DB stopped abnormally on epoch 3

Proposed changes

  • due to the refactoring of the resolvers/requesters components, we need to call the SetEpochHandler on the storage header requester in order to make the import-db process run as designed. This call is necessary so the manual epochs handler component can open one epoch in advance as of the current processing epoch so the start of epochs events will work

Testing procedure

  • standard system test
  • after the standard system test, execute an import-db process on all shards. Import-db should work.

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2023

Codecov Report

❗ No coverage uploaded for pull request base (rc/v1.6.0@d0b7a43). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head b81c183 differs from pull request most recent head d2525ec. Consider uploading reports for the commit d2525ec to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff              @@
##             rc/v1.6.0    #5018   +/-   ##
============================================
  Coverage             ?   70.64%           
============================================
  Files                ?      666           
  Lines                ?    86516           
  Branches             ?        0           
============================================
  Hits                 ?    61119           
  Misses               ?    20812           
  Partials             ?     4585           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

}

func TestSetEpochHandlerToHdrResolver_CannotSetEpoch(t *testing.T) {
func TestSetEpochHandlerToHdrRequester_GetErr(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func TestSetEpochHandlerToHdrRequester_GetErr(t *testing.T) {
func TestSetEpochHandlerToHdrRequester(t *testing.T) {

same for TestSetEpochHandlerToHdrResolver_GetErr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

)

// RequestersContainerStub -
type RequestersContainerStub struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use the one from testscommon/dataRetriever ? common_test.go is part of dataRetriever_test package so there should be no cyclic dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

t.Run("get function errors should return error", func(t *testing.T) {
t.Parallel()

localErr := errors.New("err")
Copy link
Contributor

Choose a reason for hiding this comment

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

can be extracted as global

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

System test passed.

@@ Log scanner @@

fix-import-db-on-rc-v1.6.0

================================================================================

  • Known Warnings 8
  • New Warnings 2
  • Known Errors 0
  • New Errors 1
  • Panics 0
    ================================================================================
  • block hash does not match 1951
  • miniblocks does not match 0
  • miniblock hash does not match 0
  • receipts hash missmatch 0
  • wrong nonce in block 698
    ================================================================================
  • No jailed nodes on the thestnet
    ================================================================================

@iulianpascalau iulianpascalau merged commit 5fcba4f into rc/v1.6.0 Feb 24, 2023
@iulianpascalau iulianpascalau deleted the fix-import-db-on-rc-v1.6.0 branch February 24, 2023 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants