Cmp prot#26
Closed
jonathanMweiss wants to merge 7 commits intoschnorrfrom
Closed
Conversation
protocols (FROST DKG and FROST Sign).
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for CMP (ECDSA) protocol alongside the existing FROST protocol in the TSS (Threshold Signature Scheme) implementation. The changes enable the system to handle multiple cryptographic protocols for key generation and signing.
- Adds CMP/ECDSA protocol support while maintaining backward compatibility with FROST
- Updates serialization from
encoding/gobto CBOR for improved cross-language compatibility - Extends round messages from 3 to 5 rounds to accommodate CMP protocol requirements
- Updates test data with new guardian configurations and key material
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| node/pkg/tss/util.go | Adds imports for CMP protocol types, extends signing rounds to 5, adds broadcast/unicast type checking for CMP messages |
| node/pkg/tss/parse.go | Updates function names and error messages for broadcast type checking |
| node/pkg/tss/cnfgs.go | Migrates from gob to CBOR encoding, adds ECDSA config storage and validation |
| node/pkg/tss/api.go | Updates API to require protocol type parameter for public key/address queries |
| node/pkg/tss/implementation.go | Adds ECDSA chain support, protocol selection logic, and updates engine initialization |
| node/pkg/tss/internal/cmd/dkg/server.go | Adds protocol parameter to DKG, supports merging existing TSS secrets, implements FROST validation |
| node/pkg/tss/internal/cmd/scripts_test.go | Updates test configuration to support protocol selection and loading existing secrets |
| node/pkg/tss/internal/cmd/lkg/local_key_generation.go | Migrates from gob to CBOR for marshaling |
| node/pkg/tss/implementation_test.go | Adds ECDSA signature test, updates all signing tasks with protocol type |
| node/go.mod, node/go.sum | Updates dependencies for multi-party-sig, tss-lib, and fxamacker/cbor |
| node/pkg/internal/testutils/testdata/tss5/*.json | Updates test guardian configurations with new keys and CBOR-encoded secrets |
Comments suppressed due to low confidence (1)
node/pkg/tss/util.go:273
- The
intToRoundfunction checks ifi > 2but the system now supports 5 rounds (round4Message and round5Message were added). This condition should be updated toi > 5or uselen(_intToRoundArr)to be more maintainable.
if i < 0 || i > 2 {
return ""
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| a.NoError(engine.Start(ctx)) | ||
| } | ||
|
|
||
| fmt.Println("msgHandler settup:") |
There was a problem hiding this comment.
Corrected spelling of 'settup' to 'setup'.
Suggested change
| fmt.Println("msgHandler settup:") | |
| fmt.Println("msgHandler setup:") |
Comment on lines
+172
to
+173
| lg.Info("loading existing GuardianStorage from file", zap.String("file", *existingPath)) | ||
| tmp, err := engine.NewGuardianStorageFromFile(*existingPath) |
There was a problem hiding this comment.
The log message uses *existingPath but the function parameter is prms.existingPath. The dereference operator is incorrect here since prms.existingPath is already a string. This should be zap.String(\"file\", prms.existingPath).
Suggested change
| lg.Info("loading existing GuardianStorage from file", zap.String("file", *existingPath)) | |
| tmp, err := engine.NewGuardianStorageFromFile(*existingPath) | |
| lg.Info("loading existing GuardianStorage from file", zap.String("file", prms.existingPath)) | |
| tmp, err := engine.NewGuardianStorageFromFile(prms.existingPath) |
yossigi
approved these changes
Nov 5, 2025
| @@ -340,7 +342,7 @@ func (p *Processor) handleInboundSignedVAAWithQuorum(m *gossipv1.SignedVAAWithQu | |||
|
|
|||
| var verificationPublic vaa.PublicKeys = keys | |||
| if v.Version == vaa.TSSVaaVersion { | |||
There was a problem hiding this comment.
Probably better to rename this constant now to FrostVaaVersion
| return nil, fmt.Errorf("failed to parse default port: %w", err) | ||
| } | ||
|
|
||
| port = p |
There was a problem hiding this comment.
nit: can assign directly port, err = strconv.Atoi(tss.DefaultPort)
| // attemptMergingTssSecretsToOld tries to load existing TSSSecrets from an old GuardianStorage file | ||
| // and merge them into the new TSSSecrets generated by the DKG process. | ||
| // This is useful to preserve existing keys for other protocols when only one protocol's keys are being generated. | ||
| func attemptMergingTssSecretsToOld(lg *zap.Logger, prms runParams, tssConfigs *party.TSSSecrets) { |
There was a problem hiding this comment.
this function could fail to merge, it should return some error to the caller
|
|
||
| func isBroadcastMsg(m common.ParsedMessage) bool { | ||
| // ensures content of a known broadcast type. | ||
| func isKnownBroadcastType(m common.ParsedMessage) bool { |
There was a problem hiding this comment.
nit: I think this func can be more simply isBroadcast, no?
| } | ||
| } | ||
|
|
||
| func isKnownUnicastType(m common.ParsedMessage) bool { |
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.
No description provided.