Use limits framework instead o onchain config for Vault plugin ocr config#21405
Use limits framework instead o onchain config for Vault plugin ocr config#21405prashantkumar1982 wants to merge 6 commits intodevelopfrom
Conversation
… into vault_limits_for_ocr3
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
| if configProto.LimitsMaxKeyValueModifiedKeys == 0 { | ||
| configProto.LimitsMaxKeyValueModifiedKeys = defaultLimitsMaxKeyValueModifiedKeys | ||
| } | ||
| return &ReportingPluginConfig{ |
There was a problem hiding this comment.
@prashantkumar1982 Have you checked that these values behave in the same way as the plugin limits?
I think BatchSize definitely doesn't -- if this applies unevenly across nodes in the DON we'll get a non-determinism error, regardless of whether we're hitting the batchSize or not
There was a problem hiding this comment.
The others would normally lead the plugin to return an error to the user, so I think those are also not safe to change node by node since an inconsistency will lead to a loss of quorum
There was a problem hiding this comment.
As we discussed, these 2 are problematic:
- maxSecretsPerOwnerLimiter
- batchSize
Meaning these, if mismatched on a few nodes, could cause quorum failures.
we discussed that since changes will be based on limits framework, they will apply very uniformly and quickly on all nodes, but that still could be a few minutes, when potentially we fail quorum, thus slowing Vault DON, or maybe partial outage during that time.
Thus I moved these above 2 fields back to being read from onchain config.
All others are just max size changes, and should be ok to be changed safely via limits framework.
… into vault_limits_for_ocr3
|
2 participants
New limits were added for Vault here: smartcontractkit/chainlink-common#1877
This PR makes use of those. This helps us move away from depending on onchain config, which is very cumbersome to change.
AI generated this PR by this prompts: