Skip to content

fix: increment seq_number on metadata cgc changes#7752

Merged
matthewkeil merged 1 commit intoChainSafe:peerDASfrom
hughy:peerDAS-metadata-cgc-increment-seq-number
Apr 29, 2025
Merged

fix: increment seq_number on metadata cgc changes#7752
matthewkeil merged 1 commit intoChainSafe:peerDASfrom
hughy:peerDAS-metadata-cgc-increment-seq-number

Conversation

@hughy
Copy link
Copy Markdown
Contributor

@hughy hughy commented Apr 25, 2025

Motivation

phase0 spec states that seq_number should be incremented by 1 whenever any other field in metadata changes: https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/p2p-interface.md#metadata

Description

increment seq_number when the metadata cgc field changes

@hughy hughy requested a review from a team as a code owner April 25, 2025 21:49
@hughy hughy force-pushed the peerDAS-metadata-cgc-increment-seq-number branch from 0a7fbed to aaf0e19 Compare April 29, 2025 15:54
@hughy
Copy link
Copy Markdown
Contributor Author

hughy commented Apr 29, 2025

I rebased this on top of #7751 to avoid conflicts from the rename from cgc to custodyGroupCount

@nflaig
Copy link
Copy Markdown
Member

nflaig commented Apr 29, 2025

I rebased this on top of #7751 to avoid conflicts from the rename from cgc to custodyGroupCount

merged the PR, I think you have to rebase this one once more

increment seq_number when the metadata cgc field changes to follow phase0 spec
definition of seq_number
@hughy hughy force-pushed the peerDAS-metadata-cgc-increment-seq-number branch from aaf0e19 to 7565d08 Compare April 29, 2025 16:59
Copy link
Copy Markdown
Member

@matthewkeil matthewkeil left a comment

Choose a reason for hiding this comment

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

LGTM!! 🚀

@matthewkeil matthewkeil merged commit 4d0917b into ChainSafe:peerDAS Apr 29, 2025
6 checks passed
@wemeetagain
Copy link
Copy Markdown
Member

🎉 This PR is included in v1.34.0 🎉

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.

5 participants