Support for `keyring` in runtimes #2044

michalkucharczyk · 2023-10-26T14:00:32Z

This functionality is required for #1984.

This PR enables sp-keyring in no-std environments, allowing to generate the public key (e.g. AccountKeyring::Alice.public().to_ss58check()), which can be later used in the any of built-in runtime-genesis-config variant.

The proposal is as follows:

expose core::Pair trait in no-std,
full_crypto feature enables sign method,
std feature enables generate_with_phrase and generate methods (randomness is required),
All other functionality, currently gated by full_crypto will be available unconditionally (no-std):
-- from_string
-- from_string_with_seed
-- from seed
-- from_seed_slice
-- from_phrase
-- derive
-- verify

Depends on rust-bitcoin/rust-bip39#57

michalkucharczyk · 2023-10-26T14:15:50Z

substrate/test-utils/runtime/src/lib.rs

@@ -723,6 +725,7 @@ impl_runtime_apis! {

 	impl sp_genesis_builder::GenesisBuilder<Block> for Runtime {
 		fn create_default_config() -> Vec<u8> {
+			log::info!("{:#?}", AccountKeyring::Alice.public().to_ss58check());


We need this to work in wasm runtime.
Can be checked with:
RUST_LOG=info cargo test --release -p substrate-test-runtime -- default_config_as_json_works

michalkucharczyk · 2023-10-26T14:16:40Z

it is still not working, I am getting compilation error (for rococo runtime, substrate-test-runtime works fine):

  error[E0152]: found duplicate lang item `panic_impl`
      --> /home/miszka/parity/10-genesis-config/polkadot-sdk-test/substrate/primitives/io/src/lib.rs:1749:1
       |
  1749 | pub fn panic(info: &core::panic::PanicInfo) -> ! {
       | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       |
       = note: the lang item is first defined in crate `std` (which `digest` depends on)
       = note: first definition in `std` loaded from /home/miszka/.rustup/toolchains/nightly-2023-05-22-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib/libstd-4753a923af9402b6.rlib
       = note: second definition in the local crate (`sp_io`)

Looks like we have problem with std (which is pulled by digest which is probably pulled by schnorkel).

burdges · 2023-10-27T07:04:47Z

This will require to add Keyring support into runtime:

Oh? We take the string "Alice", turn it into a secret key, which somehow pulls in some name of the chain, and then put the public key into the genesis config?

I'd expect this adds considerable code to the runtime, like the 32kb basepoint table. I'm unsure how curve25519-dalek does backend selection now, but once upon a time it was manual.

Anyways schnorrkel pulls in digest through curve25519-dalek and sha2, which could be optional probably, but schnorrkel does not obviously propagate std to either.

https://github.com/search?q=repo%3Aw3f%2Fschnorrkel+path%3A%2F%5Esrc%5C%2F%2F+digest&type=code

I'd expect digest gets pulled in a few othr places, like ss58 maybe.

michalkucharczyk · 2023-10-27T08:32:54Z

Oh? We take the string "Alice", turn it into a secret key, which somehow pulls in some name of the chain, and then put the public key into the genesis config?

Yeah, we don't need secret key, true. Some other potential ways to go around:

hard-coding the public keys bytes in chainspec + adding some some comment on the origin of the key,
adding the public-keys-db crate that contains common keys used in chainspecs (so we can still refer them by URI string).

burdges · 2023-10-27T08:45:55Z

we don't need secret key, true.

You need them for derivation since presumably "Alice" is the secret key.

hard-coding the public keys bytes in chainspec + adding some some comment on the origin of the key,

You could just add a check in the host that they agree I guess.

It's not ideal to expose signing in the runtime since then people might think they can use it.

bkchr

Looking good so far.

substrate/primitives/keyring/Cargo.toml

substrate/test-utils/runtime/Cargo.toml

substrate/primitives/core/src/sr25519.rs

bkchr · 2023-10-27T08:54:27Z

I'd expect digest gets pulled in a few othr places, like ss58 maybe.

Can we not just disable the std feature for it?

michalkucharczyk · 2023-10-27T13:16:48Z

It looks like tiny-bip39 is not ready for no-std:
https://github.com/maciejhirsz/tiny-bip39/blob/7076c6fb2390e86916b0ae36f2393d7dce61d9e3/src/lib.rs#L29

Is there a good no-std alternative for this?

bkchr · 2023-10-27T14:29:27Z

Is there a good no-std alternative for this?

Maybe the original crate bip39 this was forked from? Though, I don't know why it was forked.

@maciejhirsz can you shed some light on why you forked it?

michalkucharczyk · 2023-10-27T16:59:35Z

required support for no_std:

in tiny-bip39: Support for no_std maciejhirsz/tiny-bip39#44
in substrate-bip39: Support for no-std substrate-bip39#18

michalkucharczyk · 2023-10-27T17:02:33Z

Is there a good no-std alternative for this?

Maybe the original crate bip39 this was forked from? Though, I don't know why it was forked.

https://github.com/infincia/bip39-rs looks like abandoned.

bkchr · 2023-10-27T19:04:55Z

https://github.com/infincia/bip39-rs looks like abandoned.

https://github.com/rust-bitcoin/rust-bip39/ this is the one that was forked. At least if you follow from crates.io

michalkucharczyk · 2023-10-30T13:17:50Z

https://github.com/infincia/bip39-rs looks like abandoned.

https://github.com/rust-bitcoin/rust-bip39/ this is the one that was forked. At least if you follow from crates.io

Switched to https://github.com/rust-bitcoin/rust-bip39/ in new PR (to make review easier): #2084

burdges · 2023-10-30T14:56:27Z

Do you know what backend flags curve25519-dalek etc require for the runtime?

michalkucharczyk · 2023-10-30T15:07:21Z

Do you know what backend flags curve25519-dalek etc require for the runtime?

No. This particular one curve25519-dalek is not pulled-in explicitly.
It is pulled by:

ed25519-zebra
schnorrkel

For schnorrkel (which now is at v0.9.1) I assumed that the same feature set would work for runtime and std: "preaudit_deprecated", "u64_backend"

ed25519-zebra goes with no-default-features for runtime.

Do you recommend any adjustment here?

…ime2

burdges · 2023-10-30T15:47:25Z

For schnorrkel (which now is at v0.9.1) I assumed that the same feature set would work for runtime and std: "preaudit_deprecated", "u64_backend"

I've no idea.. A while back, curve25519-dalek got much better at autodetection, which afaik solved all native issues. I'm not sure what happens in wasm though. Anyways you've watched it actually work? No bugs appear?

michalkucharczyk · 2023-10-30T15:56:45Z

For schnorrkel (which now is at v0.9.1) I assumed that the same feature set would work for runtime and std: "preaudit_deprecated", "u64_backend"

I've no idea.. A while back, curve25519-dalek got much better at autodetection, which afaik solved all native issues. I'm not sure what happens in wasm though. Anyways you've watched it actually work? No bugs appear?

I don't have any heavy tests to cross-check keys generation. Simple, naive shot seems to work fine - basically it is all we need.

Switch from: https://crates.io/crates/tiny-bip39 to: https://crates.io/crates/bip39 Required for: #2044

…ime2

bkchr · 2024-03-11T10:41:36Z

@michalkucharczyk we can not merge this with git dependencies. This will break crates.io releases.

bkchr · 2024-03-11T10:45:44Z

We are actually working on getting rid off them: #2922

Must of them are currently either removed at release manually or are being in crates that we don't have released right now. Nothing of that applies here.

michalkucharczyk · 2024-03-11T11:20:42Z

@bkchr do you think I should create release on crates.io using this fork https://github.com/michalkucharczyk/rust-bip39 ?

There are two pending PRs in original repo, but things seems to be stuck there:
rust-bitcoin/rust-bip39#64
rust-bitcoin/rust-bip39#57

bkchr · 2024-03-11T11:34:20Z

Personally I'm not a big fan of this. The problem being that releases put onto crates.io will stay there forever.

However, I see that there is no movement. If you like, release it on your own. But please monitor and switch back to upstream when they have finished the release.

…ime2

* master: (65 commits) collator protocol changes for elastic scaling (validator side) (#3302) Contracts use polkavm workspace deps (#3715) Add elastic scaling support in ParaInherent BenchBuilder (#3690) Removes `as [disambiguation_path]` from `derive_impl` usage (#3652) fix(paseo-spec): New Paseo Bootnodes (#3674) Improve Penpal runtime + emulated tests (#3543) Staking ledger bonding fixes (#3639) DescribeAllTerminal for HashedDescription (#3349) Increase timeout for assertions (#3680) Add subsystems regression tests to CI (#3527) Always print connectivity report (#3677) Revert "FRAME: Create `TransactionExtension` as a replacement for `SignedExtension` (#2280)" (#3665) authority-discovery: Add log for debugging DHT authority records (#3668) Construct Runtime v2 (#1378) Support for `keyring` in runtimes (#2044) Add api-name in `cannot query the runtime API version` warning (#3653) Add a PolkaVM-based executor (#3458) Adds default config for assets pallet (#3637) Bump handlebars from 4.3.7 to 5.1.0 (#3248) [Collator Selection] Fix weight refund for `set_candidacy_bond` (#3643) ...

This PR replaces the usage of [secp256k](https://crates.io/crates/secp256k1) crate with [k256](https://crates.io/crates/k256) in `core::crypto::ecdsa` for `non-std` environments as outcome of discussion in paritytech#3448. `secp256k1` is used in `std`, meaning that we should not affect host performance with this PR. `k256` is enabled in runtimes (`no-std`), and is required to proceed with paritytech#2044. If desirable, in future we can switch to `k256` also for `std`. That would require some performance evaluation (e.g. for EVM chains as per paritytech#3448 (comment)). Closes paritytech#3448 --------- Co-authored-by: command-bot <> Co-authored-by: Davide Galassi <davxy@datawok.net>

This functionality is required for paritytech#1984. This PR enables [`sp-keyring`](https://github.com/paritytech/polkadot-sdk/blob/21d36b7b4229c4d5225944f197918cde23fda4ea/substrate/primitives/keyring/src/sr25519.rs#L31-L40) in `no-std` environments, allowing to generate the public key (e.g. `AccountKeyring::Alice.public().to_ss58check()`), which can be later used in the any of built-in [_runtime-genesis-config_ variant](https://github.com/paritytech/polkadot-sdk/blob/21d36b7b4229c4d5225944f197918cde23fda4ea/polkadot/node/service/src/chain_spec.rs#L1066-L1073). The proposal is as follows: - expose [`core::Pair` trait](https://github.com/paritytech/polkadot-sdk/blob/d6f15306282e3de848a09c9aa9cba6f95a7811f0/substrate/primitives/core/src/crypto.rs#L832) in `no-std`, - `full_crypto` feature enables `sign` method, - `std` feature enables `generate_with_phrase` and `generate` methods (randomness is required), - All other functionality, currently gated by `full_crypto` will be available unconditionally (`no-std`): -- `from_string` -- `from_string_with_seed` -- `from seed` -- `from_seed_slice` -- `from_phrase` -- `derive` -- `verify` --- Depends on rust-bitcoin/rust-bip39#57 --------- Co-authored-by: command-bot <> Co-authored-by: Davide Galassi <davxy@datawok.net>

Switch from: https://crates.io/crates/tiny-bip39 to: https://crates.io/crates/bip39 Required for: paritytech#2044

…ritytech#2250) The `lazy_static` package does not work well in `no-std`: it requires `spin_no_std` feature, which also will propagate into `std` if enabled. This is not what we want. This PR provides simple address uri parser which allows to get rid of _regex_ which was used to parse the address uri, what in turns allows to remove lazy_static. Three regular expressions (`SS58_REGEX`,`SECRET_PHRASE_REGEX`,`JUNCTION_REGEX`) were replaced with the parser which unifies all of them. The new parser does not support Unicode, it is ASCII only. Related to: paritytech#2044 --------- Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: Koute <koute@users.noreply.github.com> Co-authored-by: command-bot <>

This PR replaces the usage of [secp256k](https://crates.io/crates/secp256k1) crate with [k256](https://crates.io/crates/k256) in `core::crypto::ecdsa` for `non-std` environments as outcome of discussion in paritytech#3448. `secp256k1` is used in `std`, meaning that we should not affect host performance with this PR. `k256` is enabled in runtimes (`no-std`), and is required to proceed with paritytech#2044. If desirable, in future we can switch to `k256` also for `std`. That would require some performance evaluation (e.g. for EVM chains as per paritytech#3448 (comment)). Closes paritytech#3448 --------- Co-authored-by: command-bot <> Co-authored-by: Davide Galassi <davxy@datawok.net>

This functionality is required for paritytech#1984. This PR enables [`sp-keyring`](https://github.com/paritytech/polkadot-sdk/blob/21d36b7b4229c4d5225944f197918cde23fda4ea/substrate/primitives/keyring/src/sr25519.rs#L31-L40) in `no-std` environments, allowing to generate the public key (e.g. `AccountKeyring::Alice.public().to_ss58check()`), which can be later used in the any of built-in [_runtime-genesis-config_ variant](https://github.com/paritytech/polkadot-sdk/blob/21d36b7b4229c4d5225944f197918cde23fda4ea/polkadot/node/service/src/chain_spec.rs#L1066-L1073). The proposal is as follows: - expose [`core::Pair` trait](https://github.com/paritytech/polkadot-sdk/blob/d6f15306282e3de848a09c9aa9cba6f95a7811f0/substrate/primitives/core/src/crypto.rs#L832) in `no-std`, - `full_crypto` feature enables `sign` method, - `std` feature enables `generate_with_phrase` and `generate` methods (randomness is required), - All other functionality, currently gated by `full_crypto` will be available unconditionally (`no-std`): -- `from_string` -- `from_string_with_seed` -- `from seed` -- `from_seed_slice` -- `from_phrase` -- `derive` -- `verify` --- Depends on rust-bitcoin/rust-bip39#57 --------- Co-authored-by: command-bot <> Co-authored-by: Davide Galassi <davxy@datawok.net>

michalkucharczyk added 6 commits October 25, 2023 12:54

wip: sp-core::core

814b597

wip: sp-keyring

69f2168

wip: test-runtime

f6a5ec0

wip: keyring 2

a6ee754

wip: core 2

a79bfb6

wip: application-crypto 2

d6f1530

michalkucharczyk commented Oct 26, 2023

View reviewed changes

bkchr reviewed Oct 27, 2023

View reviewed changes

substrate/primitives/keyring/Cargo.toml Outdated Show resolved Hide resolved

substrate/test-utils/runtime/Cargo.toml Outdated Show resolved Hide resolved

substrate/primitives/core/src/sr25519.rs Outdated Show resolved Hide resolved

fixes

0cbfefb

michalkucharczyk mentioned this pull request Oct 30, 2023

Switch from tiny-bip39 to bip39 crate #2084

Merged

Merge remote-tracking branch 'origin/master' into mku-keyring-in-runt…

bbc43c2

…ime2

michalkucharczyk added the T0-node This PR/Issue is related to the topic “node”. label Oct 30, 2023

bkchr pushed a commit that referenced this pull request Oct 30, 2023

Switch from tiny-bip39 to bip39 crate (#2084)

a69da4a

Switch from: https://crates.io/crates/tiny-bip39 to: https://crates.io/crates/bip39 Required for: #2044

michalkucharczyk added 2 commits October 30, 2023 17:09

Merge remote-tracking branch 'origin/master' into mku-keyring-in-runt…

9650d42

…ime2

substrate-bip39 bumped to 0.4.5

cf11ccb

michalkucharczyk added 3 commits March 9, 2024 19:49

fixes

861ea0b

ecdsa: more verify variants enabled w/o full_crypto

037f70c

ecdsa: full_crypto guard removed where not needed

c55ec33

rcny approved these changes Mar 10, 2024

View reviewed changes

Merge branch 'master' into mku-keyring-in-runtime2

58512bd

michalkucharczyk added this pull request to the merge queue Mar 11, 2024

bkchr approved these changes Mar 11, 2024

View reviewed changes

bkchr removed this pull request from the merge queue due to a manual request Mar 11, 2024

michalkucharczyk added 2 commits March 12, 2024 11:21

Use parity-bip39 instead of git forks

4ec9e42

Merge remote-tracking branch 'origin/master' into mku-keyring-in-runt…

b763059

…ime2

michalkucharczyk mentioned this pull request Mar 12, 2024

Remove deps to parity-bip39 #3655

Open

fix

92fcab2

michalkucharczyk added this pull request to the merge queue Mar 12, 2024

Merged via the queue into master with commit a756baf Mar 12, 2024
130 of 131 checks passed

michalkucharczyk deleted the mku-keyring-in-runtime2 branch March 12, 2024 12:22

davxy mentioned this pull request Mar 17, 2024

Remove unsafe cell from SecretKey w3f/ring-vrf#87

Closed

bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024

Switch from tiny-bip39 to bip39 crate (paritytech#2084)

b52f961

Switch from: https://crates.io/crates/tiny-bip39 to: https://crates.io/crates/bip39 Required for: paritytech#2044

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 26, 2024

Remove unneeded files (paritytech#2044)

fa454c3

nazar-pc mentioned this pull request Apr 15, 2024

Update Substrate to 1.10.1 subspace/subspace#2683

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for `keyring` in runtimes #2044

Support for `keyring` in runtimes #2044

michalkucharczyk commented Oct 26, 2023 •

edited by davxy

michalkucharczyk Oct 26, 2023 •

edited

michalkucharczyk commented Oct 26, 2023

burdges commented Oct 27, 2023

michalkucharczyk commented Oct 27, 2023

burdges commented Oct 27, 2023 •

edited

bkchr left a comment

bkchr commented Oct 27, 2023

michalkucharczyk commented Oct 27, 2023

bkchr commented Oct 27, 2023

michalkucharczyk commented Oct 27, 2023 •

edited

michalkucharczyk commented Oct 27, 2023

bkchr commented Oct 27, 2023

michalkucharczyk commented Oct 30, 2023

burdges commented Oct 30, 2023

michalkucharczyk commented Oct 30, 2023 •

edited

burdges commented Oct 30, 2023

michalkucharczyk commented Oct 30, 2023

bkchr commented Mar 11, 2024

bkchr commented Mar 11, 2024

michalkucharczyk commented Mar 11, 2024 •

edited

bkchr commented Mar 11, 2024

Support for keyring in runtimes #2044

Support for keyring in runtimes #2044

Conversation

michalkucharczyk commented Oct 26, 2023 • edited by davxy

michalkucharczyk Oct 26, 2023 • edited

Choose a reason for hiding this comment

michalkucharczyk commented Oct 26, 2023

burdges commented Oct 27, 2023

michalkucharczyk commented Oct 27, 2023

burdges commented Oct 27, 2023 • edited

bkchr left a comment

Choose a reason for hiding this comment

bkchr commented Oct 27, 2023

michalkucharczyk commented Oct 27, 2023

bkchr commented Oct 27, 2023

michalkucharczyk commented Oct 27, 2023 • edited

michalkucharczyk commented Oct 27, 2023

bkchr commented Oct 27, 2023

michalkucharczyk commented Oct 30, 2023

burdges commented Oct 30, 2023

michalkucharczyk commented Oct 30, 2023 • edited

burdges commented Oct 30, 2023

michalkucharczyk commented Oct 30, 2023

bkchr commented Mar 11, 2024

bkchr commented Mar 11, 2024

michalkucharczyk commented Mar 11, 2024 • edited

bkchr commented Mar 11, 2024

Support for `keyring` in runtimes #2044

Support for `keyring` in runtimes #2044

michalkucharczyk commented Oct 26, 2023 •

edited by davxy

michalkucharczyk Oct 26, 2023 •

edited

burdges commented Oct 27, 2023 •

edited

michalkucharczyk commented Oct 27, 2023 •

edited

michalkucharczyk commented Oct 30, 2023 •

edited

michalkucharczyk commented Mar 11, 2024 •

edited