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

@@ -19,6 +19,7 @@
 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

@@ -7,29 +7,65 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.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;
+    uint256 public oethCreditsHighres;
+    bool private _oethCreditsInitialized;
+    uint256[48] private __gap;
 
-    constructor(
-        ERC20 underlying_,
-        string memory name_,
-        string memory symbol_
-    ) ERC20(name_, symbol_) ERC4626(underlying_) Governable() {}
+    // no need to set ERC20 name and symbol since they are overridden in WOETH & WOETHBase
+    constructor(ERC20 underlying_)
+        ERC20("", "")
+        ERC4626(underlying_)
+        Governable()
+    {}
 
     /**
      * @notice Enable OETH rebasing for this contract
      */
     function initialize() external onlyGovernor initializer {
         OETH(address(asset())).rebaseOptIn();
+
+        initialize2();
+    }
+
+    /**
+     * @notice secondary initializer that newly deployed contracts will execute as part
+     *         of primary initialize function and the existing contracts will have it called
+     *         as a governance operation.
+     */
+    function initialize2() public onlyGovernor {
+        require(!_oethCreditsInitialized, "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 = _getOETHCredits();
     }
 
     function name()
@@ -62,7 +98,95 @@ contract WOETH is ERC4626, Governable, Initializable {
         external
         onlyGovernor
     {
+        //@dev TODO: we could implement a feature where if anyone sends OETH directly 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 See {IERC4262-totalAssets} */
+    function totalAssets() public view override returns (uint256) {
+        uint256 creditsPerTokenHighres = OETH(asset())
+            .rebasingCreditsPerTokenHighres();
+
+        return (oethCreditsHighres).divPrecisely(creditsPerTokenHighres);
+    }
+
+    function _getOETHCredits()
+        internal
+        view
+        returns (uint256 oethCreditsHighres)
+    {
+        (oethCreditsHighres, , ) = OETH(asset()).creditsBalanceOfHighres(
+            address(this)
+        );
+    }
+
+    /** @dev See {IERC4262-deposit} */
+    function deposit(uint256 oethAmount, address receiver)
+        public
+        override
+        returns (uint256 woethAmount)
+    {
+        if (oethAmount == 0) return 0;
+
+        /**
+         * Initially we attempted to do the credits calculation within this contract and try
+         * to mimic OUSD's implementation. This way 1 external call less would be required. Due
+         * to a different way OUSD is calculating credits:
+         *  - always rounds credits up
+         *  - operates on final user balances before converting to credits
+         *  - doesn't perform additive / subtractive calculation with credits once they are converted
+         *    from balances
+         *
+         * We've decided that it is safer to read the credits diff directly from the OUSD contract
+         * and not face the risk of a compounding error in oethCreditsHighres that could result in
+         * inaccurate `convertToShares` & `convertToAssets` which consequently would result in faulty
+         * `previewMint` & `previewRedeem`. High enough error can result in different conversion rates
+         * which a flash loan entering via `deposit` and exiting via `redeem` (or entering via `mint`
+         * and exiting via `withdraw`) could take advantage of.
+         */
+        uint256 creditsBefore = _getOETHCredits();
+        woethAmount = super.deposit(oethAmount, receiver);
+        oethCreditsHighres += _getOETHCredits() - creditsBefore;
+    }
+
+    /** @dev See {IERC4262-mint} */
+    function mint(uint256 woethAmount, address receiver)
+        public
+        override
+        returns (uint256 oethAmount)
+    {
+        if (woethAmount == 0) return 0;
+
+        uint256 creditsBefore = _getOETHCredits();
+        oethAmount = super.mint(woethAmount, receiver);
+        oethCreditsHighres += _getOETHCredits() - creditsBefore;
+    }
+
+    /** @dev See {IERC4262-withdraw} */
+    function withdraw(
+        uint256 oethAmount,
+        address receiver,
+        address owner
+    ) public override returns (uint256 woethAmount) {
+        if (oethAmount == 0) return 0;
+
+        uint256 creditsBefore = _getOETHCredits();
+        woethAmount = super.withdraw(oethAmount, receiver, owner);
+        oethCreditsHighres -= creditsBefore - _getOETHCredits();
+    }
+
+    /** @dev See {IERC4262-redeem} */
+    function redeem(
+        uint256 woethAmount,
+        address receiver,
+        address owner
+    ) public override returns (uint256 oethAmount) {
+        if (woethAmount == 0) return 0;
+
+        uint256 creditsBefore = _getOETHCredits();
+        oethAmount = super.redeem(woethAmount, receiver, owner);
+        oethCreditsHighres -= creditsBefore - _getOETHCredits();
+    }
 }

contracts/contracts/token/WOETHBase.sol

@@ -10,9 +10,7 @@ import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
  */
 
 contract WOETHBase is WOETH {
-    constructor(ERC20 underlying_)
-        WOETH(underlying_, "Wrapped Super OETH", "wsuperOETHb")
-    {}
+    constructor(ERC20 underlying_) WOETH(underlying_) {}
 
     function name() public view virtual override returns (string memory) {
         return "Wrapped Super OETH";

contracts/deploy/deployActions.js

@@ -1516,6 +1516,26 @@ const deployWOusd = async () => {
   ](dWrappedOusdImpl.address, governorAddr, initData);
 };
 
+const deployWOeth = async () => {
+  const { deployerAddr, governorAddr } = await getNamedAccounts();
+  const sDeployer = await ethers.provider.getSigner(deployerAddr);
+
+  const oeth = await ethers.getContract("OETHProxy");
+  const dWrappedOethImpl = await deployWithConfirmation("WOETH", [
+    oeth.address,
+  ]);
+  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);
+};
+
 const deployOETHSwapper = async () => {
   const { deployerAddr, governorAddr } = await getNamedAccounts();
   const sDeployer = await ethers.provider.getSigner(deployerAddr);
@@ -1632,6 +1652,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/112_upgrade_woeth.js

@@ -0,0 +1,42 @@
+const { deploymentWithGovernanceProposal } = require("../../utils/deploy");
+
+module.exports = deploymentWithGovernanceProposal(
+  {
+    deployName: "112_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,
+    ]);
+
+    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

@@ -79,6 +79,9 @@ const simpleOETHFixture = deployments.createFixture(async () => {
   );
   const oeth = await ethers.getContractAt("OETH", oethProxy.address);
 
+  const cWOETHProxy = await ethers.getContract("WOETHProxy");
+  const woeth = await ethers.getContractAt("WOETH", cWOETHProxy.address);
+
   const oethHarvesterProxy = await ethers.getContract("OETHHarvesterProxy");
   const oethHarvester = await ethers.getContractAt(
     "OETHHarvester",
@@ -200,6 +203,7 @@ const simpleOETHFixture = deployments.createFixture(async () => {
     // OETH
     oethVault,
     oeth,
+    woeth,
     nativeStakingSSVStrategy,
     oethDripper,
     oethFixedRateDripper,
@@ -229,12 +233,12 @@ const getVaultAndTokenConracts = async () => {
   );
   const oeth = await ethers.getContractAt("OETH", oethProxy.address);
 
-  let woeth, woethProxy, mockNonRebasing, mockNonRebasingTwo;
+  let mockNonRebasing, mockNonRebasingTwo;
 
-  if (isFork) {
-    woethProxy = await ethers.getContract("WOETHProxy");
-    woeth = await ethers.getContractAt("WOETH", woethProxy.address);
-  } else {
+  const woethProxy = await ethers.getContract("WOETHProxy");
+  const woeth = await ethers.getContractAt("WOETH", woethProxy.address);
+
+  if (!isFork) {
     // Mock contracts for testing rebase opt out
     mockNonRebasing = await ethers.getContract("MockNonRebasing");
     await mockNonRebasing.setOUSD(ousd.address);
@@ -1062,14 +1066,21 @@ 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(vaultAndTokenConracts.vault.address, daiUnits("100"));
       await vaultAndTokenConracts.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(vaultAndTokenConracts.oethVault.address, oethUnits("100"));
     }
   }
   return {

contracts/test/helpers.js

@@ -119,6 +119,15 @@ chai.Assertion.addMethod(
   }
 );
 
+chai.Assertion.addMethod("totalSupply", async function (expected, message) {
+  const contract = this._obj;
+  const actual = await contract.totalSupply();
+  if (!BigNumber.isBigNumber(expected)) {
+    expected = parseUnits(expected, await decimalsFor(contract));
+  }
+  chai.expect(actual).to.equal(expected, message);
+});
+
 chai.Assertion.addMethod(
   "assetBalanceOf",
   async function (expected, asset, message) {