Base precompile code between connectors by mfornet · Pull Request #73 · aurora-is-near/aurora-engine

mfornet · 2021-05-11T23:19:14Z

Add precompiles, so eth-connector and erc20-connector can work from it.

sept-en

Added some comments (we already discussed some of them on the call with you), but I'm fine if we don't address them in the current PR to speed up the process. Noted here just for a future reference if this won't be fixed right now.

Looks good to me, great job!

sept-en · 2021-05-11T23:49:12Z

+                String::from_utf8(sdk::current_account_id()).unwrap(),
+                crate::prelude::format!(
+                    r#"{{"receiver_id": "{}", "amount": "{}", "memo": null}}"#,
+                    String::from_utf8(input.to_vec()).unwrap(),


Please, add a check to validate the recipient_account_id and fail if any other data is provided (as we've discussed).

sept-en · 2021-05-11T23:49:35Z

+            let mut input_mut = input;
+            let amount = U256::from_big_endian(&input_mut[..32]).as_u128();
+            input_mut = &input_mut[32..];
+            let receiver_account_id: AccountId = String::from_utf8(input_mut.to_vec()).unwrap();


The same thing about the receiver_account_id validation

sept-en · 2021-05-11T23:50:38Z

+            // Input slice format:
+            //      eth_recipient (20 bytes) - the address of recipient which will receive ETH on Ethereum
+
+            let eth_recipient: String = hex::encode(input);


Please, check the input is exactly 20 bytes long.

sept-en · 2021-05-11T23:55:21Z

+#[allow(dead_code)]
+pub fn signer_account_id() -> Vec<u8> {
+    unsafe {
+        exports::signer_account_id(1);
+        let bytes: Vec<u8> = vec![0u8; exports::register_len(1) as usize];
+        exports::read_register(1, bytes.as_ptr() as *const u64 as u64);
+        bytes
+    }
+}
+
+#[allow(dead_code)]
+pub fn signer_account_pk() -> Vec<u8> {
+    unsafe {
+        exports::signer_account_pk(1);
+        let bytes: Vec<u8> = vec![0u8; exports::register_len(1) as usize];
+        exports::read_register(1, bytes.as_ptr() as *const u64 as u64);
+        bytes
+    }
+}


Why do we need these in the current PR?

mrLSD · 2021-05-12T12:14:29Z

 pub type RawH256 = [u8; 32]; // Unformatted binary data of fixed length.
+pub type Gas = u64;
+pub type Balance = u128;
+


Just notice that we have the same type et eth-connector

Let's have it here since they are common to both

mfornet · 2021-05-12T14:34:51Z

+                // TODO: Which is the expected result in case of failure
+                //  Stopped vs Returned vs Suicided
+                return Ok((ExitSucceed::Stopped, Vec::new(), 0));


What is the expected PrecompileResult if it should fail?

Should we return a flag instead 1 on success, and 0 on failure?

return Ok((ExitSucceed::Returned, [flag].to_vec(), 0));

From what I remember using sha precompile, when you called it, the result is stored in the range from [0, 32]
However, it also returns a boolean value, whether it succeeded or not. How can I return this boolean value from here?

I think we should use ExitError for that purpose

mrLSD · 2021-05-12T17:34:40Z

+/// Get the current nep141 token associated with the current erc20 token.
+/// This will fail is none is associated.
+fn get_nep141_from_erc20(_erc20_token: &[u8]) -> AccountId {
+    // TODO:


Where it will TODO? If not planned now we should create an issue.

it is done in pr #51 , didnt remove for it to compile

mrLSD · 2021-05-12T17:42:43Z

+                    ),
+                )
+            } else {
+                // TODO: Which is the expected result in case of failure


Maybe for that cases, we can add log message?

artob · 2021-05-12T19:09:44Z

@mfornet Is this ready to end up on chain on MainNet tomorrow? If not, please retarget it at the develop branch instead of master.

* Base precompile code between connectors * Handle errors and validate input * Use proper result

* Base precompile code between connectors (#73) * Base precompile code between connectors * Handle errors and validate input * Use proper result * Document `aurora encode-address` usage. * Cache cargo artifacts between CI runs. (#92) * Address comments from audit. (#86) * Validate register length in `read_input_arr20()` * Only read register length in `Engine::get_code_size` * Add `read_input_borsh()` * Ensure `method.args.len() == args_decoded.len()` * Ensure register size is 8 in `read_u64` * Use constant to specify the register ID used in `read_input()` * Reduce size of `cargo cache` in CI. (#95) * Define a `Wei` newtype for balances. (#96) * Fix evm-bully builds after recent refactoring. (#100) * Refactor `Engine::get_state` to return a `Result`. (#99) * Ensure that `Cargo.lock` in the repo is valid. (#101) * Remove unneeded nightly feature. (#102) * Implement BC generational storage. * fix address input * remove note * put key on the end of the storage key * remove pub from methods * Dispatch precompiles on the full address. (#107) * Support state migration on upgrade. (#103) * Implement the ETH connector. (#59) * Move when to call `set_generation` * Fix arg Co-authored-by: Marcelo Fornet <mfornet94@gmail.com> Co-authored-by: Arto Bendiken <arto@aurora.dev> Co-authored-by: Michael Birch <michael@near.org> Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com> Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru>

* Add tests for state check after selfdestruct * Aurora runner tracks storage usage to avoid underflow when storage is released in future transactions (#85) * Implement generational storage (#87) * Base precompile code between connectors (#73) * Base precompile code between connectors * Handle errors and validate input * Use proper result * Document `aurora encode-address` usage. * Cache cargo artifacts between CI runs. (#92) * Address comments from audit. (#86) * Validate register length in `read_input_arr20()` * Only read register length in `Engine::get_code_size` * Add `read_input_borsh()` * Ensure `method.args.len() == args_decoded.len()` * Ensure register size is 8 in `read_u64` * Use constant to specify the register ID used in `read_input()` * Reduce size of `cargo cache` in CI. (#95) * Define a `Wei` newtype for balances. (#96) * Fix evm-bully builds after recent refactoring. (#100) * Refactor `Engine::get_state` to return a `Result`. (#99) * Ensure that `Cargo.lock` in the repo is valid. (#101) * Remove unneeded nightly feature. (#102) * Implement BC generational storage. * fix address input * remove note * put key on the end of the storage key * remove pub from methods * Dispatch precompiles on the full address. (#107) * Support state migration on upgrade. (#103) * Implement the ETH connector. (#59) * Move when to call `set_generation` * Fix arg Co-authored-by: Marcelo Fornet <mfornet94@gmail.com> Co-authored-by: Arto Bendiken <arto@aurora.dev> Co-authored-by: Michael Birch <michael@near.org> Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com> Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru> * Fix layout of the key * Fix all tests (don't wipe the storage all the time) * Use correct generation in writing storage * Remove unnecessary references Co-authored-by: Michael Birch <michael@near.org> Co-authored-by: Joshua J. Bouw <dev@joshuajbouw.com> Co-authored-by: Arto Bendiken <arto@aurora.dev> Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com> Co-authored-by: Evgeny Ukhanov <mrlsd@ya.ru> Co-authored-by: Michael Birch <michael.birch@aurora.dev>

Base precompile code between connectors

2a2faed

mfornet requested review from mrLSD and sept-en May 11, 2021 23:19

sept-en approved these changes May 11, 2021

View reviewed changes

mrLSD reviewed May 12, 2021

View reviewed changes

Handle errors and validate input

9c158fa

mfornet requested review from mrLSD and sept-en May 12, 2021 14:32

mfornet marked this pull request as ready for review May 12, 2021 14:32

mfornet requested a review from artob as a code owner May 12, 2021 14:32

mfornet commented May 12, 2021

View reviewed changes

mrLSD reviewed May 12, 2021

View reviewed changes

Use proper result

df15ae2

mfornet assigned artob May 12, 2021

artob added the C-enhancement Category: New feature or request label May 12, 2021

mfornet changed the base branch from master to develop May 12, 2021 22:22

mfornet merged commit 37c8334 into develop May 12, 2021

mfornet deleted the connectors branch May 12, 2021 22:22

joshuajbouw pushed a commit that referenced this pull request May 18, 2021

Base precompile code between connectors (#73)

80f71db

* Base precompile code between connectors * Handle errors and validate input * Use proper result

Conversation

mfornet commented May 11, 2021

Uh oh!

sept-en left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

mfornet May 12, 2021 • 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!

artob commented May 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mfornet May 12, 2021 •

edited

Loading