near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.32k stars 619 forks source link

Signature API for Ed25519 and ECDSA-secp256k1 signatures doesn't match #2835

Open abacabadabacaba opened 4 years ago

abacabadabacaba commented 4 years ago

The parameter data to SecretKey::sign() and Signature::verify() is supposed to hold the message being signed. However, in the current implementation, its meaning depends on signature type:

As a result, it is not possible to use this API properly if support for both signature types is desired. Currently, the code that signs transactions hashes them one extra time when using Ed25519 (which is not necessary), and the code that signs approvals only uses Ed25519 signatures (and will crash if we try to make it work with ECDSA-secp256k1 signatures, because approvals' length is not 32 bytes).

We need to change the API to interpret data as the message itself for both signature types (or get rid of ECDSA-secp256k1 signatures).

ilblackdragon commented 4 years ago

I don't mind removing support of secp256k1 (or of ed25519 if we want use secp256k1 to simplify verification in Ethereum) for specifically ValidatorSigner / block & approval signatures. I don't think we need to have 2 possible key types there.

For transactions it still make sense IMO. And may be even adding secp256p1 to add support for iOS secure enclaves.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.