Darc ECDSA by pm-duokey · Pull Request #2484 · dedis/cothority

pm-duokey · 2021-12-21T12:05:35Z

What this PR does

See commit message
This PR

🙅‍ Friendly checklist:

ineiti

Thanks for this PR. I think the most difficult bit is the naming :) You called it ECDSA, but I think it would be more appropriate to call it something like MPCECDSA, or TSMECDSA or such.

ineiti · 2021-12-21T15:20:21Z

 		return id.EvmContract.Address[:]
+	case 5:
+		buf := elliptic.Marshal(id.ECDSA.PublicKey.Curve, id.ECDSA.PublicKey.X, id.ECDSA.PublicKey.Y)
+		//TODO: add error check here?


What kind of error check can you do here? The GetPublicBytes doesn't return an error, so all you could do is panic.

ineiti · 2021-12-21T15:21:14Z

+	}
+}
+
+//TODO make calls to tsm  available


Please resolve all TODOs.

From what I understand, the Verify method will not call to the TSM, no?

ineiti · 2021-12-21T15:39:01Z

+//necessary function, needs to be refactored only supports elliptic.P256 curve
+//needs to be tested Unmarshal might not work


Suggested change

//necessary function, needs to be refactored only supports elliptic.P256 curve

//needs to be tested Unmarshal might not work

// Tries to convert a string into a sec256k1 point.

// TODO: needs to be tested Unmarshal might not work

ineiti · 2021-12-21T15:40:03Z

 	case 4:
 		return NewIdentityEvmContract(s.EvmContract)
+	case 5:
+		return NewIdentityECDSA(s.ECDSA.PublicKey)


Here and everywhere else: we should call the ECDSA identity MPCECDSA to indicate we're doing something special..

ineiti · 2021-12-21T15:42:26Z

+	id := make([]byte, hex.DecodedLen(len(in)))
+	_, err := hex.Decode(id, []byte(in))


Suggested change

id := make([]byte, hex.DecodedLen(len(in)))

_, err := hex.Decode(id, []byte(in))

buf, err := hex.DecodeString(in)

if err != nil {

return xerrors.Errorf("couldn't parse hex-string: %v", err)

}

ineiti · 2021-12-21T15:43:33Z

+	if err != nil {
+		return Identity{}, err
+	}


Suggested change

if err != nil {

return Identity{}, err

}

Treat the error as close as possible to the source. Else it is very confusing.

ineiti · 2021-12-21T15:45:09Z

 	return nil, errors.New("not yet implemented")
 }

+// new signer creates a signer only with a public key used to verify signatures


Suggested change

// new signer creates a signer only with a public key used to verify signatures

// NewSignerECDSA only takes a public key as the MPC is needed to sign data.

ineiti · 2021-12-21T15:46:53Z

+	}}
+}
+
+//TODO


One way to implement sign would be to give the necessary configuration when calling NewSignerECDSA, and then having Sign call the MPC backend. But that would introduce a dependency on the MPC code that we probably don't want to have in here.

Suggested change

//TODO

// Sign cannot be implemented, as only the MPC can sign a message.

ineiti · 2021-12-21T15:48:30Z

+	msg := []byte(`Hello World`)
+
+	//Signature from code example go-tsm-sdk corresponding to ecdsa public key example
+	signed, _ := hex.DecodeString("304402204f0b20a44efacec7b0514683233a79552026fe80e468078f6fed6cfe3f3e8a0402201eb12db7f6fe0828cafe8b0a032a37ff377b342799cfe77cfbac40c8ec1fa9e8")


Suggested change

signed, _ := hex.DecodeString("304402204f0b20a44efacec7b0514683233a79552026fe80e468078f6fed6cfe3f3e8a0402201eb12db7f6fe0828cafe8b0a032a37ff377b342799cfe77cfbac40c8ec1fa9e8")

signed, err := hex.DecodeString("304402204f0b20a44efacec7b0514683233a79552026fe80e468078f6fed6cfe3f3e8a0402201eb12db7f6fe0828cafe8b0a032a37ff377b342799cfe77cfbac40c8ec1fa9e8")

require.NoError(t, err)

ALWAYS do error checking! You will save yourself a lot of pain!

ineiti · 2021-12-21T15:48:45Z

+	var x, _ = new(big.Int).SetString("25613385885653880697990944418179706546134037329992108968315147853972798913688", 10)
+	var y, _ = new(big.Int).SetString("74946767262888349555270609195205284686604880870734462312238891495596941025713", 10)


Error checking needed.

ineiti · 2021-12-22T10:28:39Z

You can also use the Commit Suggestions in the comments to directly add the suggestions I made.

sonarqubecloud · 2021-12-29T11:22:42Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
8.4% Duplication

sonarqubecloud · 2022-03-04T09:47:41Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
3.9% Duplication

CLAassistant · 2026-04-06T22:06:23Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ineiti
❌ pm-duokey
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

pm-duokey added 5 commits December 17, 2021 03:01

new Darc ECDSA

0e73fd2

"implementation ECDSADarc"

596a5c7

ECDSADarc

5c21329

ECDSA signer

3632b20

merge temporary darc branch darc_temp

4a85f31

ineiti requested changes Dec 21, 2021

View reviewed changes

removing superflue import

ec6cdb2

pm-duokey and others added 4 commits December 22, 2021 06:59

ecdsa calypso/api_test.go error

b63fc6f

Add marshalling and a test

2d78896

added tsm to darcs, too, but signature is failing

fdfc3eb

Adding a TSM signer

b67548c

TestClient_Calypso_Simple

a06494a

		//necessary function, needs to be refactored only supports elliptic.P256 curve
		//needs to be tested Unmarshal might not work

		id := make([]byte, hex.DecodedLen(len(in)))
		_, err := hex.Decode(id, []byte(in))

	// new signer creates a signer only with a public key used to verify signatures
	// NewSignerECDSA only takes a public key as the MPC is needed to sign data.

+              	}}
+              }
+              //TODO

	//TODO
	// Sign cannot be implemented, as only the MPC can sign a message.

	signed, _ := hex.DecodeString("304402204f0b20a44efacec7b0514683233a79552026fe80e468078f6fed6cfe3f3e8a0402201eb12db7f6fe0828cafe8b0a032a37ff377b342799cfe77cfbac40c8ec1fa9e8")
	signed, err := hex.DecodeString("304402204f0b20a44efacec7b0514683233a79552026fe80e468078f6fed6cfe3f3e8a0402201eb12db7f6fe0828cafe8b0a032a37ff377b342799cfe77cfbac40c8ec1fa9e8")
	require.NoError(t, err)

		var x, _ = new(big.Int).SetString("25613385885653880697990944418179706546134037329992108968315147853972798913688", 10)
		var y, _ = new(big.Int).SetString("74946767262888349555270609195205284686604880870734462312238891495596941025713", 10)

Conversation

pm-duokey commented Dec 21, 2021

Uh oh!

ineiti left a comment

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!

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!

ineiti commented Dec 22, 2021

Uh oh!

sonarqubecloud Bot commented Dec 29, 2021

Uh oh!

sonarqubecloud Bot commented Mar 4, 2022

Uh oh!

CLAassistant commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CLAassistant commented Apr 6, 2026 •

edited

Loading