CRE-4087: Fix to DKG config generation#22319
Merged
cedric-cordenier merged 1 commit intodevelopfrom May 7, 2026
Merged
Conversation
product-security-plaid-production
Bot
requested review from
DylanTinianov and
giogam
Contributor
|
👋 cedric-cordenier, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
Contributor
|
✅ No conflicts with other open PRs targeting |
Contributor
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH — This PR changes how DKG reporting plugin config bytes are derived and propagated into OCR3.1 contract configuration, which can directly affect on-chain/off-chain compatibility during deployments.
This PR aims to fix DKG config generation by removing the separate dkgocrtypes.ReportingPluginConfig plumbing and instead marshaling the DKG plugin config from V3_1OracleConfig.DKGOffchainConfig.
Changes:
- Removed passing
dkgocrtypes.ReportingPluginConfigthrough DKG config generation and contract configuration operations; DKG config now marshals fromcfg.DKGOffchainConfig. - Simplified
DKGOffchainConfigto be a YAML/JSON-friendly struct (string public keys) and adjusted call sites accordingly. - Updated tests and deployment changesets to use the new
DKGOffchainConfigflow.
Scrupulous human review recommended:
deployment/cre/ocr3/ocr3_1/oracle_config.go:DKGOffchainConfig.Marshal()semantics aroundPreviousInstanceIDand correctness of producedReportingPluginConfigbytes.deployment/cre/ocr3/ocr3_1/config_dkg.go: exportedGenerateDKGConfigFromNodessafety/validation whenDKGOffchainConfigis absent (panic risk).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| deployment/cre/ocr3/ocr3_1/registry.go | Stops threading a separate DKG plugin config; uses oracleOffchainConfig.DKGOffchainConfig and adds a required-non-nil check. |
| deployment/cre/ocr3/ocr3_1/oracle_config.go | Refactors DKGOffchainConfig to remove embedded ReportingPluginConfig and rely on marshal-time conversion. |
| deployment/cre/ocr3/ocr3_1/config_test.go | Updates DKG config generation test to match new GenerateDKGConfigFromNodes signature and config shape. |
| deployment/cre/ocr3/ocr3_1/config_dkg.go | Updates DKG config generation API to marshal from cfg.DKGOffchainConfig instead of an explicit dkgCfg argument. |
| deployment/cre/ocr3/ocr3_1/changeset/operations/contracts/configure_vault_dkg.go | Populates OracleConfig.DKGOffchainConfig from DON inputs and removes explicit plugin-config creation/validation. |
| deployment/cre/ocr3/ocr3_1/changeset/operations/contracts/configure_dkg.go | Removes ReportingPluginConfig from operation inputs and relies on OracleConfig.DKGOffchainConfig. |
Comment on lines
+50
to
55
| func GenerateDKGConfigFromNodes(cfg V3_1OracleConfig, nodes []deployment.Node, registryChainSel uint64, secrets ocr.OCRSecrets, extraSignerFamilies []string) (ocr3.OCR2OracleConfig, error) { | ||
| nca := ocr3.MakeNodeKeysSlice(nodes, registryChainSel, extraSignerFamilies) | ||
| cfgBytes, err := dkgCfg.MarshalBinary() | ||
| cfgBytes, err := cfg.DKGOffchainConfig.Marshal() | ||
| if err != nil { | ||
| return ocr3.OCR2OracleConfig{}, fmt.Errorf("failed to marshal ReportingPluginConfig: %w", err) | ||
| } |
|
|
cedric-cordenier
force-pushed
the
CRE-4087-dkg-fix
branch
2 times, most recently
from
998dd5f to
00a8587
Compare
cedric-cordenier
force-pushed
the
CRE-4087-dkg-fix
branch
from
00a8587 to
27f8b6b
Compare
|
prashantkumar1982
approved these changes
May 6, 2026
DylanTinianov
approved these changes
May 6, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requires
Supports