Add support for secp256k1 consensus keys

[mkaczanowski]

Closes: #1364

This adds the support for parsing secp256k1 consensus key from priv_validator_key.json.

A while ago I've merged support for secp256k1 consensus key type to tmkms mainly for band protocol network. It came out that the functionality to import the private key from tendermint format to tmkms doesn't work due to lack of support in tendermint-rs.

• Referenced an issue explaining the need for the change
• Enable support for secp256k1 consensus key iqlusioninc/tmkms#761
• Updated all relevant documentation in docs
• to the best of my knowledge
• Updated all code comments where relevant
• Wrote tests
• Added entry in .changelog/

[codecov-commenter]

Codecov Report

Merging #1358 (f9b0a55) into main (4d81b67) will increase coverage by 0.5%.
Report is 1 commits behind head on main.
The diff coverage is 44.7%.

❗ Current head f9b0a55 differs from pull request most recent head 7ff521d. Consider uploading reports for the commit 7ff521d to get more accurate results

@@           Coverage Diff           @@
##            main   #1358     +/-   ##
=======================================
+ Coverage   59.8%   60.4%   +0.5%     
=======================================
  Files        274     275      +1     
  Lines      27254   27062    -192     
=======================================
+ Hits       16314   16361     +47     
+ Misses     10940   10701    -239     

Files Changed	Coverage Δ
tendermint/src/abci/response/process_proposal.rs	0.0% <0.0%> (ø)
...dermint/src/abci/response/verify_vote_extension.rs	0.0% <0.0%> (ø)
tendermint/src/abci/types.rs	0.2% <0.0%> (-0.1%)	⬇️
tools/proto-compiler/src/constants.rs	0.0% <ø> (ø)
tendermint/src/private_key.rs	45.0% <50.0%> (+19.0%)	⬆️
config/tests/mod.rs	100.0% <100.0%> (ø)
tendermint/src/public_key.rs	79.5% <100.0%> (+3.1%)	⬆️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[romac]

Thanks!

[mzabaluev]

Feature-guarded enum variants bring fragility. I'd rather make the whole thing non-optional, secp256k1 support does not add much in terms of dependencies. But the enum is #[non_exhaustive] so maybe it's OK for now.

[mkaczanowski]

since the secp256k1 is marked almost everywhere as feature I fell with the same approach. But I agree that conditional enums are not as great. Though rust is pretty good with handling conditional compilation via features.

Therefore I hold no strong opinion here

[mzabaluev]

The problem with conditional enum variants is that they make match expressions fail to compile dependent on features. #[non_exhaustive] does a lot to make it less fragile, as you can't have a match that would be exhaustive without certain features and then fail to compile if the features are enabled. But anyone wanting to match on the conditionally-enabled variant would have to enable the corresponding feature. This, perhaps, is no worse than other conditionally defined APIs, so I'm willing to let it go.

[tarcieri]

FYI, I believe the original motivation for feature-gating was at the time libsecp256k1 was being used and it didn’t work with WASM.

We’ve since moved to the pure Rust k256 crate so that isn’t an issue.

I think it would probably make more sense to feature gate all the cryptographic functionality under a single feature rather than just secp256k1, that way people who just want e.g. tendermint-rpc for an API client aren’t pulling in a bunch of crypto deps they aren’t using.