Cmp protocol by jonathanMweiss · Pull Request #7 · XLabs/multi-party-sig

jonathanMweiss · 2025-09-01T12:30:58Z

this PR adjusts the cmp protocol from tauros to match the xlabs apis.

…he following: commit 0f99555 Author: Jonathan <jonathan.weiss1@mail.huji.ac.il> Date: Tue Aug 19 09:49:36 2025 +0300 pk marshalled as (pk.x<<1| parity) (#4) * pk marshalled as (pk.x<<1| parity) * PR fixes commit 3841be3 Author: Jonathan <jonathan.weiss1@mail.huji.ac.il> Date: Thu Aug 7 15:57:12 2025 +0300 Audit fixes (#2) * fx: exponent unmarshal panic with short data * ensure phii degree == threshold (as created in round 1) * fix: Secp256k1Scalar.Act normalizes the point first * change to challenge * changed deriveHashKeyContext * fix: tuple B in round2 should've included the index of each party * static marshal size * supporting new Unmarshal of exponentPoly in keygen * tweak * Unmarshal points/scalars removed without the context of their group commit fad331e Author: Jonathan <jonathan.weiss1@mail.huji.ac.il> Date: Fri Jun 27 08:48:51 2025 +0300 update tss-common commit 41fe152 Author: Jonathan <jonathan.weiss1@mail.huji.ac.il> Date: Wed Jun 25 12:51:28 2025 +0300 upgrading tss-common dependency + msg.ToParsed() fix commit 77c3534 Author: Jonathan Weiss <jonathan.weiss1@mail.huji.ac.il> Date: Thu Jun 19 14:21:53 2025 +0300 curve.Point clone method commit c1cbdc3 Author: Jonathan Weiss <jonathan.weiss1@mail.huji.ac.il> Date: Thu Jun 19 08:50:40 2025 +0300 upgrade tss-common dependency commit b006488 Author: Jonathan <jonathan.weiss1@mail.huji.ac.il> Date: Wed Jun 18 15:33:04 2025 +0300 Frost for ecrecover and tss-lib (#1) * changed gomod, upgrade go ver * removed-non-frost * not-yet working * fix: keygen finilize issue due to self not stored * adding proto messages * upgrade gomod * moving round to be an exported package * made the eth one public * added func: to contract Bytes * marshalling frostsig * README note change * removed unused hash * nits and removed unused packages * crypto readability fix + securly setting nonce=0 * readability and comments, moved unused func into tests * clearer challenge func name * refactor frost/types.go * refreshed common-tss version. added protocol type to msgs * dropping bnb-tss old partyID style * comment improve * fix: broadcast creation funcs names * fix test helper * upgrade tss-common * branch in round3 moved code * refactor

Copilot

Pull Request Overview

This PR migrates the multi-party signature protocol implementation from the tauros framework to the xlabs APIs. The migration involves updating import paths, modifying message structures to use protocol buffers, and adjusting protocol implementations to match the xlabs API conventions.

Updates all import paths from github.com/taurusgroup/multi-party-sig to github.com/xlabs/multi-party-sig
Migrates protocol message structures to use protocol buffer definitions with serialization helpers
Adapts FROST and CMP signing protocols to work with xlabs' round management and tracking systems

Reviewed Changes

Copilot reviewed 178 out of 181 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
protocols/frost/sign/types.go	Removes old signature types and verification logic
protocols/frost/sign/signature.go	Adds new EVM-compatible signature implementation with contract support
protocols/frost/sign/sign_test.go	Updates tests with xlabs imports and adds contract signature testing
protocols/frost/sign/sign.go	Updates imports and adds tracking ID support
protocols/frost/sign/round3.go	Migrates to protobuf messages and xlabs round handling
protocols/frost/sign/round2.go	Updates message handling and adds EVM-compatible challenge generation
protocols/frost/sign/round1.go	Updates imports and adds CanFinalize method
protocols/frost/sign/messages.go	Adds protobuf message creation and validation helpers
protocols/frost/sign/frost-signing.pb.go	Generated protobuf code for FROST signing messages
protocols/frost/keygen/*.go	Migrates keygen protocol to xlabs APIs with protobuf messages
protocols/frost/frost.go	Updates main FROST interface with xlabs imports
protocols/cmp/sign/*.go	Migrates CMP signing protocol to xlabs message structures
protocols/doerner/*.go	Removes deprecated Doerner protocol implementation
protocols/example/*.go	Removes example XOR protocol implementation

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-01T12:33:10Z

+// Verify checks if a signature equation actually holds.
+//
+// Note that m is the hash of a message, and not the message itself.
+func (sig Signature) Verify(public curve.Point, m []byte) error {


The Verify method returns an error instead of a boolean, which is inconsistent with the original implementation. This breaking change could confuse API consumers who expect a boolean return value from signature verification.

Copilot · 2025-09-01T12:33:11Z

+	// TODO: this assumes we've received Broadcast2 before we reach this point.
+	// (r.K[from] is nil and will FAIL to verify this proof). Consider how to handle this when Broadcast2 is not yet received.


The TODO comment indicates a potential race condition where message verification depends on broadcast message order. This could lead to verification failures if messages arrive out of order.

yossigi · 2025-09-07T09:47:54Z

+			return err
+		}
+
+		if len(b) > math.MaxUint16 {


This may be an issue because MarshalBinary could allocate more than 2^16B memory and only after that will it be filtered (e.g., this may allow someone to allocate enough memory to trigger a crash). It's better if MarshalBinary would return an error if it needs to allocate more than 2^16 B.

How do you enforce that on marshalBinary?

yossigi · 2025-09-07T09:54:55Z

+}
+
+func sum(sizes []int) int {
+	total := 0


Could this overflow?
(if len(sizes) is larger than 2^15)

I've added a check on the number of items , ensuring smaller than 2^16 list

yossigi · 2025-09-07T09:56:25Z

+	}
+
+	sizes := make([]int, numItems)
+	for i := 0; i < numItems; i++ {


is this correct? shouldn't it skip sizes[i] worth of data bytes?

No that's okay.
i first encode the size of all items, then put them one after the other on a single buffer.

yossigi · 2025-09-07T10:22:53Z

+		return false
+	}
+
+	if len(b.Di) == 0 || len(b.Di) > 33 {


should 33 be a global const? Also, is the check accurate?

I hadn't change this frost file. not sure why it shows it in the diff. locally it doesn't seem to show any git diff.

yossigi · 2025-09-07T10:33:45Z

-	body, ok := msg.Content.(*broadcast2)
-	if !ok || body == nil {
+	body, ok := msg.Content.(*Broadcast2)
+	if !ok || !body.ValidateBasic() {


ValidateBasic also checks whether the receiver is nil?

changed zk:marshal package api

Copilot

Pull Request Overview

Copilot reviewed 65 out of 66 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-23T08:08:25Z

 type command struct {
 	search bool
 	// This counter indicates the number of results that still need to be produced.
-	ctr *int64
-	// This channel is used to signal that the counter was modified
-	ctrChanged chan<- struct{}
-	// This is the index we evaluate our function at, when not searching
+	// also used to store results in search mode.
+	ctr *atomic.Int64


The comment on line 37 is confusing as it mentions 'also used to store results in search mode', but ctr is a counter, not a storage mechanism for results. The results are stored in the results field. Consider clarifying that ctr tracks the number of remaining results to produce.

Copilot · 2025-10-23T08:08:25Z

+
+	// keep searching while we still need results
+	for c.ctr.Load() > 0 {
+		// using 0 as dummy values, since search functions don't use the index


The comment 'using 0 as dummy values' should be 'using 0 as a dummy value' (singular) since only one value is being passed.

Suggested change

// using 0 as dummy values, since search functions don't use the index

// using 0 as a dummy value, since search functions don't use the index

Copilot · 2025-10-23T08:08:25Z

+		newChainKey = c.ChainKey
+	}
+	if len(newChainKey) != params.SecBytes {
+		return nil, fmt.Errorf("expecte %d bytes for chain key, found %d", params.SecBytes, len(newChainKey))


Corrected spelling of 'expecte' to 'expected'.

Suggested change

return nil, fmt.Errorf("expecte %d bytes for chain key, found %d", params.SecBytes, len(newChainKey))

return nil, fmt.Errorf("expected %d bytes for chain key, found %d", params.SecBytes, len(newChainKey))

…uct value

yossigi · 2025-11-05T12:50:48Z

+
+// get a signature in ethereum format
+func (sig *Signature) SigEthereum() ([]byte, error) {
+	IsOverHalfOrder := sig.S.IsOverHalfOrder() // s-values greater than secp256k1n/2 are considered invalid


this is only for FROST, right?

yossigi · 2025-11-05T13:27:24Z

@@ -0,0 +1,105 @@
+package marshal


is this file a variant of something?

yossigi · 2025-11-05T13:32:24Z

+		return
+	}
+
+	return


nit: no op return

go compilation demands this return, indicating there is value (see signature)

yossigi · 2025-11-05T20:11:05Z

+func makeBroadcast3(
+	// RID = RIDᵢ
+	RID types.RID,
+	C types.RID, //chainkey RID


nit: the comments should say what RID means, and what is chainkey

yossigi · 2025-11-05T20:13:27Z

+func (body *Broadcast3) unmarshalVssExpoly(crv curve.Curve, threshold int, vssConstant bool) (*polynomial.Exponent, error) {
+	vssSize := threshold + 1
+	if vssConstant {
+		// polynomial[0] is not sent in refresh mode ( due to it being the identity point ) as optimization.


is supporting resharing necessary? (or is it used in the protocol?)

* contractSig corrections * fix comments and updated gomod.

jonathanMweiss added 13 commits August 25, 2025 16:02

fx: imports are from xlabs

553352a

cmp dkg merge

9fc933f

cmp: remove presign package. perhaps added back in future

f7e98a6

marshal-zkenc

cbf5364

marshal methods without cbor

cbab0fc

sign round 1

669e8d2

zk marshal of affg + change to sch

eca7d30

round2 messages and marshalling

1b70b48

up to round 4 (not including)

a16bb6c

up to round 5

b838145

sign works

c6342c9

added todo

a7b89dd

jonathanMweiss requested review from Copilot and yossigi September 1, 2025 12:30

jonathanMweiss self-assigned this Sep 1, 2025

Copilot AI reviewed Sep 1, 2025

View reviewed changes

jonathanMweiss added 5 commits September 7, 2025 11:48

fix: unit-test fail due to missing function

ad293e6

removed code used by cmp/presign package

b1aef23

fix: sch commit marshalling

d2257ee

fx: unit-test panics due to bad key generation

6cf3d12

trackingID usage when creating cmp session

fff38e3

yossigi reviewed Sep 7, 2025

View reviewed changes

jonathanMweiss added 7 commits September 7, 2025 15:04

wip:zk:mod into saferith marshal

5979fb7

zkmod marshall without cbor

b8e82bf

improve readability of unmarshalling and marshalling primitives

eb0841c

safety-check on sum in marshal

f6616c3

changed zk:marshal package api

cmp protocol names

1c8b498

primitives read in marshal

1f3344f

fx

8340930

jonathanMweiss added 4 commits September 29, 2025 14:06

marshal change

0052851

Merge branch 'main' into cmp-prot

1ebf943

Merge branch 'main' into cmp-prot

e2c96f4

exportable config gen

d7355d6

jonathanMweiss force-pushed the cmp-prot branch from 34e030c to d7355d6 Compare October 8, 2025 11:22

jonathanMweiss added 6 commits October 21, 2025 08:56

cloning configs

4192081

secp256k1.Act(pnt) clones pnt

ac8511b

removed todo

a5c772b

cmp ProtocolSignRounds const

45350cd

translate signature code

0c5365a

fx: pool race

08ae059

jonathanMweiss requested review from Copilot and yossigi October 23, 2025 08:07

jonathanMweiss marked this pull request as ready for review October 23, 2025 08:07

Copilot AI reviewed Oct 23, 2025

View reviewed changes

jonathanMweiss added 3 commits October 27, 2025 09:07

cmp cnfg cache for pk computations

8ce22a5

added unit-test for marshalling the cmp config

e786727

fix: verifyMessage can be parallizeable now

0a5794b

jonathanMweiss force-pushed the cmp-prot branch 2 times, most recently from 312157e to bb53773 Compare November 3, 2025 12:16

update tss-common

f369c25

jonathanMweiss force-pushed the cmp-prot branch from bb53773 to f369c25 Compare November 3, 2025 13:20

jonathanMweiss added 2 commits November 5, 2025 08:41

changing the return value of cmp/sign from pointer of a struct to str…

e70a652

…uct value

update gomod for tss-common.ecdsa branch

cef4bcd

yossigi reviewed Nov 5, 2025

View reviewed changes

jonathanMweiss added 5 commits November 6, 2025 13:14

update gomod gosum

bf7821c

removed deadcode

ff0a531

go.mod tss-common update

03df0eb

contractSig corrections (#12)

e61f42f

* contractSig corrections * fix comments and updated gomod.

fix: ecSign mutated the secret

8065045

		// TODO: this assumes we've received Broadcast2 before we reach this point.
		// (r.K[from] is nil and will FAIL to verify this proof). Consider how to handle this when Broadcast2 is not yet received.

	// using 0 as dummy values, since search functions don't use the index
	// using 0 as a dummy value, since search functions don't use the index

	return nil, fmt.Errorf("expecte %d bytes for chain key, found %d", params.SecBytes, len(newChainKey))
	return nil, fmt.Errorf("expected %d bytes for chain key, found %d", params.SecBytes, len(newChainKey))

+              		return
+              	}
+              	return

Conversation

jonathanMweiss commented Sep 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathanMweiss Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jonathanMweiss Sep 7, 2025 •

edited

Loading