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

sc_cli key insert gives wrong name to _gran_ key file created in local keystorage (BREAKS blocks finalization) #9888

Closed
agryaznov opened this issue Sep 28, 2021 · 4 comments · Fixed by #9909
Labels
J2-unconfirmed Issue might be valid, but it’s not yet known. Z7-question Issue is a question. Closer should answer.

Comments

@agryaznov
Copy link
Contributor

agryaznov commented Sep 28, 2021

(found while passing this Tutorial)

Summary

key insert gives a wrong name to file containing gran (ed25519) key, which effectively leads to broken GRANDPA finalization in node

Details

Performing the key insert command operation with pre-generated Alice' key

# insert Alice's sr25519 aura key into storage
$ ./target/release/node-template key insert --base-path /tmp/node01 --chain local --key-type aura --suri "clip organ olive upper oak void inject side suit toilet stick narrow"

# insert Alice's ed25519 gran key into storage
$ ./target/release/node-template key insert --base-path /tmp/node01 --chain local --key-type gran --suri "clip organ olive upper oak void inject side suit toilet stick narrow"

leads to creating these two files :

$ find /tmp/node01/chains/local_testnet/keystore -type f -printf "%f\n"
617572619effc1668ca381c242885516ec9fa2b19c67b6684c02a8a3237b6862e5c8cd7e
6772616e9effc1668ca381c242885516ec9fa2b19c67b6684c02a8a3237b6862e5c8cd7e

one of which is possibly wrong-named, as in their names:

  • 61757261 encodes aura key type
  • 6772616e encodes gran key type

and the other 9effc1668ca381c242885516ec9fa2b19c67b6684c02a8a3237b6862e5c8cd7e part derives from public key, which should be different for aura (sr25519) and gran (ed25519), but it turns out to be just the same for both

how it (supposedly) should work

if we just insert the same keys using JSON RPC (e.g. with this script), we get the gran key file named properly (and nodes finalizing blocks successfully)

$ find /tmp/node01/chains/local_testnet/keystore -type f -printf "%f\n"
617572619effc1668ca381c242885516ec9fa2b19c67b6684c02a8a3237b6862e5c8cd7e
6772616eb48004c6e1625282313b07d1c9950935e86894a2e4f21fb1ffee9854d180c781
@agryaznov
Copy link
Contributor Author

found in version:

[dependencies.sc-cli]
features = ['wasmtime']
git = 'https://github.com/paritytech/substrate.git'
tag = 'monthly-2021-09+1'
version = '0.10.0-dev'

@agryaznov agryaznov changed the title sp_cli key insert gives wrong name to _gran_ key file created in local keystorage sp_cli key insert gives wrong name to _gran_ key file created in local keystorage (BREAKS blocks finalization) Sep 28, 2021
@agryaznov
Copy link
Contributor Author

Figured out that:

InsertKeyCmd run() function just assumes the default Sr25519 crypto scheme when no --scheme parameter set to node-template key insert command, no matter which --key-type was set.

But shouldn't it implicitly derive the Ed25519 crypto scheme in case when --key-type=gran ?

agryaznov pushed a commit to agryaznov/substrate-developer-hub.github.io that referenced this issue Sep 29, 2021
Option 3: Use the `key` command --- corrected to workaround the #9888 issue (otherwise it just won't work for <gran> key)  
see paritytech/substrate#9888 for details
agryaznov pushed a commit to agryaznov/substrate-developer-hub.github.io that referenced this issue Sep 29, 2021
Option 3: Use the `key` command --- corrected to workaround the #9888 issue (otherwise it just won't work for <gran> key)  
see paritytech/substrate#9888 for details
@agryaznov agryaznov changed the title sp_cli key insert gives wrong name to _gran_ key file created in local keystorage (BREAKS blocks finalization) sc_cli key insert gives wrong name to _gran_ key file created in local keystorage (BREAKS blocks finalization) Sep 29, 2021
nuke-web3 pushed a commit to polkadot-developers/substrate-developer-hub.github.io that referenced this issue Sep 29, 2021
Option 3: Use the `key` command --- corrected to workaround the #9888 issue (otherwise it just won't work for <gran> key)  
see paritytech/substrate#9888 for details
@nuke-web3
Copy link
Contributor

Thanks for the docs correction to specify scheme. I think this is OK to close, but possibly there should be a fix in the CLI's behavior to try and reason about if your key is correct or not. I will let the core team comment on that idea 🙏

@nuke-web3 nuke-web3 added J2-unconfirmed Issue might be valid, but it’s not yet known. Z7-question Issue is a question. Closer should answer. labels Sep 29, 2021
@bkchr
Copy link
Member

bkchr commented Oct 1, 2021

But shouldn't it implicitly derive the Ed25519 crypto scheme in case when --key-type=gran ?

It doesn't know this.

However, with this: #9909 the scheme will be required to be passed and not assumed anymore.

@ghost ghost closed this as completed in #9909 Oct 4, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J2-unconfirmed Issue might be valid, but it’s not yet known. Z7-question Issue is a question. Closer should answer.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants