From 7e47616a616515eb604bf2f28172434fff20b64c Mon Sep 17 00:00:00 2001 From: tim Date: Sun, 5 Jan 2025 17:35:08 -0400 Subject: [PATCH] withdrawTo() moved to Impl --- src/core/Vault.sol | 47 +++++++++++++++++++++------------------ src/core/VaultImpl.sol | 21 ++++++++++------- src/interface/IVault.sol | 6 ++--- src/more/VaultAddress.sol | 2 +- 4 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/core/Vault.sol b/src/core/Vault.sol index 1059aec..586a602 100644 --- a/src/core/Vault.sol +++ b/src/core/Vault.sol @@ -18,6 +18,28 @@ contract VaultStorage is ReentrancyGuard { // The re-entrancy lock is part of th OrdersInfo internal _ordersInfo; } + +contract VaultBase is VaultStorage { + + function _withdrawNative(address payable recipient, uint256 amount) internal onlyOwner { + recipient.transfer(amount); + emit IVaultProxy.Withdrawal(recipient, msg.value); + } + + + function _withdraw(IERC20 token, address recipient, uint256 amount) internal onlyOwner nonReentrant { + token.transfer(recipient, amount); + } + + + modifier onlyOwner() { + require(msg.sender == _owner, "not owner"); + _; + } + +} + + // Vault is implemented in three parts: // 1. VaultStorage contains the data members, which cannot be upgraded. The number of data slots is fixed upon // construction of the Vault contract. @@ -28,7 +50,7 @@ contract VaultStorage is ReentrancyGuard { // The re-entrancy lock is part of th // OrderLib, implementing the common order manipulation methods. Each Vault may be independently upgraded to point // to a new VaultImpl contract by the owner calling their vault's `upgrade()` method with the correct argument. -contract Vault is IVaultProxy, VaultStorage, Proxy { +contract Vault is IVaultProxy, VaultBase, Proxy { //REV putting all variable decls together so we can more easily see visibility and immutability errors IVaultFactory immutable private _factory; uint8 immutable private _num; @@ -87,30 +109,11 @@ contract Vault is IVaultProxy, VaultStorage, Proxy { _withdrawNative(payable(_owner), amount); } - function withdrawTo(address payable recipient, uint256 amount) external override { - _withdrawNative(recipient, amount); - } - - function _withdrawNative(address payable recipient, uint256 amount) internal onlyOwner { - recipient.transfer(amount); - emit Withdrawal(recipient, msg.value); - } - function withdraw(IERC20 token, uint256 amount) external override { _withdraw(token, _owner, amount); } - function withdrawTo(IERC20 token, address recipient, uint256 amount) external override { - _withdraw(token, recipient, amount); - } - - function _withdraw(IERC20 token, address recipient, uint256 amount) internal onlyOwner nonReentrant { - token.transfer(recipient, amount); - } - - modifier onlyOwner() { - require(msg.sender == _owner, "not owner"); - _; - } + // withdrawTo(...) methods are in the VaultImpl. If the Vault is killed, withdrawals are only allowed to the + // owner of this Vault. } diff --git a/src/core/VaultImpl.sol b/src/core/VaultImpl.sol index fb463ce..e617364 100644 --- a/src/core/VaultImpl.sol +++ b/src/core/VaultImpl.sol @@ -5,7 +5,7 @@ import {console2} from "@forge-std/console2.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IFeeManager} from "../interface/IFeeManager.sol"; import {IVaultProxy, IVaultImpl} from "../interface/IVault.sol"; -import {VaultStorage} from "./Vault.sol"; +import {VaultBase} from "./Vault.sol"; import {Dexorder} from "../more/Dexorder.sol"; import "./OrderSpec.sol"; import {OrderLib} from "./OrderLib.sol"; @@ -19,9 +19,9 @@ import {UniswapV3Arbitrum} from "./UniswapV3.sol"; // the implementation contract, the original Vault's state is used. So here the VaultImpl inherits the vault state but in // usage, this state will be the state of the calling Vault, not of the deployed VaultImpl contract instance. -contract VaultImpl is IVaultImpl, VaultStorage { +contract VaultImpl is IVaultImpl, VaultBase { - uint256 constant public version = 2; + uint256 constant public version = 1; IFeeManager public immutable feeManager; IRouter private immutable router; @@ -33,6 +33,16 @@ contract VaultImpl is IVaultImpl, VaultStorage { weth9 = IWETH9(weth9_); } + // withdrawals to addresses other than the owner are only allowed if the Vault is not killed + + function withdrawTo(address payable recipient, uint256 amount) external override { + _withdrawNative(recipient, amount); + } + + function withdrawTo(IERC20 token, address recipient, uint256 amount) external override { + _withdraw(token, recipient, amount); + } + function numSwapOrders() external view returns (uint64 num) { return uint64(_ordersInfo.orders.length); } @@ -142,11 +152,6 @@ contract VaultImpl is IVaultImpl, VaultStorage { weth9.withdraw(amount); } - modifier onlyOwner() { - require(msg.sender == _owner, "not owner"); - _; - } - } diff --git a/src/interface/IVault.sol b/src/interface/IVault.sol index f8c7a3e..5a852e0 100644 --- a/src/interface/IVault.sol +++ b/src/interface/IVault.sol @@ -10,6 +10,9 @@ interface IVaultImpl { function version() external view returns (uint256); + function withdrawTo(address payable recipient, uint256 amount) external; + function withdrawTo(IERC20 token, address recipient, uint256 amount) external; + function feeManager() external view returns (IFeeManager); function placementFee(SwapOrder memory order, IFeeManager.FeeSchedule memory sched) external view returns (uint256 orderFee, uint256 gasFee); function placementFee(SwapOrder[] memory orders, IFeeManager.FeeSchedule memory sched) external view returns (uint256 orderFee, uint256 gasFee); @@ -65,11 +68,8 @@ interface IVaultProxy { function withdraw(uint256 amount) external; - function withdrawTo(address payable recipient, uint256 amount) external; - function withdraw(IERC20 token, uint256 amount) external; - function withdrawTo(IERC20 token, address recipient, uint256 amount) external; } interface IVault is IVaultProxy, IVaultImpl {} diff --git a/src/more/VaultAddress.sol b/src/more/VaultAddress.sol index 31ac9b4..45813ce 100644 --- a/src/more/VaultAddress.sol +++ b/src/more/VaultAddress.sol @@ -6,7 +6,7 @@ import "@forge-std/console2.sol"; library VaultAddress { // keccak-256 hash of the Vault's bytecode (not the deployed bytecode but the initialization bytecode) - bytes32 public constant VAULT_INIT_CODE_HASH = 0xda672cdca096de00f3fed8150430564c059a59ad30cb2c824902097e25cd8b3a; + bytes32 public constant VAULT_INIT_CODE_HASH = 0x8bcfa810f4ebc5344dc7b646c460fafa5b4354f8840208a255aa8c217d6690b1; // the contract being constructed must not have any constructor arguments or the determinism will be broken. // instead, use a callback to get construction arguments