Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow key_format = { type = "cosmos-json" } #407

Merged

Conversation

michaelfig
Copy link
Contributor

@michaelfig michaelfig commented Sep 25, 2021

Fixes #352

NOTE: I haven't actually tested this with an actual KMS. Help would be appreciated.

I've verified importing a validator's private key with softsign and that the public key that is printed matches Cosmos's simd tendermint show-validator output.

This allows people to use:

key_format = { type = "cosmos-json" }

and then tmkms starts like this:

tmkms  start -c ~/.tmkms/tmkms.toml 2021-10-07T15:19:43.500379Z  INFO tmkms::commands::start: tmkms 0.11.0-pre starting up...
2021-10-07T15:19:43.502401Z  INFO tmkms::keyring: [keyring:softsign] added consensus Ed25519 key: {"@type":"/cosmos.crypto.ed25519.PubKey","key":"BSqtyJPQauUUXJbY3ltndfmCfyYziwtlbDDSaJGCEBc="}

Outputting that key format allows it to be used in simd tx staking create-validator --pubkey='{"@type":...}' ....

@michaelfig michaelfig marked this pull request as draft September 25, 2021 03:33
@tony-iqlusion
Copy link
Member

tony-iqlusion commented Oct 6, 2021

cosmrs v0.2.1 is out with serde support, but unfortunately due to upstream problems with tendermint-rs the docs don't build.

This type, however, (linking to an older build) now impls Deserialize and Serialize using the Cosmos JSON format:

https://docs.rs/cosmrs/0.1.0/cosmrs/crypto/struct.PublicKey.html

@michaelfig michaelfig force-pushed the mfig-352-key-type-cosmos-json branch 2 times, most recently from 535bfc8 to fed995c Compare October 7, 2021 04:06
@michaelfig
Copy link
Contributor Author

cosmrs v0.2.1 is out with serde support, but unfortunately due to upstream problems with tendermint-rs the docs don't build.

Thanks for this!

As an aside, I went through an interesting adventure doing the tendermint-rs 0.22 upgrade myself. And got it working. And then discovered you had already done it!

Oh, and this is the first-ever Rust code I've written. 😅

@michaelfig michaelfig marked this pull request as ready for review October 7, 2021 04:09
@michaelfig michaelfig force-pushed the mfig-352-key-type-cosmos-json branch from fed995c to 523a0f2 Compare October 7, 2021 05:02
src/keyring/format.rs Outdated Show resolved Hide resolved
@michaelfig michaelfig force-pushed the mfig-352-key-type-cosmos-json branch from 523a0f2 to d7ad3b6 Compare October 8, 2021 00:46
@Tomas-Eminger
Copy link
Contributor

I have tested this PR, but tmkms yubihsm keys list still show key which is unrelated to the actual key on HSM :( Using version tmkms v0.11.0-pre

src/keyring/format.rs Outdated Show resolved Hide resolved
@michaelfig michaelfig force-pushed the mfig-352-key-type-cosmos-json branch 2 times, most recently from 85cd547 to f81606a Compare October 10, 2021 00:06
@tony-iqlusion
Copy link
Member

@michaelfig if you rebase on top of #412 it should fix the security audit failure

@michaelfig michaelfig force-pushed the mfig-352-key-type-cosmos-json branch from f81606a to 85e5ccf Compare October 13, 2021 20:22
@tony-iqlusion
Copy link
Member

@Tomas-Eminger can you paste the two keys in question regarding your issue?

I will also check this locally. It would also be good to add an automated test.

@Tomas-Eminger
Copy link
Contributor

I still get

Listing keys in YubiHSM #0013202806:

  • 0x0001: [cons] {"@type":"/cosmos.crypto.ed25519.PubKey","key":"qIHw97tezljp7i4s09etmpD9Oao52RfJWdgjP3yvmbU="}
    label: "agoricvalconspub:2021-09-24T18:28:03Z"

Which is the same key as before from the source code from @michaelfig and the validator created with this key was unable to sign any transaction. Did you also changed the signing logic, or just the output of tmkms yubihsm keys list? Maybe we can chat on telegram (my contact is EmiT_RBF)

@tony-iqlusion
Copy link
Member

This PR only affects how keys are displayed: https://github.com/iqlusioninc/tmkms/pull/407/files#diff-f137d1d1045d9ae7502f39fe026c2e497a3f86a5ce2f73b49b6981141a7823bfR44

Note that iqlusion we are running this prerelease in production without issue: #410

@michaelfig
Copy link
Contributor Author

michaelfig commented Oct 17, 2021

I have verified that this PR successfully round-trips a softsign private key import:

$ ./target/debug/tmkms softsign import ~/.agoric/config/priv_validator_key.json ~/.tmkms/secrets/agoric-consensus.key
2021-10-17T03:22:45.260943Z  INFO tmkms::commands::softsign::import: Imported Ed25519 private key to /Users/michael/.tmkms/secrets/agoric-consensus.key
$ ./target/debug/tmkms start -c ~/.tmkms/tmkms.toml 
2021-10-17T03:23:08.818459Z  INFO tmkms::commands::start: tmkms 0.11.0-pre starting up...
2021-10-17T03:23:08.820535Z  INFO tmkms::keyring: [keyring:softsign] added consensus Ed25519 key: \
{"@type":"/cosmos.crypto.ed25519.PubKey","key":"DBUu7r7n0JSLwNi64q6HR7K1L9pqBBss9DaYHiaJgE4="}
[...]
$ agd tendermint show-validator
{"@type":"/cosmos.crypto.ed25519.PubKey","key":"DBUu7r7n0JSLwNi64q6HR7K1L9pqBBss9DaYHiaJgE4="}
$

@tony-iqlusion tony-iqlusion merged commit 687c163 into iqlusioninc:main Oct 18, 2021
@michaelfig michaelfig deleted the mfig-352-key-type-cosmos-json branch October 18, 2021 20:38
@tony-iqlusion tony-iqlusion mentioned this pull request Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update keys to the new Cosmos format
3 participants