fix(keeper): thread context into GetUpkeep and GetConfig RPC calls#21490
fix(keeper): thread context into GetUpkeep and GetConfig RPC calls#21490Aboudjem wants to merge 1 commit intosmartcontractkit:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Risk Rating: LOW (threads existing context.Context through to on-chain RPC calls; behavior change is limited to cancellation/shutdown paths).
Fixes slow shutdown/job stop behavior in the keeper registry synchronizer by ensuring GetUpkeep and GetConfig respect caller cancellation instead of waiting for RPC timeouts.
Changes:
- Pass
ctxviabind.CallOpts{Context: ctx}intoGetUpkeepcalls during upkeep sync. - Pass
ctxviabind.CallOpts{Context: ctx}intoGetConfigcalls when building a registry from chain state.
|
|
||
| func (rs *RegistrySynchronizer) syncUpkeep(ctx context.Context, getter upkeepGetter, registry Registry, upkeepID *sqlutil.Big) error { | ||
| upkeep, err := getter.GetUpkeep(nil, upkeepID.ToInt()) | ||
| upkeep, err := getter.GetUpkeep(&bind.CallOpts{Context: ctx}, upkeepID.ToInt()) |
There was a problem hiding this comment.
Add a regression unit test that asserts syncUpkeep threads the provided ctx into the *bind.CallOpts passed to GetUpkeep (e.g., a fake upkeepGetter that captures opts.Context). This prevents future regressions back to nil/background contexts which would reintroduce slow shutdown behavior.
| contractAddress := rs.job.KeeperSpec.ContractAddress | ||
|
|
||
| registryConfig, err := rs.registryWrapper.GetConfig(nil) | ||
| registryConfig, err := rs.registryWrapper.GetConfig(&bind.CallOpts{Context: ctx}) |
There was a problem hiding this comment.
Consider adding test coverage that newRegistryFromChain passes the caller ctx into the bind.CallOpts used by GetConfig (and thus GetKeeperList). One approach is a small fake bind.ContractBackend that records the ctx it receives in CallContract, and wiring it into a generated registry wrapper in the test.
Fixes #21489.
Noticed during a slow shutdown investigation that syncUpkeep and newRegistryFromChain both pass nil CallOpts to their on-chain calls even though they receive a context from the caller. The GetUpkeep and GetConfig RPCs end up ignoring cancellation, so they block until RPC timeout during shutdown instead of stopping when the context is done.
The fix just wires the existing ctx through bind.CallOpts on both call sites. No behavior change under normal operation - the context only matters on cancellation paths.