Feat(engine): Cross contract calls #560

mfornet · 2022-07-25T13:02:07Z

Enable access to trigger cross contract call from addresses internal to Aurora, to NEAR Account Ids.

AIP link: aurora-is-near/AIPs#2
Forum discussion: https://forum.aurora.dev/t/cross-contract-calls/304

Notes on the design

Cross contract calls can be triggered by any aurora address calling a particular precompile. When the precompile is called, a new proxy NEAR contract will be deployed (only the first time) and the contract call will be routed through this account.

There is a mechanism on the engine that allows upgrading the bytecode of the proxy. The proxy will be upgraded automatically if there is an update before routing the call.

engine/src/lib.rs

mfornet

Everything looks good! Already reviewed what is in the code, I'll do a future review about what could be potentially missing from it.

I can't approve / reject this PR as I created it 🤦 . Let's discuss the comments I made, and then I'll approve in a comment.

engine/src/xcc.rs

etc/xcc-router/src/lib.rs

engine/src/xcc.rs

engine/src/lib.rs

XCC.md

engine-precompiles/src/xcc.rs

joshuajbouw

All in all looks good. I don't have any issues per se with the logic itself. I want to go once again when we reach a final review and try to come up with scenarios on how we could break this. I have not done a security analysis of this yet.

engine-precompiles/src/xcc.rs

engine/src/xcc.rs

joshuajbouw · 2022-08-08T17:27:39Z

This is blocked until #565 is in, correct?

engine-sdk/Cargo.toml

mfornet · 2022-08-18T14:38:09Z

Approve ✅

(I can't approve on GitHub interface, as I initiated the PR)

joshuajbouw · 2022-08-18T14:59:23Z

Approve ✅

(I can't approve on GitHub interface, as I initiated the PR)

We need to get better at not contributing to others' work directly, but via PR me thinks :P. It is awkward you can't exactly review your own PR in this case.

joshuajbouw · 2022-08-18T16:15:56Z

engine-precompiles/src/native.rs

@@ -30,12 +30,10 @@ mod costs {
    // TODO(#483): Determine the correct amount of gas
    pub(super) const EXIT_TO_ETHEREUM_GAS: EthGas = EthGas::new(0);

-    // TODO(#332): Determine the correct amount of gas
-    pub(super) const FT_TRANSFER_GAS: NearGas = NearGas::new(100_000_000_000_000);
+    pub(super) const FT_TRANSFER_GAS: NearGas = NearGas::new(10_000_000_000_000);


I don't want to block this PR, but I believe the rationale for this value should be attributed. Same for the one blow.

joshuajbouw · 2022-08-18T16:23:09Z

engine-types/src/parameters.rs

+    /// For example, if an expensive EVM call ends with a NEAR cross contract call, then there may not be
+    /// much gas left to perform it. In this case, the promise could be `Delayed` (stored in the router)
+    /// and executed in a separate transaction with a fresh 300 Tgas available for it.
+    Delayed(PromiseArgs),


What if these are never consumed? Could we have some process to free these up if they are old? Or should we?

Ultimately it is the user that has to cover the storage staking for stored promises they never used. I don't know why they would not use the promises they have scheduled. We could add a cancellation interface, but this adds more complexity to the design, and I do not think it is worth having that complexity for this edge case that may never come up.

Also this bring the question: who can destroy it and when? probably the answer is the same user that scheduled it, but is still a bit pointless! Agree on leaving it there until it is consumed!

joshuajbouw · 2022-08-18T16:25:13Z

engine/src/xcc.rs

+pub const INITIALIZE_GAS: NearGas = NearGas::new(15_000_000_000_000);
+pub const UNWRAP_AND_REFUND_GAS: NearGas = NearGas::new(25_000_000_000_000);
+pub const WITHDRAW_GAS: NearGas = NearGas::new(30_000_000_000_000);
+pub const WITHDRAW_TO_NEAR_SELECTOR: [u8; 4] = [0x6b, 0x35, 0x18, 0x48];


This should explain what the 4 bytes are meant to represent.

joshuajbouw · 2022-08-18T16:25:27Z

engine/src/xcc.rs

+pub const VERSION_UPDATE_GAS: NearGas = NearGas::new(5_000_000_000_000);
+pub const INITIALIZE_GAS: NearGas = NearGas::new(15_000_000_000_000);
+pub const UNWRAP_AND_REFUND_GAS: NearGas = NearGas::new(25_000_000_000_000);
+pub const WITHDRAW_GAS: NearGas = NearGas::new(30_000_000_000_000);


These gas costs should be explained.

joshuajbouw · 2022-08-18T16:27:25Z

etc/xcc-router/src/lib.rs

+const WNEAR_WITHDRAW_GAS: Gas = Gas(5_000_000_000_000);
+const WNEAR_REGISTER_GAS: Gas = Gas(5_000_000_000_000);
+const REFUND_GAS: Gas = Gas(5_000_000_000_000);
+const WNEAR_REGISTER_AMOUNT: u128 = 1_250_000_000_000_000_000_000;
+/// Must match arora_engine_precompiles::xcc::state::STORAGE_AMOUNT
+const REFUND_AMOUNT: u128 = 2_000_000_000_000_000_000_000_000;


These gas costs should be explained.

joshuajbouw

My only real complaint is that there is a lot of lacking documentation that will not only help ourselves, but also others and especially the auditors.

joshuajbouw

Sorry, forgot to say that we require all the engine/src/lib.rs new functions should also be feature gated.

engine/src/lib.rs

joshuajbouw

It's fine, @birchmd had informed me that this should not be an issue in the end, and I do agree.

…ess_version

…ile gas estimate tests

birchmd · 2022-08-18T18:06:13Z

My only real complaint is that there is a lot of lacking documentation that will not only help ourselves, but also others and especially the auditors.

I'll address requests for more documentation in a follow-up PR.

@mooori

Feat(tracing): Implement callTracer (#538) Add docker as a prerequisites (#542) This was found by @mooori while running tests for first time. Update dependencies (#541) Update etc/self-contained-5bEgfRQ Decrease gas usage for some tests Merge: 2.6.1 into `develop` from `master` (#546) * Feat(standalone): Add debug tracing of remaining gas values (#391) * Balance type refactoring (#386) * Remove standalone binary (#403) * Eth Address cleanup (#387) * Fix(dep) Pass std feature on to rjson library (#402) * Feat(benchmarks): update gas bounds after wasm cost reduction (#406) * Feat(tests): Mock touching trie nodes (#408) * Fix: Only allow owner to call deploy_upgrade (#410) * Feat(tests): Uniswap multi-hop swap benchmark (#415) * Fix(engine): Return revert error message during contract deploy (#424) * Move engine transactions module to engine-transactions crate (#418) * Avoid using wasmer directly (#420) * Fix compilation after #418 merge * Use default git attributes for Cargo.lock (#421) * Switch to rebranded blake2 (#422) * Remove unused unused (#423) * Chore: make aurora compatible with latest nightlies (#425) * Chore: upgrade to the recent nightly (#426) * chore: upgrade to the latest nearcore version (#427) * chore: move to 2021 edition (#428) * Benchmark contract performing only pure arithmetic and memory operations (#429) * Fix(engine): Require chain_id (#432) * chore: make sure aurora is using the latest cost config (#435) * Fix(test): make uniswap benchmarks reproducible (#437) * Fix(test): lower gas limit on OOG test; this is needed because the solidity compiler got more efficient, not because of a regression in EVM gas metering correctness * Fix(engine): Cache generation values to avoid repeated state reads (#438) * Fix(engine): Optimize is_account_empty (#439) * Fix(test): update uniswap wasm fraction; this is needed because optimizations from #438 and #439 together lowered the fraction of gas spent on IO * Fix(engine): Upgrade to latest SputnikVM (#445) * Fix(engine): Simple cache to stop consecutive duplicate reads (#446) * Version update and change log Co-authored-by: Aleksey Kladov <[email protected]> Co-authored-by: Evgeny Ukhanov <[email protected]> Co-authored-by: Michael Birch <[email protected]> Release 2.5.0. (#460) * Engine: Upgrade to latest SputnikVM changes (#455) * Engine: further caching optimizations (#456) * Use EthTransactionKind reference to serialize into bytes (#457) * Test: Reproduce GdASJ3KESs8VegpFECTveCwLQp8fxw8yvsauNEmGb6pZ gas failure (#454) * Fix typo: promise_create (#452) * Fix(Engine): Transactions to the zero address are not the same as transactions with empty to field (#458) Co-authored-by: Marcelo Fornet <[email protected]> Co-authored-by: Michael Birch <[email protected]> Release 2.5.1. (#470) * Feat(Engine): Precompiles for predecessor_account_id and current_account_id (#462) * Implement a NEAR native version of the Solidity pure arithmetic benchmark to compare EVM execution cost with direct wasm cost (#463) * Fix features related to panic_info_message (#466) * Fix(standalone): set predecessor_account_id appropriately when executing promise callbacks (#467) * Fix(engine): fix bug in checking if an address exists (#469) * Version update * Update unreleased link Co-authored-by: Joshua J. Bouw <[email protected]> Release 2.5.2. (#475) * Revert "Feat(Engine): Precompiles for predecessor_account_id and current_account_id (#462)" Release 2.5.3. (#527) * Fix(precompile): ExitToNear ExitToEthereum vulnerability patch Fix vulnerability Include exploit contract * Release 2.5.3 notes * Update solidity version Co-authored-by: Michael Birch <[email protected]> Release 2.6.0. (#494) * Feat(Engine): Precompiles for predecessor_account_id and current_account_id (#462) * Fix(precompiles): Allow native precompiles to work in the standalone engine (#473) * Standalone engine improvements (#478) * Fix(Engine): Predecessor id precompile works in view calls (#477) * Feat(standalone): Persist transaction data (#481) * Feat(Engine): Add custom precompile for NEAR prepaid_gas (#479) * Test: Reproduce 8ru7VEAEbyfZdbC1W2PYQv2cY3W92rbTToDEN4yTp8aZ gas failure (#485) * Feat(standalone): Function to get latest/earliest block (#482) * Engine optimization: cache all reads from NEAR state (#488) * Feat(standalone): Handle call to new method of Engine contract (#490) * Feat(engine-types): optional serde integration (#468) * Changed near-blake2 dependency (#484) * Feat(standalone): thread-safety and serde (#496) * Refactor repro tests to have less boilerplate (#497) * Add serde for TraceTransaction (#495) * Fix(standalone): include possible execution errors in the TransactionIncludedOutcome result field (#500) * Reproduce transaction FRcorNvFojoxBrdiVMTy9gRD3H8EYXXKau4feevMZmFV (#498) * Reproduce transaction 5bEgfRQ5TSJfN9XCqYkMr9cgBLToM7JmS1bNzKpDXJhT (#499) * Fix(connector): Return an error when storage cannot be read instead of panicking (#501) * Standalone: forward underlying rocksdb crate features (#502) * Feat(tests): Multisender benchmark (#503) * Feat(test): Reproduce D98vwmi44hAYs8KtX5aLne1zEkj3MUss42e5SkG2a4SC (#504) * Standalone engine storage saves the AccountId of the associated engine deployed on chain (#510) * Fix(precompiles): Fix secp256k1 run not returning empty slice on an incorrect V byte (#513) * Self-contained wasm contract for testing 5bEgfRQ (#514) * Update gas estimation code to use new data from NEAR protocol v53 (#517) * Fix: Legacy transactions must be allowed (#520) * Chore(precompiles): Include blake2 compression only (#528) * Build(deps): Upgrade `libsecp256k1` version 0.3.5 => 0.7.0 (#515) * Bump regex from 1.5.4 to 1.5.6 (#526) * Bump regex from 1.5.4 to 1.5.6 in /etc/ft-receiver (#525) * Bump zeroize_derive from 1.1.0 to 1.3.2 (#523) * Bump crossbeam-deque from 0.8.0 to 0.8.1 (#521) * Bump crossbeam-utils from 0.8.4 to 0.8.8 (#524) * Bump cross-fetch from 2.2.3 to 2.2.6 in /etc/eth-contracts (#508) * Bump simple-get from 2.8.1 to 2.8.2 in /etc/eth-contracts (#507) * Bump follow-redirects from 1.14.0 to 1.15.1 in /etc/eth-contracts (#529) * Bump shelljs from 0.8.4 to 0.8.5 in /etc/eth-contracts * Fix(ecrecover): Set malleability flag to `0` for ecrecover. (#474) Co-authored-by: Evgeny Ukhanov <[email protected]> Co-authored-by: Marcelo Fornet <[email protected]> Co-authored-by: Michael Birch <[email protected]> Co-authored-by: Roman Hodulák <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Release 2.6.1. (#537) * Fix: Don't allow fee stealing. * Fix: Don't allow bridge receipt forging. * Fix(engine): Correctly account for changes in total supply of ETH on Aurora (#536) Co-authored-by: Michael Birch <[email protected]> Co-authored-by: Michael Birch <[email protected]> Chore: Update js dependencies (#543) # Update JS dependencies A number of the JS dependencies were out of date and require to be updated. While a number of these were used almost exclusively for testing, the EVM ERC 20 contract is obviously used in production. Over the last year, we had carefully monitored to ensure that we have not been allowing security-related vulnerabilities into our production contracts. However updating these contracts is a good thing to do, and should be actively updated going forward as we do see slight performance gains in doing so. As a small bonus, this also introduces a force of updating the docker image used to compile some of the test / benchmark contracts. ## Related Issues There are no known related issues. ## Risk Analysis The EvmErc20 and EvmErc20V2 contracts have been recompiled due to the nature of these changes as the compiler has been updated. However, this is expected and intended. ## Testing None are required. Constant error messages (#535) * Added errors module and changed json errors * Set engine module errors as consts * Set connector module errors as consts * deposit_event const errors * extend const errors msgs chore: Cargo make integration (#378) Make files can be a bit messy and hard to read through the file. Luckily, a rust alternative exists which is quite clean and accomplishes the same goals in a Rust-friendly manner. What this accomplishes is a replace of `make` with `cargo make`. Why this is important is because we are able to get a cleaner make file that can have even better control with the use of profiles. That would make it quite difficult to run the run deployments on the wrong binaries as it'll correctly read the appropriate ENV file. On top of this, I had reworked the Github action workflows and I am getting no failures now. Which is good, less time spent with kickstarting it to work again. This also includes a monthly "sweep" to keep our footprint smaller. In the future, this can be improved by dropping `default_to_workspace = false` and adding a Makefile.toml to every single crate in our workspace. Do note, for whatever reason there was an efficiency being made on the `profile_erc20_get_balance()` test with this compilation which doesn't make itself immediately obvious. I have tried to figure out why this is but can not come up with a reason. I request reviewers to please try to notice why this is. This is low priority, and I have only been working on this while debugging precompiles. Chore: PR template (#548)  ## Description This adds a PR template to ensure that PRs are of the highest quality by providing guidelines and a helpful checklist. ## How should this be reviewed Ensure that everything that could be said has been said in this template. Do comment below if we should add any more particulars. ## Additional information This is based on a number of other templates that other projects have adopted (mainly Rust projects) but also heavily modified and have included particulars to the project itself. Chore: Delete binaries (#553) Chore(engine): Update to latest version of SputnikVM (#565) Chore(CI): Update checkout to v3 (#566) Chore: Disallow as_conversions (#561) Fix feature gated import in hash precompile Change missed featured `into` to `try_into` birchmd fixes Cleaner usize conversion error Fix missing vec in hash Fix tests Fix V check Fix testnet env EVM account Change hex feature from alloc to std Improve u64 to u8 conversion in JSON Cargo fmt Chore: Update Rust crypto libraries (#568) Revert ripemd160, as the lib is deprecated Fix(engine-transactions): Prevent panic on empty input (#573) Feat(engine): Get promise results precompile (#575) Fix(engine): Return correct value for get_bridge_prover function (#581) Refactor(Tests): Move test contracts to test etc folder (#577) Co-authored-by: Michael Birch <[email protected]> Chore: Cleanup dead dependencies (#571) Chore(Precompiles): Ripemd160 lib deprecated (#570) Feat(precompiles): Use near host functions for alt_bn128 when compiling wasm artifact (#540) Co-authored-by: Joshua J. Bouw <[email protected]> Feat(engine): Cross contract calls (#560) Chore(eth-contracts): Update openzeppelin to 4.7.3 (#584) Fix: Make binaries on push (#585) 2.7.0 release prep

Feat(tracing): Implement callTracer (#538) Add docker as a prerequisites (#542) Update dependencies (#541) Chore: Update js dependencies (#543) Constant error messages (#535) chore: Cargo make integration (#378) Chore: PR template (#548) Chore: Delete binaries (#553) Chore(engine): Update to latest version of SputnikVM (#565) Chore(CI): Update checkout to v3 (#566) Chore: Disallow as_conversions (#561) Chore: Update Rust crypto libraries (#568) Fix(engine-transactions): Prevent panic on empty input (#573) Feat(engine): Get promise results precompile (#575) Fix(engine): Return correct value for get_bridge_prover function (#581) Refactor(Tests): Move test contracts to test etc folder (#577) Co-authored-by: Michael Birch <[email protected]> Chore: Cleanup dead dependencies (#571) Chore(Precompiles): Ripemd160 lib deprecated (#570) Feat(precompiles): Use near host functions for alt_bn128 when compiling wasm artifact (#540) Co-authored-by: Joshua J. Bouw <[email protected]> Feat(engine): Cross contract calls (#560) Chore(eth-contracts): Update openzeppelin to 4.7.3 (#584) Fix: Make binaries on push (#585) 2.7.0 release prep

mfornet requested a review from birchmd July 25, 2022 13:02

birchmd force-pushed the xcc branch from 1951b72 to 9bb550e Compare July 27, 2022 14:23

birchmd changed the base branch from master to develop July 27, 2022 14:23

birchmd reviewed Jul 27, 2022

View reviewed changes

engine/src/lib.rs Outdated Show resolved Hide resolved

birchmd changed the title ~~WIP: Precompile for cross contract call~~ Feat(engine): Cross contract calls Aug 1, 2022

birchmd marked this pull request as ready for review August 1, 2022 20:12

birchmd requested a review from joshuajbouw as a code owner August 1, 2022 20:12

mfornet commented Aug 2, 2022

View reviewed changes

joshuajbouw suggested changes Aug 3, 2022

View reviewed changes

engine-precompiles/src/xcc.rs Outdated Show resolved Hide resolved

engine-precompiles/src/xcc.rs Outdated Show resolved Hide resolved

engine-precompiles/src/xcc.rs Outdated Show resolved Hide resolved

engine/src/xcc.rs Show resolved Hide resolved

birchmd mentioned this pull request Aug 4, 2022

Feat(xcc): Estimate EVM gas cost for xcc precompile #564

Merged

joshuajbouw suggested changes Aug 9, 2022

View reviewed changes

engine-sdk/Cargo.toml Outdated Show resolved Hide resolved

birchmd force-pushed the xcc branch from 5fa6694 to 2d91055 Compare August 10, 2022 13:42

birchmd mentioned this pull request Aug 10, 2022

Feat(xcc): User payment using wNEAR ERC-20 #572

Merged

birchmd force-pushed the xcc branch from be232fd to 77b3b4a Compare August 17, 2022 13:10

birchmd requested a review from joshuajbouw August 17, 2022 13:51

joshuajbouw mentioned this pull request Aug 18, 2022

Refactor(Tests): Move test contracts to test etc folder #577

Merged

joshuajbouw reviewed Aug 18, 2022

View reviewed changes

joshuajbouw approved these changes Aug 18, 2022

View reviewed changes

joshuajbouw suggested changes Aug 18, 2022

View reviewed changes

engine/src/lib.rs Show resolved Hide resolved

joshuajbouw approved these changes Aug 18, 2022

View reviewed changes

Marcelo Fornet and others added 3 commits August 18, 2022 19:42

Add cross contract call support in the engine

66eba13

Fix compilation

0f0092e

Add some implementation details to the cross contract call precompile

169fb9e

birchmd added 18 commits August 18, 2022 19:45

Fix typo

f24def8

Verify target_account is of the form {address}.{aurora}

13d2533

Set current xcc-router version to 1

94a5241

Set xcc router storage amount to 4 NEAR

1234b91

Check xcc precompile does not create promises with attached NEAR

60cc71b

Standalone implementations for factory_update and factory_update_addr…

6271db3

…ess_version

Remove XCC.md

1869894

Remove TODO, borsh encoding is fine

4e76bda

Add PromiseArgs::total_gas function

0a86302

Reduce xcc-router storage cost to 2 NEAR

21df808

Move error messages to mod consts

c28a33c

Feat(xcc): Estimate EVM gas cost for xcc precompile (#564)

cc2b9b7

Post-rebase fixes

f431f5f

Specify borsh 0.9.3

a0bf051

Feat(xcc): User payment using wNEAR ERC-20 (#572)

1d51597

Factor out within_x_percent to test_utils to reuse it in both precomp…

8643c3d

…ile gas estimate tests

Do not enable the xcc precompile on mainnet yet

0a9121c

Post-rebase fixes

c2ec976

birchmd force-pushed the xcc branch from 9d4c9b2 to c2ec976 Compare August 18, 2022 17:53

birchmd merged commit 7dea1c3 into develop Aug 18, 2022

birchmd deleted the xcc branch August 18, 2022 18:05

joshuajbouw pushed a commit that referenced this pull request Aug 18, 2022

Feat(engine): Cross contract calls (#560)

43f5da6

birchmd pushed a commit that referenced this pull request Aug 18, 2022

Feat(engine): Cross contract calls (#560)

1fb194b

joshuajbouw pushed a commit that referenced this pull request Aug 18, 2022

Feat(engine): Cross contract calls (#560)

1fb4d3d

joshuajbouw mentioned this pull request Aug 18, 2022

Release 2.7.0. #587

Merged

birchmd mentioned this pull request Aug 19, 2022

Chore(docs): Additional documentation for xcc gas values #590

Merged

birchmd mentioned this pull request Sep 7, 2022

Implement AsyncAurora contract #489

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(engine): Cross contract calls #560

Feat(engine): Cross contract calls #560

mfornet commented Jul 25, 2022 •

edited by birchmd

Loading

mfornet left a comment •

edited

Loading

joshuajbouw left a comment

joshuajbouw commented Aug 8, 2022

mfornet commented Aug 18, 2022

joshuajbouw commented Aug 18, 2022

joshuajbouw Aug 18, 2022 •

edited

Loading

birchmd Aug 19, 2022

joshuajbouw Aug 18, 2022

birchmd Aug 19, 2022

mfornet Aug 22, 2022

joshuajbouw Aug 18, 2022

joshuajbouw Aug 18, 2022

joshuajbouw Aug 18, 2022

joshuajbouw left a comment

joshuajbouw left a comment

joshuajbouw left a comment

birchmd commented Aug 18, 2022

Feat(engine): Cross contract calls #560

Feat(engine): Cross contract calls #560

Conversation

mfornet commented Jul 25, 2022 • edited by birchmd Loading

Notes on the design

mfornet left a comment • edited Loading

Choose a reason for hiding this comment

joshuajbouw left a comment

Choose a reason for hiding this comment

joshuajbouw commented Aug 8, 2022

mfornet commented Aug 18, 2022

joshuajbouw commented Aug 18, 2022

joshuajbouw Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

birchmd Aug 19, 2022

Choose a reason for hiding this comment

joshuajbouw Aug 18, 2022

Choose a reason for hiding this comment

birchmd Aug 19, 2022

Choose a reason for hiding this comment

mfornet Aug 22, 2022

Choose a reason for hiding this comment

joshuajbouw Aug 18, 2022

Choose a reason for hiding this comment

joshuajbouw Aug 18, 2022

Choose a reason for hiding this comment

joshuajbouw Aug 18, 2022

Choose a reason for hiding this comment

joshuajbouw left a comment

Choose a reason for hiding this comment

joshuajbouw left a comment

Choose a reason for hiding this comment

joshuajbouw left a comment

Choose a reason for hiding this comment

birchmd commented Aug 18, 2022

mfornet commented Jul 25, 2022 •

edited by birchmd

Loading

mfornet left a comment •

edited

Loading

joshuajbouw Aug 18, 2022 •

edited

Loading