Express relay upgradable (#40)

* make express relay upgradable * add setter for express relay in opportunity adapter * corrected tests and deploy scripts * some better logging * some fixes to make tilt tests work * bump to 0.3.0 --------- Co-authored-by: --systemdf <[email protected]>
pyth-network · Mar 31, 2024 · 722c481 · 722c481
1 parent d52e62f
commit 722c481
Show file tree

Hide file tree

Showing 12 changed files with 275 additions and 62 deletions.
diff --git a/auction-server/Cargo.lock b/auction-server/Cargo.lock
diff --git a/auction-server/Cargo.toml b/auction-server/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "auction-server"
-version = "0.2.3"
+version = "0.3.0"
 edition = "2021"
 license-file = "license.txt"
 

diff --git a/per_multicall/script/Vault.s.sol b/per_multicall/script/Vault.s.sol
@@ -25,6 +25,7 @@ import "openzeppelin-contracts/contracts/utils/Strings.sol";
 
 import "../src/Errors.sol";
 import {OpportunityAdapterUpgradable} from "../src/OpportunityAdapterUpgradable.sol";
+import {ExpressRelayUpgradable} from "../src/ExpressRelayUpgradable.sol";
 
 contract VaultScript is Script {
     string public latestEnvironmentPath = "latestEnvironment.json";
@@ -64,6 +65,14 @@ contract VaultScript is Script {
             );
         opportunityAdapter.initialize(owner, admin, expressRelay, wethAddress);
         vm.stopBroadcast();
+        console.log(
+            "deployed OpportunityAdapterUpgradable implementation contract at",
+            address(_opportunityAdapter)
+        );
+        console.log(
+            "OpportunityAdapterUpgradeable proxy at",
+            address(opportunityAdapter)
+        );
         return address(opportunityAdapter);
     }
 
@@ -80,40 +89,66 @@ contract VaultScript is Script {
         vm.stopBroadcast();
     }
 
-    function deployExpressRelay() public returns (address) {
+    function deployExpressRelay(
+        address owner,
+        address admin,
+        address relayer,
+        uint256 feeSplitProtocolDefault,
+        uint256 feeSplitRelayer
+    ) public returns (address) {
         (, uint256 skDeployer) = getDeployer();
-        (address relayer, uint256 relayerSk) = makeAddrAndKey("relayer");
-        // TODO: set admin to different address than relayer
-        address admin = relayer;
-        console.log("pk relayer", relayer);
-        console.log("sk relayer", relayerSk);
-        // since feeSplitPrecision is set to 10 ** 18, this represents ~50% of the fees
-        uint256 feeSplitProtocolDefault = 50 * (10 ** 16);
-        // ~5% (10% of the remaining 50%) of the fees go to the relayer
-        uint256 feeSplitRelayer = 10 * (10 ** 16);
         vm.startBroadcast(skDeployer);
-        payable(relayer).transfer(0.01 ether);
-        ExpressRelay multicall = new ExpressRelay(
+        ExpressRelayUpgradable _expressRelayUpgradable = new ExpressRelayUpgradable();
+        // deploy proxy contract and point it to implementation
+        ERC1967Proxy proxy = new ERC1967Proxy(
+            address(_expressRelayUpgradable),
+            ""
+        );
+        // wrap in ABI to support easier calls
+        ExpressRelayUpgradable expressRelay = ExpressRelayUpgradable(
+            payable(proxy)
+        );
+        expressRelay.initialize(
+            owner,
             admin,
             relayer,
             feeSplitProtocolDefault,
             feeSplitRelayer
         );
         vm.stopBroadcast();
-        console.log("deployed ExpressRelay contract at", address(multicall));
-        return address(multicall);
+        console.log(
+            "deployed ExpressRelayUpgradable implementation at",
+            address(_expressRelayUpgradable)
+        );
+        console.log("ExpressRelay ERC1967 proxy at", address(expressRelay));
+        return address(expressRelay);
+    }
+
+    function upgradeExpressRelay(address proxyAddress) public {
+        (, uint256 skDeployer) = getDeployer();
+        vm.startBroadcast(skDeployer);
+        ExpressRelayUpgradable _newImplementation = new ExpressRelayUpgradable();
+        // Proxy object is technically an ExpressRelayUpgradable because it points to an implementation
+        // of such contract. Therefore we can call the upgradeTo function on it.
+        ExpressRelayUpgradable proxy = ExpressRelayUpgradable(
+            payable(proxyAddress)
+        );
+        proxy.upgradeTo(address(_newImplementation));
+        vm.stopBroadcast();
     }
 
     function deployVault(
         address multicall,
         address oracle,
         bool allowUndercollateralized
     ) public returns (address) {
+        (address deployer, ) = getDeployer();
         // make token vault deployer wallet
         (, uint256 tokenVaultDeployerSk) = makeAddrAndKey("tokenVaultDeployer");
         console.log("sk token vault deployer", tokenVaultDeployerSk);
         vm.startBroadcast(tokenVaultDeployerSk);
         TokenVault vault = new TokenVault(
+            deployer,
             multicall,
             oracle,
             allowUndercollateralized
@@ -138,7 +173,20 @@ contract VaultScript is Script {
     {
         (address deployer, ) = getDeployer();
         address weth = deployWeth();
-        address expressRelay = deployExpressRelay();
+
+        (address relayer, uint256 relayerSk) = makeAddrAndKey("relayer");
+        // since feeSplitPrecision is set to 10 ** 18, this represents ~50% of the fees
+        uint256 feeSplitProtocolDefault = 50 * (10 ** 16);
+        // ~5% (10% of the remaining 50%) of the fees go to the relayer
+        uint256 feeSplitRelayer = 10 * (10 ** 16);
+        address expressRelay = deployExpressRelay(
+            deployer,
+            deployer,
+            relayer,
+            feeSplitProtocolDefault,
+            feeSplitRelayer
+        );
+
         address opportunityAdapter = deployOpportunityAdapter(
             deployer,
             deployer,
@@ -164,7 +212,20 @@ contract VaultScript is Script {
         (address deployer, uint256 skDeployer) = getDeployer();
         if (pyth == address(0)) pyth = deployMockPyth();
         if (weth == address(0)) weth = deployWeth();
-        address expressRelay = deployExpressRelay();
+
+        (address relayer, uint256 relayerSk) = makeAddrAndKey("relayer");
+        // since feeSplitPrecision is set to 10 ** 18, this represents ~50% of the fees
+        uint256 feeSplitProtocolDefault = 50 * (10 ** 16);
+        // ~5% (10% of the remaining 50%) of the fees go to the relayer
+        uint256 feeSplitRelayer = 10 * (10 ** 16);
+        address expressRelay = deployExpressRelay(
+            deployer,
+            deployer,
+            relayer,
+            feeSplitProtocolDefault,
+            feeSplitRelayer
+        );
+
         address opportunityAdapter = deployOpportunityAdapter(
             deployer,
             deployer,

diff --git a/per_multicall/src/ExpressRelay.sol b/per_multicall/src/ExpressRelay.sol
@@ -13,22 +13,24 @@ contract ExpressRelay is ExpressRelayHelpers, ExpressRelayState {
     event ReceivedETH(address sender, uint256 amount);
 
     /**
-     * @notice ExpressRelay constructor - Initializes a new ExpressRelay contract with given parameters
+     * @notice ExpressRelay initializer - Initializes a new ExpressRelay contract with given parameters
      *
      * @param admin: address of admin of express relay
      * @param relayer: address of relayer EOA
      * @param feeSplitProtocolDefault: default fee split to be paid to the protocol whose permissioning is being used
      * @param feeSplitRelayer: split of the non-protocol fees to be paid to the relayer
      */
-    constructor(
+    function _initialize(
         address admin,
         address relayer,
         uint256 feeSplitProtocolDefault,
         uint256 feeSplitRelayer
-    ) {
+    ) internal {
         state.admin = admin;
         state.relayer = relayer;
 
+        setFeeSplitPrecision();
+
         validateFeeSplit(feeSplitProtocolDefault);
         state.feeSplitProtocolDefault = feeSplitProtocolDefault;
 

diff --git a/per_multicall/src/ExpressRelayState.sol b/per_multicall/src/ExpressRelayState.sol
@@ -28,7 +28,7 @@ contract ExpressRelayStorage {
 contract ExpressRelayState is IExpressRelay {
     ExpressRelayStorage.State state;
 
-    constructor() {
+    function setFeeSplitPrecision() internal {
         state.feeSplitPrecision = 10 ** 18;
     }
 

diff --git a/per_multicall/src/ExpressRelayUpgradable.sol b/per_multicall/src/ExpressRelayUpgradable.sol
@@ -0,0 +1,105 @@
+// Copyright (C) 2024 Lavra Holdings Limited - All Rights Reserved
+pragma solidity ^0.8.13;
+
+import "./Errors.sol";
+import "./Structs.sol";
+import "./SigVerify.sol";
+import "./ExpressRelay.sol";
+import "./WETH9.sol";
+
+import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
+import "openzeppelin-contracts/contracts/utils/Strings.sol";
+import "openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol";
+import "openzeppelin-contracts-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol";
+import "openzeppelin-contracts-upgradeable/contracts/access/Ownable2StepUpgradeable.sol";
+import {ExpressRelay} from "./ExpressRelay.sol";
+
+contract ExpressRelayUpgradable is
+    Initializable,
+    Ownable2StepUpgradeable,
+    UUPSUpgradeable,
+    ExpressRelay
+{
+    event ContractUpgraded(
+        address oldImplementation,
+        address newImplementation
+    );
+
+    // The contract will have an owner and an admin
+    // The owner will have all the power over it.
+    // The admin can set some config parameters only.
+    function initialize(
+        address owner,
+        address admin,
+        address relayer,
+        uint256 feeSplitProtocolDefault,
+        uint256 feeSplitRelayer
+    ) public initializer {
+        require(owner != address(0), "owner is zero address");
+        require(admin != address(0), "admin is zero address");
+        require(relayer != address(0), "relayer is zero address");
+
+        __Ownable_init();
+        __UUPSUpgradeable_init();
+
+        ExpressRelay._initialize(
+            admin,
+            relayer,
+            feeSplitProtocolDefault,
+            feeSplitRelayer
+        );
+
+        // We need to transfer the ownership from deployer to the new owner
+        _transferOwnership(owner);
+    }
+
+    /// Ensures the contract cannot be uninitialized and taken over.
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() initializer {}
+
+    // Only allow the owner to upgrade the proxy to a new implementation.
+    function _authorizeUpgrade(address) internal override onlyOwner {}
+
+    // We have not overridden these methods in Pyth contracts implementation.
+    // But we are overriding them here because there was no owner before and
+    // `_authorizeUpgrade` would cause a revert for these. Now we have an owner, and
+    // because we want to test for the magic. We are overriding these methods.
+    function upgradeTo(address newImplementation) external override onlyProxy {
+        address oldImplementation = _getImplementation();
+        _authorizeUpgrade(newImplementation);
+        _upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
+
+        magicCheck();
+
+        emit ContractUpgraded(oldImplementation, _getImplementation());
+    }
+
+    function upgradeToAndCall(
+        address newImplementation,
+        bytes memory data
+    ) external payable override onlyProxy {
+        address oldImplementation = _getImplementation();
+        _authorizeUpgrade(newImplementation);
+        _upgradeToAndCallUUPS(newImplementation, data, true);
+
+        magicCheck();
+
+        emit ContractUpgraded(oldImplementation, _getImplementation());
+    }
+
+    function magicCheck() internal view {
+        // Calling a method using `this.<method>` will cause a contract call that will use
+        // the new contract. This call will fail if the method does not exists or the magic
+        // is different.
+        if (this.expressRelayUpgradableMagic() != 0x292e6740)
+            revert InvalidMagicValue();
+    }
+
+    function expressRelayUpgradableMagic() public pure returns (uint32) {
+        return 0x292e6740;
+    }
+
+    function version() public pure returns (string memory) {
+        return "0.1.0";
+    }
+}
diff --git a/per_multicall/src/OpportunityAdapter.sol b/per_multicall/src/OpportunityAdapter.sol
@@ -16,7 +16,7 @@ abstract contract OpportunityAdapter is SigVerify {
     mapping(bytes => bool) _signatureUsed;
 
     /**
-     * @notice OpportunityAdapter constructor - Initializes a new opportunity adapter contract with given parameters
+     * @notice OpportunityAdapter initializer - Initializes a new opportunity adapter contract with given parameters
      *
      * @param admin: address of admin of opportunity adapter
      * @param expressRelay: address of express relay
@@ -32,6 +32,18 @@ abstract contract OpportunityAdapter is SigVerify {
         _weth = weth;
     }
 
+    /**
+     * @notice setExpressRelay function - sets the address of the express relay authenticated for calling this contract
+     *
+     * @param expressRelay: address of express relay contract
+     */
+    function setExpressRelay(address expressRelay) public {
+        if (msg.sender != _admin) {
+            revert Unauthorized();
+        }
+        _expressRelay = expressRelay;
+    }
+
     /**
      * @notice getExpressRelay function - returns the address of the express relay authenticated for calling this contract
      */

diff --git a/per_multicall/src/TokenVault.sol b/per_multicall/src/TokenVault.sol
@@ -19,8 +19,9 @@ contract TokenVault is IExpressRelayFeeReceiver {
 
     event VaultReceivedETH(address sender, uint256 amount, bytes permissionKey);
 
+    address _admin;
     uint256 _nVaults;
-    address public immutable expressRelay;
+    address public expressRelay;
     mapping(uint256 => Vault) _vaults;
     address _oracle;
     bool _allowUndercollateralized;
@@ -33,16 +34,25 @@ contract TokenVault is IExpressRelayFeeReceiver {
      * @param allowUndercollateralized: boolean to allow undercollateralized vaults to be created and updated. Can be set to true for testing.
      */
     constructor(
+        address admin,
         address expressRelayAddress,
         address oracleAddress,
         bool allowUndercollateralized
     ) {
+        _admin = admin;
         _nVaults = 0;
         expressRelay = expressRelayAddress;
         _oracle = oracleAddress;
         _allowUndercollateralized = allowUndercollateralized;
     }
 
+    function setExpressRelayAddress(address expressRelayAddress) public {
+        if (msg.sender != _admin) {
+            revert NotAdmin();
+        }
+        expressRelay = expressRelayAddress;
+    }
+
     //TODO: Fix function name/logic/documentation to match each other
     /**
      * @notice getLastVaultId function - getter function to get the id of the next vault to be created

diff --git a/per_multicall/src/TokenVaultErrors.sol b/per_multicall/src/TokenVaultErrors.sol
@@ -1,6 +1,9 @@
 // Copyright (C) 2024 Lavra Holdings Limited - All Rights Reserved
 pragma solidity ^0.8.13;
 
+// Signature: 0x7bfa4b9f
+error NotAdmin();
+
 // Signature: 0xe922edfd
 error UncollateralizedVaultCreation();