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: update p2p metaData file when it is changed #14401

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

syjn99
Copy link
Contributor

@syjn99 syjn99 commented Aug 30, 2024

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

This PR addresses Issue #13586, where the beacon-chain does not update the metaData file as expected. The issue arises because there is currently no code to write to the metaData file after initialization. As a result, the sequence number stored on disk always remains zero. This behavior is acceptable when the --p2p-static-id flag is not provided. However, it causes significant issues in the P2P processes when a user intends to maintain the same peer ID across sessions. (especially when ping to peers, but those peers found that the sequence number of sender is lower than they manage in their own tables.)

Which issues(s) does this PR fix?

Fixes #13586

Other notes for review

  • There is a migration code(migrateFromProtoToSsz) in case user has metaData file already.
  • A point of discussion: Should we handle potential errors when the P2P service encounters issues while writing to files? Currently, these errors are ignored. Feedback on this approach would be appreciated.
  • I'm afraid that it is common to refer metadata version at phase0 is V1(consensus-specs), but prysm has been referring it as V0.

Acknowledgements

  • I have read CONTRIBUTING.md.
  • I have made an appropriate entry to CHANGELOG.md.
  • I have added a description to this PR with sufficient context for reviewers to understand this PR.

@syjn99 syjn99 changed the title fix: update metaData file when it is changed fix: update p2p metaData file when it is changed Aug 30, 2024
@syjn99 syjn99 marked this pull request as ready for review August 30, 2024 12:40
@syjn99 syjn99 requested a review from a team as a code owner August 30, 2024 12:40
@nalepae nalepae self-requested a review September 6, 2024 15:32
@nalepae
Copy link
Contributor

nalepae commented Sep 6, 2024

Thanks for you PR, will look.

@syjn99
Copy link
Contributor Author

syjn99 commented Sep 9, 2024

@nalepae
Fixed to save metaData file when p2p service stops.

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.

Prysm doesn't update metaData in file
2 participants