From 722c48102526e5c19de2dac4dd0abc1fdd42a2ac Mon Sep 17 00:00:00 2001 From: Anirudh Suresh Date: Sun, 31 Mar 2024 19:30:05 -0400 Subject: [PATCH] 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 --- auction-server/Cargo.lock | 2 +- auction-server/Cargo.toml | 2 +- per_multicall/script/Vault.s.sol | 93 +++++++++++++--- per_multicall/src/ExpressRelay.sol | 8 +- per_multicall/src/ExpressRelayState.sol | 2 +- per_multicall/src/ExpressRelayUpgradable.sol | 105 +++++++++++++++++++ per_multicall/src/OpportunityAdapter.sol | 14 ++- per_multicall/src/TokenVault.sol | 12 ++- per_multicall/src/TokenVaultErrors.sol | 3 + per_multicall/test/ExpressRelayTestSetup.sol | 24 ++++- per_sdk/protocols/token_vault_monitor.py | 70 +++++++------ per_sdk/utils/pyth_prices.py | 2 +- 12 files changed, 275 insertions(+), 62 deletions(-) create mode 100644 per_multicall/src/ExpressRelayUpgradable.sol diff --git a/auction-server/Cargo.lock b/auction-server/Cargo.lock index 4c993af3..ccff29cf 100644 --- a/auction-server/Cargo.lock +++ b/auction-server/Cargo.lock @@ -190,7 +190,7 @@ dependencies = [ [[package]] name = "auction-server" -version = "0.2.3" +version = "0.3.0" dependencies = [ "anyhow", "async-stream", diff --git a/auction-server/Cargo.toml b/auction-server/Cargo.toml index 5f9f1a44..06418d36 100644 --- 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 index 8476d932..31bb7b6e 100644 --- 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,28 +89,52 @@ 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( @@ -109,11 +142,13 @@ contract VaultScript is Script { 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 index 114d48d1..f5c57faf 100644 --- 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 index 39df3dcb..8d7d4fe5 100644 --- 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 new file mode 100644 index 00000000..704e3d1d --- /dev/null +++ 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.` 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 index 5363ecfd..84b70d09 100644 --- 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 index b7206655..5cf7325c 100644 --- 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 index 0d25fc37..b3c32231 100644 --- 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(); diff --git a/per_multicall/test/ExpressRelayTestSetup.sol b/per_multicall/test/ExpressRelayTestSetup.sol index cd3de4ca..7e53fa59 100644 --- a/per_multicall/test/ExpressRelayTestSetup.sol +++ b/per_multicall/test/ExpressRelayTestSetup.sol @@ -28,6 +28,7 @@ import "./helpers/PriceHelpers.sol"; import "./helpers/TestParsingHelpers.sol"; import "./helpers/MulticallHelpers.sol"; import "../src/OpportunityAdapterUpgradable.sol"; +import "../src/ExpressRelayUpgradable.sol"; /** * @title ExpressRelayTestSetUp @@ -49,7 +50,7 @@ contract ExpressRelayTestSetup is TokenVault public tokenVault; SearcherVault public searcherA; SearcherVault public searcherB; - ExpressRelay public expressRelay; + ExpressRelayUpgradable public expressRelay; WETH9 public weth; OpportunityAdapterUpgradable public opportunityAdapter; MockPyth public mockPyth; @@ -135,7 +136,16 @@ contract ExpressRelayTestSetup is function setUpContracts() public { // instantiate multicall contract with ExpressRelay operator as the deployer vm.prank(relayer); - expressRelay = new ExpressRelay( + ExpressRelayUpgradable _expressRelay = new ExpressRelayUpgradable(); + // deploy proxy contract and point it to implementation + ERC1967Proxy proxyExpressRelay = new ERC1967Proxy( + address(_expressRelay), + "" + ); + expressRelay = ExpressRelayUpgradable(payable(proxyExpressRelay)); + expressRelay.initialize( + // TODO: fix the owner and admin here + relayer, admin, relayer, feeSplitProtocolDefault, @@ -148,8 +158,13 @@ contract ExpressRelayTestSetup is vm.prank(relayer); OpportunityAdapterUpgradable _opportunityAdapter = new OpportunityAdapterUpgradable(); // deploy proxy contract and point it to implementation - ERC1967Proxy proxy = new ERC1967Proxy(address(_opportunityAdapter), ""); - opportunityAdapter = OpportunityAdapterUpgradable(payable(proxy)); + ERC1967Proxy proxyOpportunityAdapter = new ERC1967Proxy( + address(_opportunityAdapter), + "" + ); + opportunityAdapter = OpportunityAdapterUpgradable( + payable(proxyOpportunityAdapter) + ); opportunityAdapter.initialize( // TODO: fix the owner and admin here relayer, @@ -164,6 +179,7 @@ contract ExpressRelayTestSetup is bool allowUndercollateralized = false; vm.prank(tokenVaultDeployer); // we prank here to standardize the value of the token contract address across different runs tokenVault = new TokenVault( + admin, address(expressRelay), address(mockPyth), allowUndercollateralized diff --git a/per_sdk/protocols/token_vault_monitor.py b/per_sdk/protocols/token_vault_monitor.py index 47a450f5..4cbf9443 100644 --- a/per_sdk/protocols/token_vault_monitor.py +++ b/per_sdk/protocols/token_vault_monitor.py @@ -206,41 +206,45 @@ async def get_liquidation_opportunities(self) -> list[Opportunity]: liquidatable = [] # just get the last 5 accounts to optimize for rpc calls accounts = await self.get_recent_accounts(5) - price_ids = set() - for account in accounts: - price_ids.add(account["token_id_collateral"]) - price_ids.add(account["token_id_debt"]) - price_ids = list(price_ids) - prices = await self.price_feed_client.get_pyth_prices_latest(price_ids) - price_dict = dict(zip(price_ids, prices)) - for account in accounts: - # vault is already liquidated - if account["amount_collateral"] == 0 and account["amount_debt"] == 0: - continue - price_collateral = price_dict.get(account["token_id_collateral"]) - price_debt = price_dict.get(account["token_id_debt"]) - if price_collateral is None: - raise Exception( - f"Price for collateral token {account['token_id_collateral']} not found" - ) + if len(accounts) > 0: + price_ids = set() + for account in accounts: + price_ids.add(account["token_id_collateral"]) + price_ids.add(account["token_id_debt"]) + price_ids = list(price_ids) + prices = await self.price_feed_client.get_pyth_prices_latest(price_ids) + price_dict = dict(zip(price_ids, prices)) + for account in accounts: + # vault is already liquidated + if account["amount_collateral"] == 0 and account["amount_debt"] == 0: + continue + price_collateral = price_dict.get(account["token_id_collateral"]) + price_debt = price_dict.get(account["token_id_debt"]) + if price_collateral is None: + raise Exception( + f"Price for collateral token {account['token_id_collateral']} not found" + ) - if price_debt is None: - raise Exception( - f"Price for debt token {account['token_id_debt']} not found" - ) + if price_debt is None: + raise Exception( + f"Price for debt token {account['token_id_debt']} not found" + ) - value_collateral = ( - int(price_collateral["price"]["price"]) * account["amount_collateral"] - ) - value_debt = int(price_debt["price"]["price"]) * account["amount_debt"] - health = value_collateral / value_debt - logger.debug(f"Account {account['account_number']} health: {health}") - if ( - value_debt * int(account["min_health_ratio"]) - > value_collateral * 10**18 - ): - price_updates = [price_collateral, price_debt] - liquidatable.append(self.create_liquidation_opp(account, price_updates)) + value_collateral = ( + int(price_collateral["price"]["price"]) + * account["amount_collateral"] + ) + value_debt = int(price_debt["price"]["price"]) * account["amount_debt"] + health = value_collateral / value_debt + logger.debug(f"Account {account['account_number']} health: {health}") + if ( + value_debt * int(account["min_health_ratio"]) + > value_collateral * 10**18 + ): + price_updates = [price_collateral, price_debt] + liquidatable.append( + self.create_liquidation_opp(account, price_updates) + ) return liquidatable diff --git a/per_sdk/utils/pyth_prices.py b/per_sdk/utils/pyth_prices.py index b1a8c5a6..44e78369 100644 --- a/per_sdk/utils/pyth_prices.py +++ b/per_sdk/utils/pyth_prices.py @@ -3,7 +3,7 @@ import httpx -HERMES_ENDPOINT_HTTPS = "http://hermes-stable.hermes-stable:8080/api/" +HERMES_ENDPOINT_HTTPS = "https://hermes.pyth.network/api/" HERMES_ENDPOINT_WSS = "wss://hermes.pyth.network/ws"