Follow-ups of #562 - Githubissues

MystenLabs / fastcrypto

Common cryptographic library used in software at Mysten Labs.

Apache License 2.0

244 stars 138 forks source link

Follow-ups of #562 #34

Open huitseeker opened 2 years ago

huitseeker commented 2 years ago

Approving this, and we need a few extra upcoming PRs

clear api in our codebase for signing using a) randomized signatures b) deterministic RFC6979, default should be the randomized (just for consistency with most implementations out there and the main ECDSA standard).
an extra test for randomized sigs (sign with implA, verify with implB and vice versa)
comparison benchmark and test for ecrecover as well.
ECDSA-NM (which is already in progress)

Originally posted by @kchalkias in https://github.com/MystenLabs/narwhal/pull/562#pullrequestreview-1048514745

huitseeker commented 2 years ago

I am weary about allowing standard randomized ECDSA at all, for reasons of malleability, in the current state of our commitments in data structures (in particular, we don't have a commitment on the signature data).

kchalkias commented 2 years ago

We're not changing our malleability story, both randomized and deterministic versions should be with reduced s values. As mentioned elsewhere, the blockchain itself should only specify signature validation rules. SignAMessage api is here mainly for api completeness. Thus, similarly to most ECDSA libs, this crate should support conventional randomized ECDSA libs as well (I'd mention hybrid too to cover all options - but this is not a requirement atm).

I'm fine if to satisfy our sign trait the team prefers the deterministic version as the default one.