Skip to content

Enable DON2DON flow for Vault service's GetSecrets() method #18514

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.

Sign up for GitHub

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

Jump to bottom
Merged
DeividasK merged 36 commits into from
Jul 10, 2025
Merged

Enable DON2DON flow for Vault service's GetSecrets() method #18514

DeividasK merged 36 commits into from
Jul 10, 2025

Conversation

@prashantkumar1982
Copy link
Contributor

@prashantkumar1982 prashantkumar1982 commented Jul 7, 2025 •
edited
Loading

This change enables Vault Don2DON flow, by making sure all Vault GetSecrets() requests from Workflow Nodes are exactly same.
This is done by:

  • Each workflow node, when requesting for GetSecrets(), passes in all the workflow DON nodes' public encryption keys, in a standard sorted way.
    • Thus, the requests are all identical, which is a pre-requisite for current DON2DON layer.
  • CapabilityRegistry interface now exports a new method allowing to fetch Nodes by peerID, and the Node contains encyption keys as response.
  • In SecretsFetcher, we fetch the list of Workflow DON nodes, and fetch each Node individually. Then combine the encryption keys from them.
  • The integration tests now ensure this logic is tested, by specifically expecting a sorted list of requests to be sent back as response.

https://smartcontract-it.atlassian.net/browse/PRIV-92

@prashantkumar1982 prashantkumar1982 changed the title SecretsFetcher support for fetching public keys of all Nodes in a DON Fix DON2DON flow for Vault service's GetSecrets() method Jul 8, 2025
@prashantkumar1982 prashantkumar1982 changed the title Fix DON2DON flow for Vault service's GetSecrets() method Enable DON2DON flow for Vault service's GetSecrets() method Jul 8, 2025
@prashantkumar1982 prashantkumar1982 marked this pull request as ready for review July 8, 2025 01:53
@prashantkumar1982 prashantkumar1982 requested review from a team as code owners July 8, 2025 01:53
DeividasK
DeividasK reviewed Jul 8, 2025
View reviewed changes
DeividasK
DeividasK reviewed Jul 8, 2025
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2025

Flakeguard Summary

Ran new or updated tests between develop and fe5072a (vault-don2don).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

3 Results
Name Pass Ratio Panicked? Timed Out? Race? Runs Successes Failures Skips Package Package Panicked? Avg Duration Code Owners
TestEngine_WASMBinary_Simple 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/v2 false 4.153333333s @smartcontractkit/keystone
TestEngine_WASMBinary_With_Config 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/v2 false 96.666666ms @smartcontractkit/keystone
TestSecretsFetcher_Integration 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/v2 false 93.333333ms @smartcontractkit/keystone

Artifacts

For detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2025

Flakeguard Summary

Ran new or updated tests between develop and c04d8ff (vault-don2don).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

3 Results
Name Pass Ratio Panicked? Timed Out? Race? Runs Successes Failures Skips Package Package Panicked? Avg Duration Code Owners
TestEngine_WASMBinary_Simple 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/v2 false 3.89s @smartcontractkit/keystone
TestEngine_WASMBinary_With_Config 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/v2 false 90ms @smartcontractkit/keystone
TestSecretsFetcher_Integration 0% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/v2 false 90ms @smartcontractkit/keystone

Artifacts

For detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json.

@jmank88
Copy link
Contributor

jmank88 commented Jul 10, 2025

Please link dependent PRs to one another from their descriptions:

This was hard to find, and the breaking signature change is blocking others from bumping common.

DeividasK
DeividasK previously approved these changes Jul 10, 2025
View reviewed changes
@cl-sonarqube-production
Copy link

cl-sonarqube-production bot commented Jul 10, 2025

Quality Gate passed Quality Gate passed

Issues
2 New issues
2 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

@DeividasK DeividasK added this pull request to the merge queue Jul 10, 2025
Merged via the queue into develop with commit 9af582e Jul 10, 2025
188 of 189 checks passed
@DeividasK DeividasK deleted the vault-don2don branch July 10, 2025 16:26
This was referenced Jul 9, 2025
Draft
Draft
Draft
Draft
cawthorne pushed a commit that referenced this pull request Jul 23, 2025
@prashantkumar1982 @MStreet3 @cawthorne
Enable DON2DON flow for Vault service's GetSecrets() method (#18514)
945d09a 
* SecretsFetcher support for fetching public keys of all Nodes in a DON

* Logic update for DON members

* add new mock file

* fix unittest

* Changeset and lint

* Integration Test support

* Lint Errors

* nit var rename and formatting

* mockery fix

* Add a tag to changeset

* review feedback

* chore(multi): bumps chainlink-common

* fix(capabilities/fakes): updates use of consensus capability

* fix: re-bump common

* Refactor to get all nodes in a don from capability registry

* test fixes

* test fix

* chore: re-bump common

* fix(capabilities/fakes): modify report request

* lint

* fix: make generate

* get latest chainlink-common change

* Pull breaking change fix from another PR(18558) to unblock test failures

* chore: bump cre-sdk-go

* chore: bump sdk

* bump chainlink-common to latest commit on branch

---------

Co-authored-by: Michael Street <5597260+MStreet3@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Jul 25, 2025
Merged
@github-actions github-actions bot mentioned this pull request Aug 21, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmank88 jmank88 jmank88 approved these changes

@asoliman92 asoliman92 asoliman92 approved these changes

@cedric-cordenier cedric-cordenier cedric-cordenier left review comments

@winder winder Awaiting requested review from winder winder is a code owner automatically assigned from smartcontractkit/ccip-offchain

@matYang matYang Awaiting requested review from matYang matYang is a code owner automatically assigned from smartcontractkit/ccip-offchain

@makramkd makramkd Awaiting requested review from makramkd makramkd is a code owner automatically assigned from smartcontractkit/ccip-offchain

@AmrMohamedRezk AmrMohamedRezk Awaiting requested review from AmrMohamedRezk AmrMohamedRezk is a code owner automatically assigned from smartcontractkit/ccip-offchain

@dimkouv dimkouv Awaiting requested review from dimkouv dimkouv is a code owner automatically assigned from smartcontractkit/ccip-offchain

@mateusz-sekara mateusz-sekara Awaiting requested review from mateusz-sekara mateusz-sekara is a code owner automatically assigned from smartcontractkit/ccip-offchain

@Atrax1 Atrax1 Awaiting requested review from Atrax1 Atrax1 is a code owner automatically assigned from smartcontractkit/foundations

@ecPablo ecPablo Awaiting requested review from ecPablo

+1 more reviewer

@DeividasK DeividasK DeividasK approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@prashantkumar1982 @jmank88 @asoliman92 @DeividasK @cedric-cordenier @MStreet3