withdrawTo() moved to Impl

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 {
+    // withdrawTo(...) methods are in the VaultImpl. If the Vault is killed, withdrawals are only allowed to the
-        _withdraw(token, recipient, amount);
+    // owner of this Vault.
-    }
-
-    function _withdraw(IERC20 token, address recipient, uint256 amount) internal onlyOwner nonReentrant {
-        token.transfer(recipient, amount);
-    }
-
-    modifier onlyOwner() {
-        require(msg.sender == _owner, "not owner");
-        _;
-    }
 
 }

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");
-        _;
-    }
-
 }
 
 

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 {}

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