Skip to content

refactor: rename ValidatorConsensusData to ProposerConsensusData#808

Merged
mergify[bot] merged 2 commits intosigp:unstablefrom
shane-moore:refactor/rename-validator-data-to-proposer
Feb 11, 2026
Merged

refactor: rename ValidatorConsensusData to ProposerConsensusData#808
mergify[bot] merged 2 commits intosigp:unstablefrom
shane-moore:refactor/rename-validator-data-to-proposer

Conversation

@shane-moore
Copy link
Member

@shane-moore shane-moore commented Feb 6, 2026

Issue Addressed

Spec alignment with SSV spec renaming PR #596 (commit e5171469) and SSV Go commit 7b3f7a77f.

Proposed Changes

  • Rename ValidatorConsensusData to ProposerConsensusData and related types (ValidatorConsensusDataValidator, ValidatorConsensusDataLen, ValidatorInstanceId) to align with the spec
  • Add TODO(post-boole) comments on pre-Boole code paths that will be removed when Aggregator and SyncCommittee post-fork

Additional Info

N/A

@shane-moore
Copy link
Member Author

@claude

@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Feb 6, 2026

Claude finished @shane-moore's task in 1m 18s —— View job


Code Review Progress ✓

  • Read all changed files
  • Review renaming consistency
  • Verify SSZ max size calculations
  • Review TODO comments
  • Check for edge cases and potential issues
  • Provide final review feedback

Review Summary

This PR implements a straightforward and well-executed refactoring to align Anchor's naming with the SSV specification. The changes are consistent, correctly applied, and include proper documentation for future cleanup work.

Detailed Findings

Strengths

  1. Naming Consistency: The renaming from ValidatorConsensusData to ProposerConsensusData (and related types) is consistently applied across all 5 files with no missed references.

  2. Correct SSZ Size Calculations:

    • PartialSignatureMessagesLen: Updated from 1512 to 5048 to accommodate worst-case committee scenario (3000 aggregators + 512×4 sync committee contributors). Math verified: 3000 + 2048 = 5048
    • SSVMessageDataLen: Updated from 722412 to 726932, correctly bounded by the larger of consensus messages (722412) and partial sig messages (726932). Math verified: 726 * 1000 + 932 = 726932
  3. Future-Proofing: Appropriate TODO(post-boole) comments added to mark pre-Boole code paths that will be removed after the Aggregator and SyncCommittee roles transition post-fork.

  4. Documentation: Comments updated to reflect new terminology and explain size calculations clearly.

📋 Observations

  1. Post-Boole Cleanup Scope: The TODO comments indicate that several code paths will be removed post-Boole:

    • ValidatorDutyKind enum (anchor/qbft_manager/src/lib.rs:87-92)
    • duty field in ProposerInstanceId (anchor/qbft_manager/src/lib.rs:78-81)
    • BEACON_ROLE_AGGREGATOR and BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION validation branches (anchor/common/ssv_types/src/consensus.rs:340-342)
  2. Naming Rationale: The rename reflects that this consensus data structure is specifically for proposer duties, even though it currently handles aggregator and sync committee duties as well (pre-Boole). This aligns with the spec's direction.

🔍 Minor Observations

  1. Comment Formatting: Line anchor/qbft_manager/src/lib.rs:86 has a minor comment formatting that could be improved:

    // TODO(post-boole): remove this enum. Post-boole, `Proposal` is the only
    // validator specific duty kind.

    Suggestion: Capitalize "Post-Boole" consistently with other TODOs for uniformity.

  2. Test Coverage: The PR doesn't modify tests, which is fine since this is primarily a renaming refactor. Existing tests should continue to pass with the renamed types.

Verification

I verified:

Conclusion

Approved. This is a clean, well-documented refactoring that successfully aligns Anchor with the SSV specification naming. The SSZ size updates are correctly calculated and the TODO comments provide clear guidance for future cleanup work.


@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Feb 6, 2026

Claude finished @shane-moore's task in 1m 4s —— View job


Code Review Complete ✅

  • Read changed files and analyze diff
  • Review type renames for completeness and consistency
  • Verify SSZ size changes
  • Review TODO comments
  • Provide comprehensive feedback

Summary

This PR successfully renames ValidatorConsensusData to ProposerConsensusData and related types to align with the SSV spec. The changes are comprehensive, consistent, and include appropriate TODO comments for future cleanup. I found the implementation to be correct with proper attention to SSZ size updates.

Detailed Review

✅ Type Renames - Complete and Consistent

The rename from ValidatorConsensusDataProposerConsensusData is thorough across all files:

consensus.rs (anchor/common/ssv_types/src/consensus.rs)

  • ValidatorConsensusDataProposerConsensusData
  • ValidatorConsensusDataValidatorProposerConsensusDataValidator
  • ValidatorConsensusDataLenProposerConsensusDataLen
  • Documentation and error messages updated correctly ✅

qbft_manager/src/lib.rs

  • ValidatorInstanceIdProposerInstanceId
  • Map renamed: validator_consensus_data_instancesproposer_consensus_data_instances
  • All type references updated in trait implementations ✅

validator_store/src/lib.rs

  • Method renamed: create_validator_consensus_data_validatorcreate_proposer_consensus_data_validator
  • All instantiations updated to use new type names ✅

✅ SSZ Max Size Updates - Correct

The SSZ size calculations are accurate and properly justified:

PartialSignatureMessages Max (partial_sig.rs:12-17)

/// Maximum number of `PartialSignatureMessage`s: 5048
/// Worst case scenario for a committee with 3000 validators:
/// every validator has an aggregation duty (3000) +
/// every validator is in sync committee and is a contributor to all 4 subnets (512 * 4 = 2048)
/// Calculated as 3000 + 512 * 4 = 5048
pub type PartialSignatureMessagesLen = Sum<Prod<U3, U1000>, Prod<U512, U4>>;
  • Previous: Sum<U1000, U512> = 1512
  • New: Sum<Prod<U3, U1000>, Prod<U512, U4>> = 3000 + 2048 = 5048 ✅
  • Justification: Accounts for 3000 aggregators + 512×4 sync committee contributors ✅

SSVMessage.Data Max (message.rs:58-61)

/// `SSVMessage.Data` max size: 726932
/// `max(consensus_msg_max, partial_sig_max)` = `max(722412, 726932)` = 726932
/// 726932 = 726 * 1000 + 932
pub type SSVMessageDataLen = Sum<Prod<U726, U1000>, U932>;
  • Previous: Sum<Prod<U722, U1000>, U412> = 722412
  • New: Sum<Prod<U726, U1000>, U932> = 726932 ✅
  • Correctly represents max(consensus, partial_sig) = max(722412, 726932) = 726932 ✅

The size increase is due to the increased PartialSignatureMessagesLen, and the calculation properly takes the maximum of the two message type sizes.

✅ TODO Comments - Appropriate

The TODO comments are well-placed and clearly scoped:

consensus.rs:340-342

// TODO(post-boole): remove `BEACON_ROLE_AGGREGATOR` and
// `BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION` branches. Post-Boole, `ProposerConsensusData`
// is only used for `Proposer`.

✅ Correctly identifies code paths to be removed after the Boole fork

qbft_manager/src/lib.rs:79-80

// TODO(post-boole): remove `duty` field and `ValidatorDutyKind`. Post-boole,
// `Proposal` is the only validator specific duty

✅ Marks the duty field and enum for removal

qbft_manager/src/lib.rs:85-86

// TODO(post-boole): remove this enum. Post-boole, `Proposal` is the only
// validator specific duty kind.

✅ Marks entire ValidatorDutyKind enum for removal

These TODOs properly align with the spec changes where aggregator and sync committee will use the new committee-based consensus path post-Boole.

⚠️ Minor Observations

  1. Naming Clarity: The rename improves semantic clarity - ProposerConsensusData better reflects that this type is specifically for proposer duties, especially after the Boole fork when it will exclusively handle block proposals.

  2. Comment Update: The ASCII art comment in consensus.rs:38 was updated correctly:

    // Old: ValidatorConsensusData/BeaconVote SSZ
    // New: ProposerConsensusData/BeaconVote SSZ
  3. No Breaking Changes: The changes are internal type renames without wire format changes, so they're safe for deployment.

📝 Recommendations

No blocking issues found. The PR is ready to merge. Some optional suggestions:

  1. Consider: After the Boole fork activates, create follow-up issues to track the TODO cleanup work (removing pre-Boole code paths).

  2. Testing: The SSZ size changes should be validated by the existing test in message.rs:1184-1236 (ensure_message_sizes_correct). Consider running this test to verify the constants are correct.

  3. Documentation: Consider updating any external documentation or specs that reference ValidatorConsensusData if they exist.

✅ Conclusion

The refactoring is well-executed with:

  • Complete and consistent renames across all files
  • Correct SSZ size calculations with clear justifications
  • Appropriate TODO markers for future cleanup
  • No logic changes, just naming improvements

The PR successfully aligns the Anchor codebase with the SSV spec changes while maintaining backward compatibility and setting up a clear path for post-Boole cleanup.

--- | Branch: refactor/rename-validator-data-to-proposer

@shane-moore shane-moore force-pushed the refactor/rename-validator-data-to-proposer branch from be010db to 94ce686 Compare February 6, 2026 18:36
@shane-moore shane-moore added Boole v2.0.0 The release shipping the next network upgrade labels Feb 10, 2026
@dknopik
Copy link
Member

dknopik commented Feb 10, 2026

LGTM - but can we please split the SSV message length fix into another PR?

@shane-moore shane-moore force-pushed the refactor/rename-validator-data-to-proposer branch from 94ce686 to 4fe5fac Compare February 10, 2026 17:24
@shane-moore
Copy link
Member Author

LGTM - but can we please split the SSV message length fix into another PR?

@dknopik, certainly! cherry picked the commits over to #822

Comment on lines 340 to 342
// TODO(post-boole): remove `BEACON_ROLE_AGGREGATOR` and
// `BEACON_ROLE_SYNC_COMMITTEE_CONTRIBUTION` branches. Post-Boole, `ProposerConsensusData`
// is only used for `Proposer`.
Copy link
Member

Choose a reason for hiding this comment

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

is this related to #794 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

not directly related, that pr is for message validation whereas this comment is a clean up action item for after the fork hits mainnet. I put the comments in this PR since the comment references ProposerConsensusData, which is the rename done in this PR

we'll want to discuss at some point in a team call how we want to handle the clean up

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this code also validation?

Copy link
Member Author

Choose a reason for hiding this comment

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

removed comment from here in 5c04089 and added to pr794 in df697d7

diegomrsantos
diegomrsantos previously approved these changes Feb 10, 2026
Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify
Copy link

mergify bot commented Feb 11, 2026

Merge Queue Status

Rule: default


This pull request spent 14 minutes 49 seconds in the queue, including 12 minutes 21 seconds running CI.

Required conditions to merge

mergify bot added a commit that referenced this pull request Feb 11, 2026
@mergify mergify bot merged commit 5bad119 into sigp:unstable Feb 11, 2026
22 checks passed
@mergify mergify bot removed the queued label Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Boole ready-for-merge v2.0.0 The release shipping the next network upgrade

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants