[DRAFT] Update signature verification for CommitteeVerifier and CRE Forwarder contracts#137
Open
faisal-chainlink wants to merge 17 commits into
Open
[DRAFT] Update signature verification for CommitteeVerifier and CRE Forwarder contracts#137faisal-chainlink wants to merge 17 commits into
faisal-chainlink wants to merge 17 commits into
Conversation
… CommitteeVerifier contracts
Soroban Contract Test Coverage93.08% line coverage — 17891 / 19221 lines hit
Per-Contract Breakdown
Full file-level coverage report |
faisal-chainlink
commented
Jul 2, 2026
| const SECP256K1_ORDER: [u8; 32] = [ | ||
| 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, | ||
| 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41, | ||
| ]; |
Collaborator
Author
There was a problem hiding this comment.
Removed because Soroban's crypto SDK applies this check automatically for ECDSA and it is not needed for ED25519 (no recovery).
faisal-chainlink
commented
Jul 2, 2026
… sig quorum trait
faisal-chainlink
requested review from
FelixFan1992 and
ilija42
and removed request for
ilija42
|
Code coverage report:
|
Integration Test Coverage (excl. Token Pool) |
Integration Test Coverage (Token Pool) |
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
1 participant
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.
This PR applies the following changes to
contracts/ccvs/committee-verifierandcontracts/cre:contracts/common/signature(renamed fromcontracts/common/verifier) to include basic traits around signature config storage and validation for both ECDSA and ED25519.SignatureConfigManagerto handle signature verification storage (both instance and persistent) with an associated typeDataKeyfor a data access (defined by the concrete contracts)#[contracttrait]from the signature verification traits to allow using generics in traits. This means all methods defined in the traits mentioned will be internal to the contract and not exposed by default. Any method (ex:#KeystoneForwarder.set_configmust be defined in the contract itself).SignatureSchemetrait with 2 implementing structs (Secp256k1EthAddressandEd25519) to serve as default implementations / helpers.SignatureQuoruma bit to remove any dependency on the actual implementation of CommitteeVerifier. This currently only works forSecp256k1EthAddressbut can be easily extended to use the existingEd25519as well. The primary difference in usage between CRE and CCV for quorum is not the signature itself (can be easily differentiated with the changes in this PR) but rather than way quorum is determined. A follow up PR would allow for branching based on quorum mode (SignatureVerificationConfig::FailurevsSignatureVerificationConfig::Threshold).