A Bug Discovered

Monthly Contribution - August 2024

💡Opened 4 PRs, and 3 PRs were merged to Prysm Opened 1 PR to EIPs Prysm #1 fix(tests): Correct misleading variable names and expressions in test files by syjn99 · Pull Request #14292 · prysmaticlabs/prysmWhat type of PR is this? Other What does this PR do? Why is it needed?

Jun's LogJun Song

Last August (2025), I found there's a minor bug in P2P part of Prysm, so I submitted a PR to fix it(You can check it from above article). I'd like to briefly explain the issue:

• Every Ethereum consensus node has seq_number for versioning purpose. If its metadata is changed, seq_number is incremented.
• A node validates seq_number when it receives a ping message from random nodes. It rejects the message if "I remember larger seq_number than you provided at last time".
• Prysm has --p2p-static-id option, which persists the identity of node in terms of P2P network.
• So, there was a problem here. Prysm had not saved the updated seq_number, thus seq_number always starts from zero.

There is an option to provide the metadata as a file(--p2p-metadata), so my PR tried to write whenever the node updates its seq_number.

Unexpected Resurgence

There was no review: it was not that serious problem as I expected at first. But suddenly, the PR was getting attention from the maintainers at mid of this July. I asked Manu (who is now working on PeerDAS) why, and he said that there was a P2P issue in Fulu Devnet and happend to find this PR.

Rebasing was needed as it was one-year-old code, so I worked with joy. In Fulu, P2P metadata requires one more field which means I had to handle total three versions of metadata. For example, I figured out the way to determine its version from reading a metadata file. I completed my work real quick, and requested the review.

While I got some feedbacks about design decision and idiomatic Go, there was a new comment from Kasey, suggesting an alternative way to fix this issue.

A Pivotal Turning Point: Use DB instead!

So his point was valid. Why do we have to rely on the file system for persistence? Prysm uses a bolt DB, so let's just add one more entry for saving the P2P metadata. Also, --p2p-metadata option is quite old and not working properly, we might deprecate it.

Even though deprecating an existing feature should be decided by maintainers, I presented my opinion. I don't think writing often into file system is not a best practice: handling an error was hard because there are tons of reason of failing to write a file (e.g., permission problem). We need complex and unnecessary lines of code to handel it.

Eventually, this PR was closed. But I sent DM to Manu, that I'm keen to be assigned the follow-up task. We are all aligned with what to do for the task, so I wanted to finish my work.

Finally Merged!

Persist metadata sequence number using Beacon DB by syjn99 · Pull Request #15554 · OffchainLabs/prysm

What type of PR is this? Bug fix What does this PR do? Why is it needed? This PR uses the Beacon DB to save metadata sequence number if either of the following conditions is met: the static peer I…

GitHubOffchainLabs

I raised a Github Issue with the context and to-do list, and I submitted a new PR for resolving it. As I got some comments about idiomatic code, I took lots of effort to write my code for minimizing the review round trips.

Thanks for Ephemery Testnet and boltbrowser, I could quickly test the functionality.

After one round trip, I finally got approved from the maintainers, and my work is now merged in Prysm!

I have been contributing to open source projects of Ethereum about an year, and this contribution is especially special for me: By communicating with team members, I could significantly improve the quality of feature and code itself. I learned how to write clean Go code (especially for handling an error) and also it was a great chance to touch DB part of Prysm. It was a really enjoyable experience as the team members reviewed my work very nicely, thanks!