WOETH: Donation attack prevention

brownie/scripts/woeth_manipulation.py

@@ -0,0 +1,27 @@
+from world import *
+
+def expect_approximate(woeth_holder, expected_balance):
+	balance = woeth.balanceOf(woeth_holder)
+	diff = abs(expected_balance - balance)
+	if (diff != 0):
+		raise Exception("Unexpected balance for account: %s".format(woeth_holder))
+
+def confirm_balances_after_upgrade():
+	expect_approximate("0xBBBBBbbBBb9cC5e90e3b3Af64bdAF62C37EEFFCb", 1013453939109688661944)
+	expect_approximate("0xC460B0b6c9b578A4Cb93F99A691e16dB96Ee5833", 575896531839923556165)
+	expect_approximate("0xdca0a2341ed5438e06b9982243808a76b9add6d0", 319671606657733042618)
+	expect_approximate("0x8a9d46d28003673cd4fe7a56ecfcfa2be6372e64", 182355401624955452064)
+	expect_approximate("0xf65ecb5610000100befba41b9f9cf5ca32838078", 97352556026536192865)
+	expect_approximate("0x0a26e7ab5c554232314a8d459eff0ede72333f08", 91628532171545105831)
+
+
+def manipulate_price():
+	OETH_WHALE="0xa4C637e0F704745D182e4D38cAb7E7485321d059"
+	whl = {'from': OETH_WHALE }
+
+	woeth.convertToAssets(1e18) / 1e18
+	oeth.transfer(woeth.address, 10_000 * 1e18, whl)
+	woeth.convertToAssets(1e18) / 1e18
+
+	oeth.approve(woeth.address, 1e50, whl)
+	woeth.deposit(5_000 * 1e18, OETH_WHALE, whl)

brownie/world.py

@@ -35,6 +35,7 @@ def load_contract(name, address):
 weth = load_contract('ERC20', WETH)
 ousd = load_contract('ousd', OUSD)
 oeth = load_contract('ousd', OETH)
+woeth = load_contract('erc4626', WOETH)
 usdt = load_contract('usdt', USDT)
 usdc = load_contract('usdc', USDC)
 dai = load_contract('dai', DAI)

contracts/contracts/token/WOETH.sol

@@ -6,17 +6,30 @@
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 
+import { StableMath } from "../utils/StableMath.sol";
 import { Governable } from "../governance/Governable.sol";
 import { Initializable } from "../utils/Initializable.sol";
 import { OETH } from "./OETH.sol";
 
 /**
  * @title OETH Token Contract
  * @author Origin Protocol Inc
+ *
+ * @dev An important capability of this contract is that it isn't susceptible to changes of the
+ * exchange rate of WOETH/OETH if/when someone sends the underlying asset (OETH) to the contract.
+ * If OETH weren't rebasing this could be achieved by solely tracking the ERC20 transfers of the OETH
+ * token on mint, deposit, redeem, withdraw. The issue is that OETH is rebasing and OETH balances
+ * will change when the token rebases. For that reason we are tracking the WOETH contract credits and
+ * credits per token in those 4 actions. That way WOETH can keep an accurate track of the OETH balance
+ * ignoring any unexpected transfers of OETH to this contract.
  */
 
 contract WOETH is ERC4626, Governable, Initializable {
     using SafeERC20 for IERC20;
+    using StableMath for uint256;
+    // doesn't need to be public, but convenient to be able to confirm the state on the mainnet
+    uint256 public oethCreditsHighres;
+    bool _oethCreditsInitialized;
 
     constructor(
         ERC20 underlying_,
@@ -31,6 +44,25 @@
         OETH(address(asset())).rebaseOptIn();
     }
 
+    function initialize2() external onlyGovernor {
+        if (_oethCreditsInitialized) {
+            require(false, "Initialize2 already called");
+        }
+
+        _oethCreditsInitialized = true;
+        /*
+         * This contract is using creditsBalanceOfHighres rather than creditsBalanceOf since this
+         * ensures better accuracy when rounding. Also creditsBalanceOf can be a little
+         * finicky since it reports Highres version of credits and creditsPerToken
+         * when the account is a fresh one. That doesn't have an effect on mainnet since
+         * WOETH has already seen transactions. But it is rather annoying in unit test
+         * environment.
+         */
+        (oethCreditsHighres, , ) = OETH(asset()).creditsBalanceOfHighres(
+            address(this)
+        );
+    }
+
     function name() public view override returns (string memory) {
         return "Wrapped OETH";
     }
@@ -49,7 +81,161 @@
         external
         onlyGovernor
     {
+        //@dev TODO: we could implement a feature where if anyone sends OETH direclty to
+        // the contract, that we can let the governor transfer the excess of the token.
         require(asset_ != address(asset()), "Cannot collect OETH");
         IERC20(asset_).safeTransfer(governor(), amount_);
     }
+
+    /**
+     * @dev This function converts requested OETH token amount to its underlying OETH
+     * credits value that is stored internally in OETH.sol and is required in order to
+     * be able to rebase.
+     *
+     * @param oethAmount Amount of OETH to be converted to OETH credits
+     * @return amount of OETH credits the OETH amount corresponds to
+     */
+    function _oethToOethCreditsHighres(uint256 oethAmount)
+        internal
+        returns (uint256)
+    {
+        (, uint256 creditsPerTokenHighres, ) = OETH(asset())
+            .creditsBalanceOfHighres(address(this));
+
+        /**
+         * Multiplying OETH amount with the creditsPerTokenHighres is exactly the math that
+         * is internally being done in OETH:
+         */
+        // solhint-disable-next-line max-line-length
+        /** https://github.com/OriginProtocol/origin-dollar/blob/2314cccf2933f5c1f76a6549c1f5c9cc935b6f05/contracts/contracts/token/OUSD.sol#L242-L249
+         *
+         * This should make sure that the rounding will always be correct / mimic the rounding
+         * of OETH.
+         */
+        return oethAmount.mulTruncate(creditsPerTokenHighres);
+    }
+
+    /** @dev See {IERC4262-totalAssets} */
+    function totalAssets() public view virtual override returns (uint256) {
+        (, uint256 creditsPerTokenHighres, ) = OETH(asset())
+            .creditsBalanceOfHighres(address(this));
+
+        return (oethCreditsHighres).divPrecisely(creditsPerTokenHighres);
+    }
+
+    /** @dev See {IERC4262-deposit} */
+    function deposit(uint256 assets, address receiver)
+        public
+        virtual
+        override
+        returns (uint256)
+    {
+        require(
+            assets <= maxDeposit(receiver),
+            "ERC4626: deposit more then max"
+        );
+
+        address caller = _msgSender();
+        uint256 shares = previewDeposit(assets);
+
+        // if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
+        // the tokensToSend hook, so we need to transfer before we mint to keep the invariants.
+        SafeERC20.safeTransferFrom(
+            IERC20(asset()),
+            caller,
+            address(this),
+            assets
+        );
+        _mint(receiver, shares);
+        oethCreditsHighres += _oethToOethCreditsHighres(assets);
+
+        emit Deposit(caller, receiver, assets, shares);
+
+        return shares;
+    }
+
+    /** @dev See {IERC4262-mint} */
+    function mint(uint256 shares, address receiver)
+        public
+        virtual
+        override
+        returns (uint256)
+    {
+        require(shares <= maxMint(receiver), "ERC4626: mint more then max");
+
+        address caller = _msgSender();
+        uint256 assets = previewMint(shares);
+
+        // if _asset is ERC777, transferFrom can call reenter BEFORE the transfer happens through
+        // the tokensToSend hook, so we need to transfer before we mint to keep the invariants.
+        SafeERC20.safeTransferFrom(
+            IERC20(asset()),
+            caller,
+            address(this),
+            assets
+        );
+        _mint(receiver, shares);
+        oethCreditsHighres += _oethToOethCreditsHighres(assets);
+
+        emit Deposit(caller, receiver, assets, shares);
+
+        return assets;
+    }
+
+    /** @dev See {IERC4262-withdraw} */
+    function withdraw(
+        uint256 assets,
+        address receiver,
+        address owner
+    ) public virtual override returns (uint256) {
+        require(
+            assets <= maxWithdraw(owner),
+            "ERC4626: withdraw more then max"
+        );
+
+        address caller = _msgSender();
+        uint256 shares = previewWithdraw(assets);
+
+        if (caller != owner) {
+            _spendAllowance(owner, caller, shares);
+        }
+
+        // if _asset is ERC777, transfer can call reenter AFTER the transfer happens through
+        // the tokensReceived hook, so we need to transfer after we burn to keep the invariants.
+        _burn(owner, shares);
+        oethCreditsHighres -= _oethToOethCreditsHighres(assets);
+
+        SafeERC20.safeTransfer(IERC20(asset()), receiver, assets);
+
+        emit Withdraw(caller, receiver, owner, assets, shares);
+
+        return shares;
+    }
+
+    /** @dev See {IERC4262-redeem} */
+    function redeem(
+        uint256 shares,
+        address receiver,
+        address owner
+    ) public virtual override returns (uint256) {
+        require(shares <= maxRedeem(owner), "ERC4626: redeem more then max");
+
+        address caller = _msgSender();
+        uint256 assets = previewRedeem(shares);
+
+        if (caller != owner) {
+            _spendAllowance(owner, caller, shares);
+        }
+
+        // if _asset is ERC777, transfer can call reenter AFTER the transfer happens through
+        // the tokensReceived hook, so we need to transfer after we burn to keep the invariants.
+        _burn(owner, shares);
+        oethCreditsHighres -= _oethToOethCreditsHighres(assets);
+
+        SafeERC20.safeTransfer(IERC20(asset()), receiver, assets);
+
+        emit Withdraw(caller, receiver, owner, assets, shares);
+
+        return assets;
+    }
 }

contracts/deploy/deployActions.js

@@ -1471,6 +1471,34 @@ const deployWOusd = async () => {
   ](dWrappedOusdImpl.address, governorAddr, initData);
 };
 
+const deployWOeth = async () => {
+  const { deployerAddr, governorAddr } = await getNamedAccounts();
+  const sDeployer = await ethers.provider.getSigner(deployerAddr);
+  const sGovernor = await ethers.provider.getSigner(governorAddr);
+
+  const oeth = await ethers.getContract("OETHProxy");
+  const dWrappedOethImpl = await deployWithConfirmation("WOETH", [
+    oeth.address,
+    "Wrapped OETH IMPL",
+    "WOETH IMPL",
+  ]);
+  await deployWithConfirmation("WOETHProxy");
+  const woethProxy = await ethers.getContract("WOETHProxy");
+  const woeth = await ethers.getContractAt("WOETH", woethProxy.address);
+
+  const initData = woeth.interface.encodeFunctionData("initialize()", []);
+
+  await woethProxy.connect(sDeployer)[
+    // eslint-disable-next-line no-unexpected-multiline
+    "initialize(address,address,bytes)"
+  ](dWrappedOethImpl.address, governorAddr, initData);
+
+  await woeth.connect(sGovernor)[
+    // eslint-disable-next-line no-unexpected-multiline
+    "initialize2()"
+  ]();
+};
+
 const deployOETHSwapper = async () => {
   const { deployerAddr, governorAddr } = await getNamedAccounts();
   const sDeployer = await ethers.provider.getSigner(deployerAddr);
@@ -1564,6 +1592,7 @@ module.exports = {
   deployUniswapV3Pool,
   deployVaultValueChecker,
   deployWOusd,
+  deployWOeth,
   deployOETHSwapper,
   deployOUSDSwapper,
   upgradeNativeStakingSSVStrategy,

contracts/deploy/mainnet/001_core.js

@@ -23,6 +23,7 @@ const {
   deployUniswapV3Pool,
   deployVaultValueChecker,
   deployWOusd,
+  deployWOeth,
   deployOETHSwapper,
   deployOUSDSwapper,
 } = require("../deployActions");
@@ -54,6 +55,7 @@ const main = async () => {
   await deployUniswapV3Pool();
   await deployVaultValueChecker();
   await deployWOusd();
+  await deployWOeth();
   await deployOETHSwapper();
   await deployOUSDSwapper();
   console.log("001_core deploy done.");

contracts/deploy/mainnet/099_upgrade_woeth.js

@@ -0,0 +1,44 @@
+const { deploymentWithGovernanceProposal } = require("../../utils/deploy");
+
+module.exports = deploymentWithGovernanceProposal(
+  {
+    deployName: "099_upgrade_woeth",
+    forceDeploy: false,
+    //forceSkip: true,
+    reduceQueueTime: true,
+    deployerIsProposer: false,
+    // proposalId:
+  },
+  async ({ deployWithConfirmation, ethers }) => {
+    const cOETHProxy = await ethers.getContract("OETHProxy");
+    const cWOETHProxy = await ethers.getContract("WOETHProxy");
+
+    const dWOETHImpl = await deployWithConfirmation("WOETH", [
+      cOETHProxy.address,
+      "Wrapped OETH",
+      "WOETH",
+    ]);
+
+    const cWOETH = await ethers.getContractAt("WOETH", cWOETHProxy.address);
+
+    // Governance Actions
+    // ----------------
+    return {
+      name: `Upgrade WOETH to a new implementation.`,
+      actions: [
+        // 1. Upgrade WOETH
+        {
+          contract: cWOETHProxy,
+          signature: "upgradeTo(address)",
+          args: [dWOETHImpl.address],
+        },
+        // 2. Run the second initializer
+        {
+          contract: cWOETH,
+          signature: "initialize2()",
+          args: [],
+        },
+      ],
+    };
+  }
+);

contracts/test/_fixture.js

@@ -226,12 +226,8 @@ const defaultFixture = deployments.createFixture(async () => {
   );
   const oeth = await ethers.getContractAt("OETH", oethProxy.address);
 
-  let woeth, woethProxy;
-
-  if (isFork) {
-    woethProxy = await ethers.getContract("WOETHProxy");
-    woeth = await ethers.getContractAt("WOETH", woethProxy.address);
-  }
+  const woethProxy = await ethers.getContract("WOETHProxy");
+  const woeth = await ethers.getContractAt("WOETH", woethProxy.address);
 
   const harvesterProxy = await ethers.getContract("HarvesterProxy");
   const harvester = await ethers.getContractAt(
@@ -662,10 +658,15 @@ const defaultFixture = deployments.createFixture(async () => {
   if (!isFork) {
     await fundAccounts();
 
-    // Matt and Josh each have $100 OUSD
+    // Matt and Josh each have $100 OUSD & 100 OETH
     for (const user of [matt, josh]) {
       await dai.connect(user).approve(vault.address, daiUnits("100"));
       await vault.connect(user).mint(dai.address, daiUnits("100"), 0);
+
+      // Fund WETH contract
+      await hardhatSetBalance(user.address, "500");
+      await weth.connect(user).deposit({ value: oethUnits("100") });
+      await weth.connect(user).approve(oethVault.address, oethUnits("100"));
     }
   }
   return {