Skip to content
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

OCR3 keyring support #11956

Merged
merged 5 commits into from
Feb 12, 2024
Merged

OCR3 keyring support #11956

merged 5 commits into from
Feb 12, 2024

Conversation

samsondav
Copy link
Collaborator

Needed for LLO

@samsondav samsondav requested a review from a team as a code owner February 7, 2024 15:58
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 7, 2024

I see that you haven't updated any README files. Would it make sense to do so?

@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from feeefd0 to a114fee Compare February 7, 2024 16:01
@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from a114fee to 4bdf459 Compare February 7, 2024 16:02
@samsondav samsondav requested a review from jmank88 February 7, 2024 16:02
@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from 4bdf459 to 5f45497 Compare February 7, 2024 16:06
cedric-cordenier
cedric-cordenier reviewed Feb 9, 2024
View reviewed changes
core/services/keystore/keys/ocr2key/key_bundle.go
@@ -14,12 +14,20 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/store/models"
)

type OCR3SignerVerifier interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samsondav How does this get used in the code base? Not wild about the Sign3/Verify3 (and even less about the fact that this is located in ocr2key)

Also IIRC the Sign3/Verify3 isn't the interface that OCR3 expects is it? So do you need an adapter somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried renaming to ocr2plus but its 1,000s of changes and makes the salient points of this PR unreadable.

Not to mention we use OCR2 all over the codebase to refer to both OCR2 and OCR3. There is probably a larger ticket to clean up that naming.

jmank88
jmank88 previously approved these changes Feb 9, 2024
View reviewed changes
@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from ec8f5c7 to 8771ff0 Compare February 9, 2024 17:01
@samsondav samsondav requested review from a team as code owners February 9, 2024 17:40
@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from 16c6b08 to b178950 Compare February 9, 2024 18:37
@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from b178950 to 95a1956 Compare February 9, 2024 18:38
@samsondav
OCR3 keyring support
b6202db 
- Needed for LLO
- Only includes EVM implementation
@samsondav
Rename to ocr2plus key
454f003
@samsondav
Revert "Rename to ocr2plus key"
44f62b9 
This reverts commit 19a204cc81e0cda916293e0ba604834efb998ca4.
@samsondav
Add test
4c4b2e7
@samsondav samsondav requested a review from jmank88 February 9, 2024 18:41
@samsondav samsondav force-pushed the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch from 95a1956 to 4c4b2e7 Compare February 9, 2024 18:41
@samsondav samsondav requested a review from ferglor February 9, 2024 18:48
jmank88
jmank88 previously approved these changes Feb 9, 2024
View reviewed changes
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

@samsondav samsondav added this pull request to the merge queue Feb 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2024
@samsondav samsondav added this pull request to the merge queue Feb 12, 2024
Merged via the queue into develop with commit 0001494 Feb 12, 2024
93 checks passed
@samsondav samsondav deleted the MERC-3487-benchmark-test-mercury-parallel-compositions-keystore branch February 12, 2024 15:16
@samsondav samsondav mentioned this pull request Feb 16, 2024
Merged
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

@cedric-cordenier cedric-cordenier Awaiting requested review from cedric-cordenier

@ferglor ferglor Awaiting requested review from ferglor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samsondav @jmank88 @cedric-cordenier