Skip to content

Introduce precompiles to manage ERC20 connector#51

Merged
artob merged 64 commits into
developfrom
erc20-precompiles
Jun 17, 2021
Merged

Introduce precompiles to manage ERC20 connector#51
artob merged 64 commits into
developfrom
erc20-precompiles

Conversation

@mfornet
Copy link
Copy Markdown
Contributor

@mfornet mfornet commented May 4, 2021

This creates two new precompiles:

  • exit_to_near
  • exit_to_ethereum

Their main role is scheduling a promise call outside of the EVM to another Near contract.
Check this diagram for reference.

Testing Plan

This are a set of basic tests that should be implemented before merging. This tests only include happy path for expected workflows, but it is important to cover with tests that other interaction are behaving as expected (tokens can't get lost or stolen).

  • (Rust Test) Can deploy ERC20 tokens from the EVM
  • (Rust Test) Can mint some tokens from admin access function
  • (Rust Test) ft_on_transfer from ERC20
  • (Rust Test) Register relayer (and check fee is ok from ft_on_transfer)
  • (Rust Test) Check that relayer gets payed
  • (Rust Test) Exit to Near
  • (Rust Test) Exit to Ethereum

Integration tests should spinup Ganache, and Localnet and run there, to make sure each workflow is behaving correctly in a environment closer to production.

  • (Integration Manual Test) Send tokens from Near -> EVM
  • (Integration Manual Test) Send tokens from EVM -> Near
  • (Integration Manual Test) Send tokens from Ethereum -> EVM
  • (Integration Manual Test) Send tokens from EVM -> Ethereum

Test contract_call is only scheduled appropriately:

Create a solidity with these functions:

  • A -> Call the precompile (which in turns schedule a contract_call on success)
  • B -> Call A as an external function and run require(false)
  • C -> Call B and catch the error (so the function doesn’t fail)
  • D -> Call C and Call A

Scenarios:

  • Call A and check contract call was emitted
  • Call B and check no contract call was emitted and tx failed
  • Call C and check no contract call was emitted and tx succeeded
  • Call D and check that ONLY one contract call was emitted and tx succeded
  • Previos set of tests should be executed with ExitToNear and ExitToEthereum

@mfornet mfornet requested review from artob, frankbraun, joshuajbouw and sept-en and removed request for frankbraun May 4, 2021 12:27
@artob artob added the C-enhancement Category: New feature or request label May 4, 2021
@artob artob requested a review from birchmd May 4, 2021 12:51
Copy link
Copy Markdown
Contributor

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small changes, and also to have it fit with the other precompiles. I believe it also needs to be added to the other precompile functions.

Comment thread src/parameters.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/precompiles.rs Outdated
Comment thread src/types.rs
@artob artob requested a review from mrLSD May 4, 2021 13:18
@sept-en sept-en force-pushed the erc20-precompiles branch from a309fce to 55e5457 Compare May 4, 2021 13:35
@sept-en
Copy link
Copy Markdown
Contributor

sept-en commented May 4, 2021

@joshuajbouw thanks for your help, LGTM!
@mfornet please take a look at the updated code and approve if everything looks good for you as well.

@mfornet mfornet requested review from joshuajbouw and sept-en May 4, 2021 15:35
@mfornet mfornet marked this pull request as draft May 4, 2021 15:45
Comment thread src/engine.rs Outdated
mfornet added a commit that referenced this pull request May 6, 2021
@mfornet mfornet force-pushed the erc20-precompiles branch from 98bce50 to ecc2b36 Compare May 6, 2021 21:24
mfornet added a commit that referenced this pull request May 6, 2021
* Add common code between #51 and #59

* Address comments

* Avoid writing const prefix to the storage

* Nit
sept-en and others added 9 commits May 6, 2021 23:40
* Add custom erc20 contract to the EVM.

To compile the EVM it is required to compile ERC20 first if this change
is used. Do this using the following steps:

cd eth-contracts
yarn
yarn compile

This will generate relevant binary in:
`eth-contracts/res/EvmErc20.bin`

* Move eth contract to etc and update Makefile

* Fix CI

* Add lint
@joshuajbouw
Copy link
Copy Markdown
Contributor

joshuajbouw commented Jun 10, 2021

I feel uncertain why we need an AuroraStackState, and why the precompiles now need the state implemented? I almost went down this path, but found solutions around not having to.

The reason is because we need to know what promises were asked to be created (from calls to exit precompiles), but we can't have the precompiles eagerly create the promises because then they cannot be rolled back if the transaction (or sub-call in a transaction) reverts.

The AuroraStackState keeps track of the data needed to construct the promises and only once the entire transaction is finished do we actually create them.

Right, I get the context now. I just saw this PR rust-ethereum/evm#34 as well. Makes sense. The design still feels a little off to me, but this is critical for our deliverable for the end of the week, so let's just roll with how it is for now. Eventually, maybe something will come up where it can be improved.

@mfornet mfornet requested a review from joshuajbouw June 10, 2021 18:37
Comment thread src/precompiles/native.rs Outdated
Comment thread src/precompiles/native.rs Outdated
@artob
Copy link
Copy Markdown
Contributor

artob commented Jun 11, 2021

@mfornet Please fix the merge conflicts and re-request reviews as needed.

@birchmd birchmd mentioned this pull request Jun 15, 2021
Comment thread src/json.rs
Comment thread src/lib.rs Outdated
@mfornet mfornet requested a review from sept-en June 17, 2021 00:59
Copy link
Copy Markdown
Contributor

@artob artob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfornet Please note you have a lot of Clippy warnings and address those in a follow-up PR.

@artob artob merged commit cbb878f into develop Jun 17, 2021
@artob artob deleted the erc20-precompiles branch June 17, 2021 08:44
Copy link
Copy Markdown
Contributor

@sept-en sept-en left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great job!

There are some minor comments that I think need to be addressed and resolved.

Comment thread src/engine.rs
Comment thread src/lib.rs
Comment thread src/deposit_event.rs
artob added a commit that referenced this pull request Jun 17, 2021
* Introduce precompiles for the ETH & ERC-20 connectors. (#51)
* Implement generational storage with `SELFDESTRUCT` tests. (#84)
* Remove the dependency on Lunarity. (#115)
* Fix Clippy complaint with `+nightly`. (#117)
* Simplify the `sdk::read_u64` return type. (#118)
* Add an `is_used_proof` interface. (#120)
* Add an `evm-bully=yes` build to CI. (#121)
* Handle transaction gas limit properly. (#123)
* Fix u128 JSON parsing & tests in the ETH connector. (#125)
* Fix evm-bully builds. (#130)
* Add JSON custom error types. (#131)
* Don't burn NEP-141 on deposit. (#133)
* Fix needless borrows. (#135)
* Improve and refactor the ETH connector. (#136)
* Add a macro for logging. (#142)

Co-authored-by: Aleksey Kladov <aleksey@near.org>
Co-authored-by: Arto Bendiken <arto@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev>
Co-authored-by: Frank Braun <frank@aurora.dev>
Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>
Co-authored-by: Kirill <kirill@aurora.dev>
Co-authored-by: Marcelo Fornet <marcelo@aurora.dev>
Co-authored-by: Michael Birch <michael@aurora.dev>
artob added a commit that referenced this pull request Jun 17, 2021
* Introduce precompiles for the ETH & ERC-20 connectors. (#51)
* Implement generational storage with `SELFDESTRUCT` tests. (#84)
* Fix u128 JSON parsing & tests in the ETH connector. (#125)
* Add JSON custom error types. (#131)
* Don't burn NEP-141 on deposit. (#133)
* Fix needless borrows. (#135)
* Improve and refactor the ETH connector. (#136)
* Add a macro for logging. (#142)

Co-authored-by: Aleksey Kladov <aleksey@near.org>
Co-authored-by: Arto Bendiken <arto@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev>
Co-authored-by: Frank Braun <frank@aurora.dev>
Co-authored-by: Joshua J. Bouw <joshua@aurora.dev>
Co-authored-by: Kirill <kirill@aurora.dev>
Co-authored-by: Marcelo Fornet <marcelo@aurora.dev>
Co-authored-by: Michael Birch <michael@aurora.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-enhancement Category: New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants