Update EVM account mapping, add contract account handling #788

Mr-Leshiy · 2022-04-01T12:44:15Z

No description provided.

…ror_1 test

zeegomo

The AccountState thing looks a bit like a state machine to me (not mapped, mapped, contract account, ...), maybe we make it more explicit?

zeegomo · 2022-04-07T10:17:26Z

chain-impl-mockchain/src/ledger/evm.rs

    jor_to_evm: Hamt<DefaultHasher, JorAddress, EvmAddress>,
 }

+fn transfrom_evm_to_jor(evm_id: &EvmAddress) -> JorAddress {


This should be well documented

zeegomo · 2022-04-11T10:39:01Z

chain-impl-mockchain/src/ledger/evm.rs

+
+        // should update and move account evm account state
+        let old_jor_id = transfrom_evm_to_jor(&evm_id);
+        accounts = accounts.evm_move_state(jor_id, &old_jor_id)?;


Do we allow to map contract accounts to an arbitrary Jormungandr account?

Generally no, we only allow to map to that jormungandr account which user owns, signature validation comes in the another place.

Yes, but that is currently allowed. I would have expected the implementation to progress in the opposite order not to leave exploitable gaps, but that's not too important in the end, just do not forget to add a ticket for that if it does not exist yet

Sure, we have already tickets for that :)

chain-impl-mockchain/src/ledger/evm.rs

chain-impl-mockchain/src/accounting/account/mod.rs

Mr-Leshiy · 2022-04-12T12:29:06Z

@zeegomo

The AccountState thing looks a bit like a state machine to me (not mapped, mapped, contract account, ...), maybe we make it more explicit?

Yes, good point, but need to think how it is better to do it.

zeegomo · 2022-04-13T12:55:42Z

chain-impl-mockchain/src/ledger/evm.rs

+/// This allows to map any `EvmAddress` to the `JorAddress` before explicit execution of the `EvmMapping` transaction.
+/// Intention - is to have possibility to map EVM Contarct accounts with the Jormungandr accounts.


This is confusing though, contract accounts should never be part of a EvmMapping transaction AFAIK, right?

Yes, contract account should not be a part of the EvmMapping certificate, will try to paraphrase it. I meant that with the usage of this function we can link contract account with the jormungandr.

contract account should be a part of the EvmMapping certificate

How can this happen?

Sorry, mistaken 😓

contract account should NOT be a part of the EvmMapping certificate

zeegomo · 2022-05-12T15:24:48Z

chain-impl-mockchain/src/ledger/evm.rs

+/// This allows to map any `EvmAddress` to the `JorAddress` before explicit execution of the `EvmMapping` transaction.
+/// Intention - is to have possibility to link an EVM Contarct account with a Jormungandr account.


those are two slightly different use cases.

standard account not yet mapped, but that could be mapped in the future

contract account that can't possibly be mapped

zeegomo · 2022-05-12T15:25:41Z

chain-impl-mockchain/src/ledger/evm.rs

+///
+/// Algorithm description:
+///  1. Get `evm_address` bytes representation -> evm_address_bytes
+///  2. Append b"evm" bytes prefix to the evm_address_bytes -> bytes_data


nit: append a prefix = prepend

zeegomo · 2022-05-12T15:26:17Z

chain-impl-mockchain/src/ledger/evm.rs

+/// Algorithm description:
+///  1. Get `evm_address` bytes representation -> evm_address_bytes
+///  2. Append b"evm" bytes prefix to the evm_address_bytes -> bytes_data
+///  3. Calculate hash (pub struct Hash(crypto::Blake2b256) hash) from the bytes_data -> hash_bytes


nit: don't need a reference to the specific Rust impl here, just mentioning blake2b256 is enough

zeegomo

I do not master all of the details here, but let's proceed with what we have agreed on

Mr-Leshiy added 3 commits April 1, 2022 13:23

add transfrom_evm_to_jor function

a2c6faa

update evm ledger interface, norrow the scope

ccace08

add new evm_move_state function

3ac119a

Mr-Leshiy marked this pull request as draft April 1, 2022 12:44

Mr-Leshiy added 3 commits April 1, 2022 16:05

fix clippy

7f39eb6

fix evm_move_state function

241d75b

update transfrom_evm_to_jor

c803f5b

Mr-Leshiy force-pushed the ca-accounts-mapping branch from 0050f5e to c803f5b Compare April 4, 2022 08:25

Merge branch 'master' into ca-accounts-mapping

a81ae59

Mr-Leshiy requested review from zeegomo, saibatizoku and eugene-babichenko April 4, 2022 09:18

Mr-Leshiy marked this pull request as ready for review April 4, 2022 09:19

add account ledger tests

4ab495b

Mr-Leshiy force-pushed the ca-accounts-mapping branch from 1adea71 to 4ab495b Compare April 4, 2022 14:15

Mr-Leshiy added 15 commits April 4, 2022 18:50

fix clippy

39293bb

small refactor

f5fb2e7

fix clippy

717bc5d

add two basic test cases

a3f11c7

add one test case

1ebedd5

add additional error tests

ce63451

Merge branch 'master' into ca-accounts-mapping

516a698

fix clippy

a19d8e0

fix typos

ae45f8f

add run_transaction_call_test_1

d59b033

add run_transaction_call_test_2 test

3204dff

fix run_transaction_call_test_2, add new run_transaction_call_test_er…

c9de4f3

…ror_1 test

add run_transaction_call_test_error_2 test

7a0ac76

add contract address generation helper functions,

87df6c3

update

81f7045

Mr-Leshiy added 3 commits April 11, 2022 12:07

update run_transaction_create_test_1

55e16c5

add few more tests

72714c7

add error tests

0fd907c

zeegomo suggested changes Apr 11, 2022

View reviewed changes

Mr-Leshiy added 4 commits April 12, 2022 11:25

Merge branch 'master' into ca-accounts-mapping

9db2165

fix clippy

6b642fe

fix typo

1766581

remove unneeded code

3f5c065

Mr-Leshiy requested a review from zeegomo April 13, 2022 11:21

zeegomo reviewed Apr 13, 2022

View reviewed changes

add desctiption for the transform_evm_to_jor function

e0c0075

Mr-Leshiy force-pushed the ca-accounts-mapping branch from 2d6f3d2 to e0c0075 Compare April 14, 2022 10:00

Mr-Leshiy requested a review from zeegomo April 14, 2022 12:19

Mr-Leshiy added 4 commits April 28, 2022 15:42

Merge branch 'master' into ca-accounts-mapping

d991ac5

Merge branch 'master' into ca-accounts-mapping

72c4366

fix build

7f41922

Merge branch 'master' into ca-accounts-mapping

5b6e43b

zeegomo reviewed May 12, 2022

View reviewed changes

zeegomo approved these changes May 12, 2022

View reviewed changes

update comments

0d3f14e

saibatizoku approved these changes May 12, 2022

View reviewed changes

Mr-Leshiy merged commit f202722 into master May 13, 2022

Mr-Leshiy deleted the ca-accounts-mapping branch May 13, 2022 07:17

		/// This allows to map any `EvmAddress` to the `JorAddress` before explicit execution of the `EvmMapping` transaction.
		/// Intention - is to have possibility to map EVM Contarct accounts with the Jormungandr accounts.

Update EVM account mapping, add contract account handling #788

Update EVM account mapping, add contract account handling #788

Uh oh!

Conversation

Mr-Leshiy commented Apr 1, 2022

Uh oh!

zeegomo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Mr-Leshiy commented Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mr-Leshiy Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeegomo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zeegomo left a comment •

edited

Loading

Mr-Leshiy commented Apr 12, 2022 •

edited

Loading

Mr-Leshiy Apr 13, 2022 •

edited

Loading