Wallet Contract placeholder

[staffik]

Context

NEP: near/NEPs#518
Tracking issue: #10018.

Goal

We want the NEAR Protocol's ecosystem to be closer with Ethereum by integrating Web3 wallet support.
To accomplish that, some changes on the protocol level are needed with an emphasis on user experience continuity.
Following the design, the main change on the protocol level would be to have ETH-style addresses managed by a new Wallet Contract integrated into the protocol. For seamless onboarding of EVM users, the contract will be free of charge.

Previous work

This PR is built on top of two PRs:

• Setup for ETH-implicit accounts #10020: no-op PR, laying groundwork for further changes.
• ETH-implicit account support #10224: empty contract as a placeholder, literally deployed to new ETH accounts.

Summary

This PR adds near-wallet-contract crate (based on runtime/near-test-contracts) that exposes Wallet Contract WASM code to the runtime.
It stops deploying a copy of the Wallet Contract to each newly created ETH-implicit account.
Instead, it uses an in-memory cached contract code on execute_function_call action from such account.

Changes

• Add wallet-contract crate (separated from the workspace) with placeholder Wallet Contract implementation.
• Add near-wallet-contract crate (part of the workspace) that generates (through build.rs) and exposes the Wallet Contract WASM code.
• Do not literally deploy Wallet Contract when creating ETH-implicit account. Just store reference to the Wallet Contract.
• Treat ETH-implicit accounts specially when retrieving contract code from an account (in such case returns in-memory cached Wallet Contract code).
• Add tests calling Wallet Contract where a transfer from an ETH-implicit account is possible depending on the public key passed with rlp_transaction argument.

[codecov]

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (abf6e99) 71.81% compared to head (f4a097c) 71.90%.
Report is 25 commits behind head on master.

Files	Patch %	Lines
runtime/near-wallet-contract/src/lib.rs	58.97%	16 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10269      +/-   ##
==========================================
+ Coverage   71.81%   71.90%   +0.08%     
==========================================
  Files         712      715       +3     
  Lines      143098   143755     +657     
  Branches   143098   143755     +657     
==========================================
+ Hits       102763   103361     +598     
- Misses      35625    35651      +26     
- Partials     4710     4743      +33     

Flag	Coverage Δ
backward-compatibility	0.08% <0.00%> (-0.01%)	⬇️
db-migration	0.08% <0.00%> (-0.01%)	⬇️
genesis-check	1.26% <0.00%> (-0.01%)	⬇️
integration-tests	36.35% <76.11%> (+0.17%)	⬆️
linux	71.49% <23.52%> (-0.22%)	⬇️
linux-nightly	71.47% <76.11%> (+0.08%)	⬆️
macos	55.24% <0.00%> (+0.23%)	⬆️
pytests	1.48% <0.00%> (-0.01%)	⬇️
sanity-checks	1.28% <0.00%> (-0.01%)	⬇️
unittests	68.17% <19.40%> (+<0.01%)	⬆️
upgradability	0.13% <0.00%> (-0.01%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[staffik]

I am not sure how Wallet Contract upgrade should be supported.
E.g. do we want to be able to dynamically switch the WASM file used?

[birchmd]

Nice job! I have a few comments/questions but overall looks good.

[birchmd]

I am not sure how Wallet Contract upgrade should be supported. E.g. do we want to be able to dynamically switch the WASM file used?

I do not think we want to support dynamic upgrades. It would be a complex feature to get right because you still need to have all the nodes agree on what the right Wallet Contract binary to use is, so you would need its hash to be a property of the epoch or something like that.

Upgrades will always have to be protocol upgrades. Both the old and new binaries will need to be kept and the node would need to switch which one is used depending on the protocol version.

[wacban]

Sorry for the delay, I was busy with investigations, I'll have a look on Monday.

[wacban]

Great stuff!

Please also have it reviewed by someone from the runtime team as there are a lot of pieces that I have absolutely no context on.

[staffik]

Here are the all review changes, mainly:

• Addressed @Ekleog-NEAR comments.
• Moved Wallet Contract tests to a separate file and added some tests.
• near[wallet contract hash] is deployed as a contract code to new ETH-implicit accounts.
• Wallet Contract is checked into the repository and it is included into the binary at compile time. By default it is not recompiled, one can recompile it with cargo build --features build_wallet_contract.

The solution of pulling the crate out of the workspace works fine for near-test-contracts, which we do not really care about. However, for wallet-contract, we definitely do care a lot about it being correct. Have you tried having it within the workspace, which would allow all our CI tooling to also apply to it, and thus all the related linters?

@Ekleog-NEAR I've built this PR with the wallet-contract crate being part of the workspace, and I liked the linter.
But after the PR was ready I removed the crate with this commit because introducing near-sdk dependency caused crazy changes to the workspace Cargo.lock and introduced vulnerable dependency.

I spent some time trying to fix that but without success. The crate was only used during the build and I thought I will separate it from the workspace. Now it is not even used during build by default.

[Ekleog-NEAR]

For the vulnerable dependencies, that’s something we definitely need to fix on the near-sdk side, because right now we’re pushing everyone to use these vulnerable dependencies.

For wee-alloc, the change recommended by the rustsec advisory is to just use Rust’s default allocator. For ed25519, bumping near-crypto & co to 0.18 (as soon as I can get it released, currently we’re facing a few issues with the release) should deal with the issue. I’d suggest fixing the wee-alloc issue, and ignoring the red cargo-audit checkmark for ed25519 for now, considering it’s dependencies that only get used for non-wasm32 targets.

Anyway, I think we can merge this as is for now, and merge the wallet contract into the workspace as a follow-up PR :)

I think the only comment here that really needs action before landing is, removing the prebuilt .wasm file, because it’s currently impossible to reproduce. It’ll make operational deployment harder, but if we check in a binary file, we definitely need it to be reproducible.

In order to keep operational deployment easy for now, I’d suggest compiling the include_bytes! conditionally on the nightly feature flag only (and eg. having a hardcoded empty slice on stable), this way we can gain some time to decide what the proper process should be there :)

[birchmd]

Looks good to me! We only need to address the issue of reproducible build for the contract, and I think that can be done as a follow-up PR.