near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.32k stars 623 forks source link

Fix(wallet-contract): Use fixed amount of gas for all callbacks #11968

Closed birchmd closed 2 months ago

birchmd commented 2 months ago

The purpose of this change is to fix a bug in the wallet contract where it was up to the caller to attach enough gas for all the subsequent receipts to be able to complete (the contract was simply passing on unused gas to the callbacks). However, the account can get into a stuck state if the final callback is not executed due to it running out of gas. The PR attaches a fixed amount of gas to all receipts upfront so that they are guaranteed to complete successfully when they are eventually executed by the protocol. An additional test is added to the wallet contract to cover the case where not enough gas is attached.

Since the wallet contracts feature is not yet live on mainnet, we are not making this change to the wallet contract a protocol change as we normally would. Instead we are simply changing the binary the runtime uses for the wallet contract. The old magic bytes hash that was uses to identify the wallet contract is still recognized by the new code so that existing testnet eth-implicit accounts will continue to function.

Note that changing the wallet contract implementation means that new versions of nearcore nodes will not execute some eth-implicit accounts in the same way as old versions of nearcore (breaking change for replaying prior transactions). However since this only impacts testnet and testnet nodes always sync from regularly generated snapshots, this should not be an issue.

To prevent the wallet contract issue from ever appearing in mainnet, this fix should be pulled into the 2.1.1 nearcore release.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.

Project coverage is 71.60%. Comparing base (6412f22) to head (d6822d6). Report is 4 commits behind head on master.

Files Patch % Lines
runtime/near-wallet-contract/src/lib.rs 92.68% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11968 +/- ## ========================================== + Coverage 71.58% 71.60% +0.01% ========================================== Files 808 808 Lines 163537 163588 +51 Branches 163537 163588 +51 ========================================== + Hits 117071 117132 +61 + Misses 41386 41378 -8 + Partials 5080 5078 -2 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.33% <0.00%> (-0.01%)` | :arrow_down: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `38.50% <18.60%> (+0.04%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.36% <93.02%> (-0.01%)` | :arrow_down: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.19% <93.02%> (+0.04%)` | :arrow_up: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `54.40% <88.37%> (+0.03%)` | :arrow_up: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.60% <0.00%> (-0.01%)` | :arrow_down: | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.40% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `65.67% <90.69%> (+<0.01%)` | :arrow_up: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

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