[r3.5] cl: seed the ENR custody group count instead of an empty entry - #23592
Merged
Conversation
`setupENR` wrote the `cgc` entry as an empty byte slice:
```go
// cl/p2p/p2p.go
node.Set(enr.WithEntry(p.cfg.NetworkConfig.CgcKey, []byte{}))
```
Empty is the canonical encoding of zero, so a node advertised custody of
no groups.
Nothing corrected it: the only path that rewrites the entry is
`SetCustodyGroupCount`, gated on the count rising above `advertisedCgc`,
which
starts at `CustodyRequirement` — so on a node without attached
validators it never
fires. `p2p-interface.md` notes clients MAY reject a peer advertising
below
`CUSTODY_REQUIREMENT`, so the entry was not merely uninformative.
## Change
Seed the entry with the value `advertisedCgc` already starts at, via a
shared
`EncodeCgc` so the seed and the update path cannot drift on format.
Encoding is unchanged — `big.Int.Bytes()`, big endian with no leading
zero bytes,
zero as the empty string, per `fulu/p2p-interface.md`. The increase-only
gate is
unchanged: `fulu/validator.md` requires a node whose custody requirement
falls to
keep advertising the previous (highest) count, which is what it
implements. Both
are now covered by tests and a comment so neither gets "corrected"
later.
Verified on a gnosis archive node: `cgc` decoded to `0x` before and
`0x04` after.
## Scope
Present on `main`, `release/3.6` and `release/3.5`, and on every chain
with fulu
scheduled — none override `CustodyRequirement`. Cherry-picks to follow
once this
lands.
This does not change how a node computes its own custody, subnet
subscriptions or
column requests: those read the in-memory value, not the ENR. It fixes
what other
nodes believe about this one.
Two related gaps found in review, both pre-existing and filed separately
rather
than widened into this PR: the highest count is not persisted across
restarts, and
an archive node advertises `CustodyRequirement` while custodying all 128
groups.
domiwei
approved these changes
Aug 27, 2026
domiwei
left a comment
domiwei
left a comment
Member
There was a problem hiding this comment.
Approved. I verified this is a clean backport of #23560: the five changed paths and functional hunks are equivalent, the canonical minimal big-endian CGC encoding and increase-only ratchet are preserved, and the focused setup/encoding/ratchet tests pass. All non-skipped PR checks are green. The comment-style nits are non-blocking.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #23560 to release/3.5.
Clean pick: the same five files, same diffstat, no branch-specific adaptations.