nearcore icon indicating copy to clipboard operation
nearcore copied to clipboard

Feat: Eth wallet contract implementation

Open birchmd opened this issue 11 months ago • 2 comments

This PR adds the eth wallet contract implementation which will be automatically deployed with eth implicit accounts. This is as per the design outlined in https://github.com/near/NEPs/issues/518 .

It is intentional that the wallet contract implementation is isolated as its own crate and not contained in the broader nearcore workspace. This allows it to be reviewed, tested and audited independently from the rest of the Near code. The crate includes integration tests written using near-workspaces. However, the nearcore integration test related to the wallet contract has also been updated to check that the contract is automatically deployed when creating an eth address and that it works as expected.

This will not be the last PR in this project because I am only adding the logic for the implementation. It still remains to setup the reproducible build pipeline and update the tests which check the contract hashes. Additionally, there are still a few details which need to be finalized: (1) the Ethereum chain ID that will be associated with Near and (2) the Near account ID that will have the eth address registrar contract deployed. The eth address registrar contract stores a reverse lookup of Ethereum-like address to Near account IDs. It is necessary for the wallet contract to detect faulty relayers.

However this (relatively large) PR can be reviewed and merged, and the points above will be addressed in much more manageable follow-up PRs.

birchmd avatar Mar 21 '24 13:03 birchmd

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.39%. Comparing base (b2c82f9) to head (37175d3). Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10850      +/-   ##
==========================================
+ Coverage   71.37%   71.39%   +0.02%     
==========================================
  Files         760      760              
  Lines      152772   152772              
  Branches   152772   152772              
==========================================
+ Hits       109039   109075      +36     
+ Misses      39207    39172      -35     
+ Partials     4526     4525       -1     
Flag Coverage Δ
backward-compatibility 0.24% <ø> (ø)
db-migration 0.24% <ø> (ø)
genesis-check 1.43% <ø> (ø)
integration-tests 37.08% <ø> (+0.11%) :arrow_up:
linux 69.84% <ø> (+<0.01%) :arrow_up:
linux-nightly 70.86% <ø> (+0.02%) :arrow_up:
macos 54.41% <ø> (+<0.01%) :arrow_up:
pytests 1.65% <ø> (ø)
sanity-checks 1.44% <ø> (ø)
unittests 67.02% <ø> (+<0.01%) :arrow_up:
upgradability 0.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 21 '24 13:03 codecov[bot]

I started reviewing this but it might take a while. I plan to send feedback tomorrow.

staffik avatar Mar 25 '24 14:03 staffik

@staffik any updates on the review?

bowenwang1996 avatar Apr 01 '24 23:04 bowenwang1996

Thanks for the review @staffik ! I think I have addressed all your comments now.

the issue of checking-in the contract

I started working on this here. I was planning on pushing the commit in another PR after this one is merged.

birchmd avatar Apr 03 '24 15:04 birchmd

Thanks for the review @staffik ! I think I have addressed all your comments now.

the issue of checking-in the contract

I started working on this here. I was planning on pushing the commit in another PR after this one is merged.

There are 3 comments left and I will quickly look at test-related code now.

staffik avatar Apr 03 '24 17:04 staffik

Thanks for the review @staffik ! I think I have addressed all your comments now.

the issue of checking-in the contract

I started working on this here. I was planning on pushing the commit in another PR after this one is merged.

There are 3 comments left and I will quickly look at test-related code now.

Sorry, I underestimated, will finish tomorrow morning. We will need an approval from near code owner to be able to merge.

staffik avatar Apr 03 '24 22:04 staffik