paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

Hash raw secp256k1 Public Keys instead of compressed ones #4216

Closed fubuloubu closed 4 years ago

fubuloubu commented 4 years ago

In #3861, secp256k1 keypairs were introduced. However, do to their construction, it wasn't possible to uniquely store a public key as 32 bytes without breaking the larger API, therefore the key types were hashed here and here in order to fit the keys securely into AccountId.

The choice of Public datatype for secp256k1 keys in Substrate was the 33 byte "compressed" version of the key, which is then hashed via blake2b to produce the 32 byte size. If instead, the 64-byte "raw" version of the key (without type identifiers) was used for this hash (and the hashing algorithm were configurable to keccak256, i.e. #3624) this would directly match the address format that Ethereum uses and allow more directly compatible usage of Substrate with Ethereum-based applications that already have significant state on them, and allow a better degree of inter-operation between the two (say, for Dothereum).

I think long term what would be most preferable would be to allow users to directly configure how addresses are displayed, and be able to specify their own conversion and encoding formats between public keys and addresses (making Substrate overall a more flexible tool), but this small change will help bridge that gap a lot better than how it is currently set up.

burdges commented 4 years ago

We support secp256k1 primarily to be Ethereum friendly, so yes being compatible sounds good. I presume the 64 bit form is Affine.

I'll note however that secp256k1 ecdsa signatures cannot be batch verified like ed25519 or sr25519, so we should probably make them cost slightly more whenever we start doing batch verification. I suppose keccak256 should cost more too until hardware venders add support. It'd be simpler if we can pick onetwo somewhat Ethereum/Bitcoin friendly secp256k1 ecdsa signature form and stick with it, regardless of where it uses blake2b or keccak256.

fubuloubu commented 4 years ago

Using keccak256 for AccountId would definitely be preferred, but I think assumes the use case is Ethereum-only (it could be Bitcoin?). Being configurable is best.