Skip to content

core/validatorapi: optional committee index param - #4672

Open
KaloyanTanev wants to merge 3 commits into
gloasfrom
kalo/gloas-att-data-commidx
Open

core/validatorapi: optional committee index param#4672
KaloyanTanev wants to merge 3 commits into
gloasfrom
kalo/gloas-att-data-commidx

Conversation

@KaloyanTanev

@KaloyanTanev KaloyanTanev commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Make the deprecated committee_index query parameter optional on GET /eth/v1/validator/attestation_data, defaulting to 0 — post-gloas spec-compliant validator clients omit it and previously got a 400. Charon's own outgoing requests keep setting it for backwards compatibility with pre-electra forks, with comments noting post-electra beacon nodes ignore it.

category: feature
ticket: #4324

@KaloyanTanev KaloyanTanev self-assigned this Aug 27, 2026
@github-actions github-actions Bot added the branch-invalid PR raised against invalid branch. Not a main or release branch. label Aug 27, 2026
@KaloyanTanev
KaloyanTanev requested a lite review from Copilot August 27, 2026 13:32
@KaloyanTanev KaloyanTanev mentioned this pull request Aug 27, 2026
31 tasks

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes compatibility with post-gloas spec-compliant validator clients by making the deprecated committee_index query parameter optional on Charon’s GET /eth/v1/validator/attestation_data endpoint (defaulting to 0), preventing erroneous 400 responses when the parameter is omitted.

Changes:

  • Default committee_index to 0 when omitted on GET /eth/v1/validator/attestation_data.
  • Add a router test that asserts the endpoint succeeds without committee_index and passes CommitteeIndex=0 to the provider.
  • Add/extend comments documenting why Charon still sets committee_index in outbound BN requests for backwards compatibility.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
core/validatorapi/router.go Makes committee_index optional and defaults to 0 when absent for attestation data requests.
core/validatorapi/router_internal_test.go Adds coverage verifying the route works when committee_index is omitted and defaults to 0.
core/fetcher/fetcher.go Documents continued inclusion of committee index in BN attestation_data requests for backwards compatibility.
testutil/validatormock/attest.go Documents continued inclusion of committee index in mock VC → BN attestation_data requests for backwards compatibility.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread core/validatorapi/router_internal_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread core/fetcher/fetcher.go Outdated
Comment thread testutil/validatormock/attest.go Outdated
@KaloyanTanev
KaloyanTanev requested a lite review from Copilot August 27, 2026 13:43
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (gloas@c02d596). Learn more about missing BASE report.

Files with missing lines Patch % Lines
core/validatorapi/router.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             gloas    #4672   +/-   ##
========================================
  Coverage         ?   58.82%           
========================================
  Files            ?      246           
  Lines            ?    34887           
  Branches         ?        0           
========================================
  Hits             ?    20523           
  Misses           ?    11807           
  Partials         ?     2557           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch-invalid PR raised against invalid branch. Not a main or release branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants