Skip to content

app/eth2wrap: fix mutation of pointer#4486

Merged
KaloyanTanev merged 4 commits intomainfrom
kalo/fix-dutiescache-pointer-overwrite
Apr 22, 2026
Merged

app/eth2wrap: fix mutation of pointer#4486
KaloyanTanev merged 4 commits intomainfrom
kalo/fix-dutiescache-pointer-overwrite

Conversation

@KaloyanTanev
Copy link
Copy Markdown
Collaborator

WIP

category: bug
ticket: none

@KaloyanTanev KaloyanTanev self-assigned this Apr 21, 2026
@KaloyanTanev KaloyanTanev marked this pull request as draft April 21, 2026 12:17
@jakubgs
Copy link
Copy Markdown

jakubgs commented Apr 21, 2026

I have tested this patch with 1.9.3 release of Charon and confirmed it resolves these failures:

{"level":"error","ts":"2026-04-21T08:30:00.031081603Z","caller":"validatorapi/router.go:1759","msg":"Validator api 5xx response: pubshare not found","stacktrace":"\tcore/validatorapi/validatorapi.go:1195 .AttesterDuties\n\tcore/validatorapi/router.go:739 .func1\n\tcore/validatorapi/router.go:411 .func31\n\tapp/app.go:1073 .func1","status_code":500,"message":"Internal server error","duration":"26.396401ms","pubkey":"0xa85e33ca0737c84dc947006226462a6eba8a96c41962248031980326ee1d119b5e64bb868c9c279ca9424799dda52698","vapi_endpoint":"attester_duties","topic":"vapi"}
{"level":"error","ts":"2026-04-21T08:30:01.268150598Z","caller":"validatorapi/router.go:1759","msg":"Validator api 5xx response: pubshare not found","stacktrace":"\tcore/validatorapi/validatorapi.go:1195 .AttesterDuties\n\tcore/validatorapi/router.go:739 .func1\n\tcore/validatorapi/router.go:411 .func31\n\tapp/app.go:1073 .func1","status_code":500,"message":"Internal server error","duration":"7.595396ms","pubkey":"0xa85e33ca0737c84dc947006226462a6eba8a96c41962248031980326ee1d119b5e64bb868c9c279ca9424799dda52698","vapi_endpoint":"attester_duties","topic":"vapi"}

Thanks for the fix.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.09%. Comparing base (97959c4) to head (06c68ab).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4486      +/-   ##
==========================================
- Coverage   56.56%   56.09%   -0.48%     
==========================================
  Files         245      245              
  Lines       32817    41956    +9139     
==========================================
+ Hits        18564    23536    +4972     
- Misses      11895    16057    +4162     
- Partials     2358     2363       +5     

☔ View full report in Codecov by Sentry.
📢 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.

@KaloyanTanev KaloyanTanev marked this pull request as ready for review April 21, 2026 15:36
@KaloyanTanev KaloyanTanev requested review from Copilot and pinebit April 21, 2026 15:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes DutiesCache request handling to avoid mutating caller-provided validator index slices and to more accurately determine cache hits using the set of previously-requested indices (including “no duty” cases).

Changes:

  • Clone vidxs in proposer/attester/sync duties cache methods to avoid mutating the caller’s slice.
  • Rework cache-hit/miss detection to use requestedIdxs (indices already queried from the beacon node) and compute a “missing indices” list.
  • Add shared scenario-based tests that validate beacon-node call patterns for proposer/attester/sync committee duties caching.

Reviewed changes

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

File Description
app/eth2wrap/cache.go Prevents caller slice mutation and refines cache completeness logic using requestedIdxs.
app/eth2wrap/cache_test.go Adds reusable test harness + scenarios across duty types to validate caching behavior and beacon call minimization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/eth2wrap/cache.go
Comment thread app/eth2wrap/cache.go
Comment thread app/eth2wrap/cache.go
Comment thread app/eth2wrap/cache.go Outdated
Comment thread app/eth2wrap/cache.go Outdated
Comment thread app/eth2wrap/cache.go Outdated
Comment thread app/eth2wrap/cache.go Outdated
@KaloyanTanev KaloyanTanev enabled auto-merge (squash) April 22, 2026 08:04
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
83.9% Duplication on New Code (required ≤ 15%)

See analysis details on SonarQube Cloud

@KaloyanTanev KaloyanTanev merged commit fd36139 into main Apr 22, 2026
8 of 10 checks passed
@KaloyanTanev KaloyanTanev deleted the kalo/fix-dutiescache-pointer-overwrite branch April 22, 2026 08:12
KaloyanTanev added a commit that referenced this pull request Apr 22, 2026
* Fix mutation of pointer

* Compare contents, not only lengths in the fast path; add more tests

* Address PR review comments

* Fix linter
KaloyanTanev added a commit that referenced this pull request Apr 22, 2026
* app/eth2wrap: fix mutation of pointer (#4486)

* Fix mutation of pointer

* Compare contents, not only lengths in the fast path; add more tests

* Address PR review comments

* Fix linter

* Version to rc
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.

4 participants