Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Contracts pallet - Add support for recovering public key from elliptic curve signatures (ECDSA) #9609

Closed
PierreOssun opened this issue Aug 20, 2021 · 9 comments · Fixed by #9686
Labels
J0-enhancement An additional feature request. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@PierreOssun
Copy link

Can we add support for recovering public key from elliptic curve signatures (ECDSA) to !ink ?

We are trying to implement an ECDSA verifier in a contract. For this we are using recover function from the libsecp256k1 library developed by Parity and also used in Substrate.
The problem is that it uses a big list of constants that made cargo build fail due to its huge file size.
As a note : We could compile the contract using rustflags to increase memory and the optimized WASM file size was 1.3 Mb

In Ethereum, the ecrecover function is implemented as a precompiled contract and can be called from any contract.
A suggested solution is to implement the EDCSA recover pub key logic in contracts-pallet level, so that it can be used by !ink contracts via ext calls (as it was implemented for hashes).
What do you think ?

@cmichi
Copy link
Contributor

cmichi commented Aug 23, 2021

If we want to have this it would have to be implemented in the contracts pallet. I'm moving the issue to the appropriate repository.

cc @athei

@cmichi cmichi transferred this issue from use-ink/ink Aug 23, 2021
@github-actions github-actions bot added the J2-unconfirmed Issue might be valid, but it’s not yet known. label Aug 23, 2021
@PierreOssun PierreOssun changed the title Add support for recovering public key from elliptic curve signatures (ECDSA) Contracts pallet - Add support for recovering public key from elliptic curve signatures (ECDSA) Aug 24, 2021
@athei athei added J0-enhancement An additional feature request. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Aug 24, 2021
@athei
Copy link
Member

athei commented Aug 24, 2021

Makes sense to me. Contracts should not contain cryptographic code as it makes them way to big.

For this we are using recover function from the libsecp256k1 library developed by Parity and also used in Substrate.

A quick grep in substrate seems to suggest that it is in some Cargo.toml files but in no source file. Maybe an unused dependency. @gilescope what do you think?

@PierreOssun
Copy link
Author

Hello @athei
It is used here:

/// Verify a signature on a message. Returns true if the signature is good.

@bkchr
Copy link
Member

bkchr commented Aug 24, 2021

@athei you probably want to use this here:

fn secp256k1_ecdsa_recover(

And as @PierreOssun already pointed out, we use it in several places.

@athei
Copy link
Member

athei commented Aug 24, 2021

Yeah I messed up my grep. This is great. We can use existing client functions for that.

@PierreOssun
Copy link
Author

@athei
If you agree to implement it, Supercolony can take care of it and do a Pull Request

@athei
Copy link
Member

athei commented Aug 25, 2021

@athei
If you agree to implement it, Supercolony can take care of it and do a Pull Request

That would be great. But keep in mind that the biggest part of implementing this feature will be adding tests and benchmarks.

@atenjin
Copy link
Contributor

atenjin commented Aug 30, 2021

related to #8989, we think we not only need ecdsa recover, we also need more functions.

@athei athei removed the J2-unconfirmed Issue might be valid, but it’s not yet known. label Aug 30, 2021
@xgreenx
Copy link
Contributor

xgreenx commented Aug 30, 2021

related to #8989, we think we not only need ecdsa recover, we also need more functions.

Yea, but recovery is a feature only for ECDSA when verifying and signing must be implemented for all three supported key pairs.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

6 participants