-
Notifications
You must be signed in to change notification settings - Fork 134
*: add duties cache #4227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*: add duties cache #4227
Conversation
697631c to
b5f3d01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Implements a duties cache to reduce duplicate beacon-node duty requests (scheduler + VC) by caching proposer/attester/sync-committee duties and routing call sites through cached accessors.
Changes:
- Added
DutiesCachetoapp/eth2wrapand wired it into the app’s eth2 client viaSetDutiesCacheand reorg invalidation. - Updated scheduler, inclusion tracker, and validator API component to use cached duty fetch paths.
- Extended beaconmock and client wrappers (multi/lazy/http adapter) plus tests/mocks to support duties-cache interfaces.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| testutil/integration/simnet_test.go | Removes Teku simnet integration bits and related test scaffolding. |
| testutil/integration/helpers_test.go | Removes externalIP helper previously used by Teku simnet wiring. |
| testutil/beaconmock/options.go | Adds default/option wiring for cached duty funcs in beaconmock. |
| testutil/beaconmock/beaconmock_fuzz.go | Adds cached duty funcs to fuzz option. |
| testutil/beaconmock/beaconmock.go | Extends mock client with duties-cache funcs and UpdateCacheIndices. |
| core/validatorapi/validatorapi_test.go | Updates tests to use cached duties funcs. |
| core/validatorapi/validatorapi.go | Routes duties endpoints through cached duty calls and rewrites pubkey mapping. |
| core/tracker/inclusion.go | Uses cached attester duties for inclusion tracking. |
| core/scheduler/scheduler.go | Switches duty resolution to cached duty calls. |
| core/fetcher/fetcher_test.go | Minor formatting-only adjustments. |
| core/fetcher/fetcher.go | Minor formatting-only adjustments. |
| app/vmock.go | Wires duties cache into vmock eth2 provider. |
| app/sse/listener_internal_test.go | Uses require.Len for map length assertion. |
| app/sse/listener.go | Groups handler func types and minor formatting. |
| app/featureset/config.go | Minor formatting-only adjustments. |
| app/eth2wrap/valcache.go | Removes old validator cache file (moved into new cache module). |
| app/eth2wrap/synthproposer_test.go | Updates tests to provide cached duties funcs on mocks. |
| app/eth2wrap/synthproposer.go | Extends synth wrapper provider interface to include cached duties provider. |
| app/eth2wrap/multi_test.go | Adds coverage for new duties-cache forwarding methods. |
| app/eth2wrap/multi.go | Adds multi-client forwarding for duties-cache APIs. |
| app/eth2wrap/mocks/client.go | Regenerates mock client to include duties-cache APIs. |
| app/eth2wrap/lazy_test.go | Adds tests for lazy wrapper duties-cache APIs. |
| app/eth2wrap/lazy.go | Adds lazy wrapper support for duties-cache APIs. |
| app/eth2wrap/httpwrap.go | Adds duties-cache hooks to the HTTP adapter wrapper. |
| app/eth2wrap/genwrap/genwrap.go | Updates generated interface template to include duties-cache APIs. |
| app/eth2wrap/eth2wrap_gen.go | Updates generated client interface to include duties-cache APIs. |
| app/eth2wrap/cache_test.go | Adds initial tests for duties cache behavior (proposer only). |
| app/eth2wrap/cache.go | Introduces new cache module containing validator cache + duties cache implementation. |
| app/app.go | Wires duties cache into core workflow and hooks reorg invalidation + index updates. |
Comments suppressed due to low confidence (1)
app/eth2wrap/cache_test.go:295
- The duties cache test only covers proposer duties with a single (epoch, indices) shape. Given the new behavior, it would be valuable to add coverage for: (1) different index subsets in the same epoch (to ensure correct keying/filtering), (2) attester/sync duties cache paths, and (3) chain reorg invalidation (InvalidateCache).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4227 +/- ##
==========================================
- Coverage 56.98% 56.68% -0.31%
==========================================
Files 237 237
Lines 30997 31263 +266
==========================================
+ Hits 17664 17721 +57
- Misses 11084 11280 +196
- Partials 2249 2262 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: kalo <24719519+KaloyanTanev@users.noreply.github.com>
a343672 to
e879e9c
Compare
|
Previously we've recognised a pattern in certain validator clients - making too many obsolete requests to Charon for checking active validators. This meant that Charon was sending the same amount of requests to the beacon node for such a heavy request. The assumption that the beacon node will have this data cached and serve us quickly was not always true. Because of that we have added active validators cache. With a similar logic, some validator clients are sending requests too often for next slot's duties. This creates a lot of requests to the beacon node which can be heavy in cases of 1000+ validator keys. Moreover, Charon is also sending such requests to the beacon node, so the amount of duties requests is at least doubled each epoch. We can prevent that with a similar caching strategy. It has been implemented for attester, proposer and sync committee duties. Another small change has been made - removed Teku VC testing from integration tests. Back in the days it was included there as some sanity check with a real and not a mock VC. However, nowadays, we are doing much more complicated and closer to real scenarios of actual VCs in Kurtosis. category: feature ticket: #4009
1. Previously we have tried to parse each proposer duty's key, but the proposer duty is the only duty request that don't filter on validator indices and rather returns all. In the previous implementation we had `continue`, what I've added now as well. 2. Add a bit more debug logging on reorg 3. Fix the cache metrics labels category: bug ticket: #4227



Previously we've recognised a pattern in certain validator clients - making too many obsolete requests to Charon for checking active validators. This meant that Charon was sending the same amount of requests to the beacon node for such a heavy request. The assumption that the beacon node will have this data cached and serve us quickly was not always true. Because of that we have added active validators cache.
With a similar logic, some validator clients are sending requests too often for next slot's duties. This creates a lot of requests to the beacon node which can be heavy in cases of 1000+ validator keys. Moreover, Charon is also sending such requests to the beacon node, so the amount of duties requests is at least doubled each epoch. We can prevent that with a similar caching strategy. It has been implemented for attester, proposer and sync committee duties.
Another small change has been made - removed Teku VC testing from integration tests. Back in the days it was included there as some sanity check with a real and not a mock VC. However, nowadays, we are doing much more complicated and closer to real scenarios of actual VCs in Kurtosis.
category: feature
ticket: #4009