Add `frame_support::crypto::ecdsa::Public.to_eth_address()` (`k256`-based) and use it in pallets#11087

agryaznov · 2022-03-22T11:13:38Z

As a follow-up to #10883 and a pre-requisite to ink#1068, this PR:

adds pub fn to_eth_address(&self) -> Result<[u8; 20], ()> to ~~sp_core::ecdsa::Public~~ frame_support::crypto::ecdsa
updates existing fn convert(a: beefy_primitives::crypto::AuthorityId) -> Vec<u8> in pallet-beefy-mmr to use it
adds seal_to_eth_address to pallet-contracts API which uses it

…it, contracts Ext interface

athei

First round. Didn't look at the contracts changes, yet.

primitives/core/src/lib.rs

primitives/core/Cargo.toml

primitives/core/src/ecdsa.rs

frame/beefy-mmr/src/lib.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

…cts to use it

agryaznov · 2022-03-30T16:54:48Z

Had to put this new function into frame_support, to keep crypto calculations out of runtime (wasm performs weakly on those), while still avoiding creation of a new host function.

@shawntabrizi please advise if the place for it is chosen correctly.

athei

Please add some tests in the new ecdsa file.

frame/support/src/crypto/ecdsa.rs

frame/contracts/src/wasm/runtime.rs

frame/contracts/src/wasm/mod.rs

frame/support/src/crypto/ecdsa.rs

athei

For the CI try to merge master and see if that helps.

bkchr

Didn't check the benchmarking code in detail.

Some general points, please never use unwrap in the runtime! Either expect with a proof or remove. In your case here you need to remove all of them.

frame/beefy-mmr/src/lib.rs

frame/support/src/crypto.rs

frame/support/src/crypto/ecdsa.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

bkchr · 2022-04-15T06:12:01Z

frame/support/src/crypto/ecdsa.rs

+				<[u8; 20]>::try_from(
+					sp_io::hashing::keccak_256(&uncompressed.as_bytes()[1..])[12..].as_ref(),
+				)
+				.expect("failed to take last 20 bytes of a keccak_256 hash")


Why do you still us an expect here? I already gave you a solution where you would not need this anymore.

your version didn't compile

Yeah it failed, but the solution wasn't that complicated. I pushed a commit to fix this.

…ased) and use it in pallets (paritytech#11087) * `ecdsa::Public::to_eth_address` + test, beefy-mmr `convert()` to use it, contracts Ext interface * `seal_ecdsa_to_eth_address` all but benchmark done * `seal_ecdsa_to_eth_address` + wasm test * `seal_ecdsa_to_eth_address` + benchmark * fixed dependencies * Apply suggestions from code review Co-authored-by: Alexander Theißen <alex.theissen@me.com> * fixes from review paritytech#1 * ecdsa::Public(*pk).to_eth_address() moved to frame_support and contracts to use it * beefy-mmr to use newly added frame_support function for convertion * a doc fix * import fix * benchmark fix-1 (still fails) * benchmark fixed * Apply suggestions from code review Co-authored-by: Alexander Theißen <alex.theissen@me.com> * fixes on Alex T feedback * to_eth_address() put into extension trait for sp-core::ecdsa::Public * Update frame/support/src/crypto/ecdsa.rs Co-authored-by: Alexander Theißen <alex.theissen@me.com> * Update frame/contracts/src/wasm/mod.rs Co-authored-by: Alexander Theißen <alex.theissen@me.com> * fixes on issues pointed out in review * benchmark errors fixed * fmt fix * EcdsaRecoverFailed err docs updated * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * make applied suggestions compile * get rid of unwrap() in runtime * Remove expect Co-authored-by: Alexander Theißen <alex.theissen@me.com> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Bastian Köcher <info@kchr.de>

…ased) and use it in pallets (paritytech#11087) * `ecdsa::Public::to_eth_address` + test, beefy-mmr `convert()` to use it, contracts Ext interface * `seal_ecdsa_to_eth_address` all but benchmark done * `seal_ecdsa_to_eth_address` + wasm test * `seal_ecdsa_to_eth_address` + benchmark * fixed dependencies * Apply suggestions from code review Co-authored-by: Alexander Theißen <alex.theissen@me.com> * fixes from review #1 * ecdsa::Public(*pk).to_eth_address() moved to frame_support and contracts to use it * beefy-mmr to use newly added frame_support function for convertion * a doc fix * import fix * benchmark fix-1 (still fails) * benchmark fixed * Apply suggestions from code review Co-authored-by: Alexander Theißen <alex.theissen@me.com> * fixes on Alex T feedback * to_eth_address() put into extension trait for sp-core::ecdsa::Public * Update frame/support/src/crypto/ecdsa.rs Co-authored-by: Alexander Theißen <alex.theissen@me.com> * Update frame/contracts/src/wasm/mod.rs Co-authored-by: Alexander Theißen <alex.theissen@me.com> * fixes on issues pointed out in review * benchmark errors fixed * fmt fix * EcdsaRecoverFailed err docs updated * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * make applied suggestions compile * get rid of unwrap() in runtime * Remove expect Co-authored-by: Alexander Theißen <alex.theissen@me.com> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Bastian Köcher <info@kchr.de>

agryaznov added 6 commits March 21, 2022 17:06

ecdsa::Public::to_eth_address + test, beefy-mmr convert() to use …

4c5f9fe

…it, contracts Ext interface

seal_ecdsa_to_eth_address all but benchmark done

414c40e

seal_ecdsa_to_eth_address + wasm test

a7453f4

seal_ecdsa_to_eth_address + benchmark

7e040fd

Merge branch 'master' into ag-seal-to-eth-addr

f843bc8

fixed dependencies

d3090c7

agryaznov requested review from acatangiu and athei as code owners March 22, 2022 11:13

athei reviewed Mar 22, 2022

View reviewed changes

primitives/core/src/lib.rs Show resolved Hide resolved

primitives/core/Cargo.toml Outdated Show resolved Hide resolved

primitives/core/src/ecdsa.rs Outdated Show resolved Hide resolved

frame/beefy-mmr/src/lib.rs Show resolved Hide resolved

frame/beefy-mmr/src/lib.rs Show resolved Hide resolved

agryaznov and others added 6 commits March 22, 2022 19:15

Apply suggestions from code review

869cf84

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

fixes from review #1

c4b6a29

Merge branch 'master' into ag-seal-to-eth-addr

3dbda9f

ecdsa::Public(*pk).to_eth_address() moved to frame_support and contra…

f5feb46

…cts to use it

beefy-mmr to use newly added frame_support function for convertion

b801972

a doc fix

be3e8a9

agryaznov changed the title ~~Add sp_core::ecdsa::Public.to_eth_address() (k256-based) and use it in pallets~~ Add frame_support::crypto::ecdsa::Public.to_eth_address() (k256-based) and use it in pallets Mar 30, 2022

import fix

4037170

agryaznov requested a review from shawntabrizi March 30, 2022 16:54

agryaznov requested a review from athei March 30, 2022 16:54

agryaznov added 3 commits March 30, 2022 20:23

benchmark fix-1 (still fails)

c8ff0ef

benchmark fixed

976ed7e

Merge branch 'master' into ag-seal-to-eth-addr

a26858b

athei reviewed Apr 1, 2022

View reviewed changes

bkchr reviewed Apr 4, 2022

View reviewed changes

frame/support/src/crypto/ecdsa.rs Outdated Show resolved Hide resolved

agryaznov added 5 commits April 11, 2022 21:03

fixes on issues pointed out in review

cdd6bdb

benchmark errors fixed

b51a7b0

Merge branch 'master' into ag-seal-to-eth-addr

fb1fcc4

fmt fix

41ba268

EcdsaRecoverFailed err docs updated

34779f8

agryaznov force-pushed the ag-seal-to-eth-addr branch from a70d756 to 34779f8 Compare April 11, 2022 19:54

athei approved these changes Apr 12, 2022

View reviewed changes

Merge branch 'master' into ag-seal-to-eth-addr

10af685

github-actions bot added the A7-needspolkadotpr label Apr 12, 2022

athei requested a review from bkchr April 13, 2022 14:37

bkchr approved these changes Apr 14, 2022

View reviewed changes

agryaznov and others added 4 commits April 14, 2022 19:59

Apply suggestions from code review

7c5fa91

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

make applied suggestions compile

ec3a2af

get rid of unwrap() in runtime

dffce61

Merge branch 'master' into ag-seal-to-eth-addr

55c2d06

agryaznov requested a review from athei April 14, 2022 21:36

bkchr suggested changes Apr 15, 2022

View reviewed changes

agryaznov requested a review from bkchr April 15, 2022 07:16

Remove expect

2997b7e

bkchr approved these changes Apr 16, 2022

View reviewed changes

agryaznov merged commit a5cd385 into master Apr 16, 2022

agryaznov deleted the ag-seal-to-eth-addr branch April 16, 2022 12:51

agryaznov mentioned this pull request Apr 24, 2022

Implement ecdsa_to_eth_address() and remove eth_compatibility crate use-ink/ink#1233

Merged

github-actions bot mentioned this pull request May 31, 2022

Update substrate/polkadot/cumulus from v0.9.20 to v0.9.22 moonbeam-foundation/moonbeam#1557

Closed

ntn-x2 mentioned this pull request Jun 7, 2022

feat: support linking ethereum accounts KILTprotocol/kilt-node#355

Merged

6 tasks

github-actions bot mentioned this pull request Jun 22, 2022

Update substrate/polkadot/cumulus from v0.9.20 to v0.9.23 moonbeam-foundation/moonbeam#1623

Closed

Dengjianping mentioned this pull request Jul 7, 2022

Bump deps to v0.9.22 Manta-Network/Manta#571

Merged

10 tasks

Comments

Conversation

agryaznov commented Mar 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

athei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agryaznov commented Mar 30, 2022

Uh oh!

athei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

athei left a comment

Choose a reason for hiding this comment

Uh oh!

bkchr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkchr Apr 15, 2022

Choose a reason for hiding this comment

Uh oh!

agryaznov Apr 15, 2022

Choose a reason for hiding this comment

Uh oh!

bkchr Apr 16, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

agryaznov commented Mar 22, 2022 •

edited

Loading