Fix(xcc): Method in router contract for upgradability #866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aleksuss
approved these changes
Nov 16, 2023
mrLSD
approved these changes
Nov 16, 2023
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 17, 2023
## Description Since #750 has been merged, the ERC-20 connector is able to unwrap wrapped Near tokens into Near native tokens. This logic was duplicated by the XCC router, but in this PR we remove the logic from XCC and use the bridge's unwrapping instead. This has a nice side-effect of saving us 20 Tgas from the XCC overhead. ## Performance / NEAR gas cost considerations No impact on regular transactions, 20 Tgas improvement to XCC transactions. ## Testing Existing tests. Specifically this relies heavily on the upgrade test introduced in #866
aleksuss
pushed a commit
that referenced
this pull request
Nov 28, 2023
## Description The initial XCC implementation was meant to allow the contracts deployed to the sub-accounts created by the Engine as part of the XCC flow to be upgradable. However, that upgrade code did not work as intended. The problem is that Near enforces [only an account itself](https://github.com/near/nearcore/blob/83fe943e4e270db87a89535037d2b3a8909a2c6d/runtime/runtime/src/actions.rs#L852) can use the `DeployContract` action after the account has been created (if the account is being created during that receipt then `DeployContract` is allowed to be part of the batch). But the initial implementation attempted to push the `DeployContract` action directly, causing an `ActorNoPermission` error. The correct way to implement an upgrade mechanism is to have the deployed contract contain a method which accepts new code then creates a receipt to itself with the `DeployContract` action. This PR changes the XCC Router contract to include such a method. Unfortunately since V1 XCC Routers did not have any upgrade method, and they have no access keys associated with them, they can never be upgraded. After releasing the V2 Router contract with upgradability we may encourage existing XCC users (for example the fast bridge) to migrate to a new XCC account. ## Performance / NEAR gas cost considerations N/A ## Testing A new integration test for upgrading XCC contracts is included in this PR.
aleksuss
pushed a commit
that referenced
this pull request
Nov 28, 2023
## Description Since #750 has been merged, the ERC-20 connector is able to unwrap wrapped Near tokens into Near native tokens. This logic was duplicated by the XCC router, but in this PR we remove the logic from XCC and use the bridge's unwrapping instead. This has a nice side-effect of saving us 20 Tgas from the XCC overhead. ## Performance / NEAR gas cost considerations No impact on regular transactions, 20 Tgas improvement to XCC transactions. ## Testing Existing tests. Specifically this relies heavily on the upgrade test introduced in #866
Merged
aleksuss
added a commit
that referenced
this pull request
Nov 28, 2023
## [3.4.0] 2023-11-28 ### Additions - Added a possibility to pass initialize arguments in json format to the `new` transaction by [@aleksuss]. ([#871]) - The `SubmitResult` was made available for `ft_on_transfer` transactions in the standalone engine by [@birchmd]. ([#869]) - The order of producing the exit precompile and XCC promises has been changed to sequential by [@birchmd]. ([#868]) ### Changes - Removed the code hidden behind the feature that isn't used anymore by [@joshuajbouw]. ([#870]) - The logic of unwrapping wNEAR has been changed to the Bridge's native by [@birchmd]. ([#867]) - Bumped the `near-workspaces` to 0.9 by [@aleksuss]. ([#862]) ### Fixes - Add a method for upgrading XCC router contract by [@birchmd]. ([#866]) - Fixed a potential panic in the `ExitToNear` precompile by [@guidovranken]. ([#865]) - Fixed a behaviour when the `ft_transfer` could occur before the `near_withdraw` by [@birchmd]. ([#864]) - Fixed correctness of reproducing the NEAR runtime random value in the standalone engine by [@birchmd]. ([#863]) [#862]: #862 [#863]: #863 [#864]: #864 [#865]: #865 [#866]: #866 [#867]: #867 [#868]: #868 [#869]: #869 [#870]: #870 [#871]: #871 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Birch <[email protected]> Co-authored-by: Guido Vranken <[email protected]> Co-authored-by: Joshua J. Bouw <[email protected]>
aleksuss
added a commit
that referenced
this pull request
Nov 28, 2023
## [3.4.0] 2023-11-28 ### Additions - Added a possibility to pass initialize arguments in json format to the `new` transaction by [@aleksuss]. ([#871]) - The `SubmitResult` was made available for `ft_on_transfer` transactions in the standalone engine by [@birchmd]. ([#869]) - The order of producing the exit precompile and XCC promises has been changed to sequential by [@birchmd]. ([#868]) ### Changes - Removed the code hidden behind the feature that isn't used anymore by [@joshuajbouw]. ([#870]) - The logic of unwrapping wNEAR has been changed to the Bridge's native by [@birchmd]. ([#867]) - Bumped the `near-workspaces` to 0.9 by [@aleksuss]. ([#862]) ### Fixes - Add a method for upgrading XCC router contract by [@birchmd]. ([#866]) - Fixed a potential panic in the `ExitToNear` precompile by [@guidovranken]. ([#865]) - Fixed a behaviour when the `ft_transfer` could occur before the `near_withdraw` by [@birchmd]. ([#864]) - Fixed correctness of reproducing the NEAR runtime random value in the standalone engine by [@birchmd]. ([#863]) [#862]: #862 [#863]: #863 [#864]: #864 [#865]: #865 [#866]: #866 [#867]: #867 [#868]: #868 [#869]: #869 [#870]: #870 [#871]: #871 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Birch <[email protected]> Co-authored-by: Guido Vranken <[email protected]> Co-authored-by: Joshua J. Bouw <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 5, 2023
## Description In #866 I introduced a change to the XCC router where the contract became upgradable. In that PR I hardcoded that any XCC Router that was version 1 could not be upgraded, however this logic did not work on testnet where the XCC Router code had already been set twice, so it was actually version 3 which became the first upgradable version. In this PR I add a new key to the engine which stores the first upgradable XCC version so that it does not need to be hardcoded. This should allow the logic to work properly on both testnet and mainnet. ## Performance / NEAR gas cost considerations N/A ## Testing Existing tests + testnet deployment (manual test)
aleksuss
pushed a commit
that referenced
this pull request
Dec 6, 2023
## Description In #866 I introduced a change to the XCC router where the contract became upgradable. In that PR I hardcoded that any XCC Router that was version 1 could not be upgraded, however this logic did not work on testnet where the XCC Router code had already been set twice, so it was actually version 3 which became the first upgradable version. In this PR I add a new key to the engine which stores the first upgradable XCC version so that it does not need to be hardcoded. This should allow the logic to work properly on both testnet and mainnet. ## Performance / NEAR gas cost considerations N/A ## Testing Existing tests + testnet deployment (manual test)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The initial XCC implementation was meant to allow the contracts deployed to the sub-accounts created by the Engine as part of the XCC flow to be upgradable. However, that upgrade code did not work as intended. The problem is that Near enforces only an account itself can use the
DeployContractaction after the account has been created (if the account is being created during that receipt thenDeployContractis allowed to be part of the batch). But the initial implementation attempted to push theDeployContractaction directly, causing anActorNoPermissionerror.The correct way to implement an upgrade mechanism is to have the deployed contract contain a method which accepts new code then creates a receipt to itself with the
DeployContractaction. This PR changes the XCC Router contract to include such a method.Unfortunately since V1 XCC Routers did not have any upgrade method, and they have no access keys associated with them, they can never be upgraded. After releasing the V2 Router contract with upgradability we may encourage existing XCC users (for example the fast bridge) to migrate to a new XCC account.
Performance / NEAR gas cost considerations
N/A
Testing
A new integration test for upgrading XCC contracts is included in this PR.