Merge pull request #4 from pooltogether/fix/15

fix(contract): check for address zero in constructor

.github/workflows/main.yml

@@ -0,0 +1,23 @@
+name: Coveralls
+
+on: ["push", "pull_request"]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Use Node.js 16.x
+        uses: actions/setup-node@v2
+        with:
+          node-version: 16.x
+      - name: yarn, compile, hint, coverage
+        run: |
+          yarn
+          yarn compile
+          yarn hint
+          yarn coverage
+      - name: Coveralls
+        uses: coverallsapp/github-action@master
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}

.prettierrc.json

@@ -2,15 +2,15 @@
     "trailingComma": "es5",
     "tabWidth": 4,
     "useTabs": false,
-    "singleQuote": false,
     "printWidth": 140,
     "overrides": [
       {
         "files": "*.ts",
         "options": {
           "parser": "typescript",
           "trailingComma": "all",
-          "arrowParens": "always"
+          "arrowParens": "always",
+          "singleQuote": true
         }
       },
       {

.solhint.json

@@ -0,0 +1,20 @@
+{
+  "extends": "solhint:recommended",
+  "plugins": [],
+  "rules": {
+      "avoid-sha3": "warn",
+      "avoid-suicide": "error",
+      "code-complexity": "warn",
+      "compiler-version": ["error", "0.8.2"],
+      "const-name-snakecase": "warn",
+      "func-order": "off",
+      "function-max-lines": "warn",
+      "func-visibility": ["warn", {"ignoreConstructors": true}],
+      "mark-callable-contracts": "off",
+      "max-line-length": ["warn", 160],
+      "no-empty-blocks": "off",
+      "not-rely-on-time": "off",
+      "private-vars-leading-underscore": "off",
+      "reason-string": ["warn", {"maxLength": 64}]
+  }
+}

abis/MStableYieldSource.json

@@ -124,6 +124,19 @@
     "stateMutability": "view",
     "type": "function"
   },
+  {
+    "inputs": [
+      {
+        "internalType": "bytes32",
+        "name": "c__0x8ba2d519",
+        "type": "bytes32"
+      }
+    ],
+    "name": "c_0x8ba2d519",
+    "outputs": [],
+    "stateMutability": "pure",
+    "type": "function"
+  },
   {
     "inputs": [],
     "name": "depositToken",

abis/MStableYieldSourceHarness.json

@@ -124,6 +124,32 @@
     "stateMutability": "view",
     "type": "function"
   },
+  {
+    "inputs": [
+      {
+        "internalType": "bytes32",
+        "name": "c__0x8ba2d519",
+        "type": "bytes32"
+      }
+    ],
+    "name": "c_0x8ba2d519",
+    "outputs": [],
+    "stateMutability": "pure",
+    "type": "function"
+  },
+  {
+    "inputs": [
+      {
+        "internalType": "bytes32",
+        "name": "c__0xfc6e550e",
+        "type": "bytes32"
+      }
+    ],
+    "name": "c_0xfc6e550e",
+    "outputs": [],
+    "stateMutability": "pure",
+    "type": "function"
+  },
   {
     "inputs": [
       {

contracts/test/ATokenYieldSourceHarness.sol → contracts/test/MStableYieldSourceHarness.sol

@@ -15,7 +15,11 @@ contract MStableYieldSourceHarness is MStableYieldSource {
 
     constructor(ISavingsContractV2 _savings) MStableYieldSource(_savings) {}
 
-    function decreaseAllowance(IERC20 token, address spender, uint256 value) external returns (bool) {
+    function decreaseAllowance(
+        IERC20 token,
+        address spender,
+        uint256 value
+    ) external returns (bool) {
         token.safeDecreaseAllowance(spender, value);
         return true;
     }

contracts/yield-source/MStableYieldSource.sol

@@ -35,6 +35,8 @@ contract MStableYieldSource is IYieldSource, Ownable, ReentrancyGuard {
     event Redeemed(address indexed from, uint256 requestedAmount, uint256 actualAmount);
 
     constructor(ISavingsContractV2 _savings) ReentrancyGuard() {
+        require(address(_savings) != address(0), "MStableYieldSource/savings-not-zero-address");
+
         // As immutable storage variables can not be accessed in the constructor,
         // create in-memory variables that can be used instead.
         IERC20 mAssetMemory = IERC20(_savings.underlying());

hardhat.config.ts

@@ -10,6 +10,7 @@ const hardhatConfig = {
     networks: {
         hardhat: {
             allowUnlimitedContractSize: true,
+            initialBaseFeePerGas: 0, // remove when the following issue is fixed: https://github.com/sc-forks/solidity-coverage/issues/652
         },
         localhost: { url: "http://localhost:7545" },
         fork: {

test/MStableYieldSource.test.ts

@@ -1,6 +1,6 @@
-import debug from "debug";
+import debug from 'debug';
 
-import { JsonRpcProvider } from "@ethersproject/providers";
+import { JsonRpcProvider } from '@ethersproject/providers';
 import {
     AssetProxy__factory,
     MockERC20,
@@ -12,19 +12,19 @@ import {
     MockSavingsManager__factory,
     SavingsContract,
     SavingsContract__factory,
-} from "@mstable/protocol";
-import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
-import { MassetMachine, StandardAccounts } from "@utils/machines";
-import { expect } from "chai";
-import { ethers, waffle } from "hardhat";
+} from '@mstable/protocol';
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
+import { MassetMachine, StandardAccounts } from '@utils/machines';
+import { expect } from 'chai';
+import { ethers, waffle } from 'hardhat';
 
-import { MStableYieldSourceHarness } from "../types/pooltogether";
+import { MStableYieldSourceHarness } from '../types/pooltogether';
 
-const { MaxUint256 } = ethers.constants;
+const { AddressZero, MaxUint256 } = ethers.constants;
 
 const toWei = ethers.utils.parseEther;
 
-describe("MStableYieldSource", () => {
+describe('MStableYieldSource', () => {
     let contractsOwner: SignerWithAddress;
     let yieldSourceOwner: SignerWithAddress;
     let wallet2: SignerWithAddress;
@@ -41,11 +41,24 @@ describe("MStableYieldSource", () => {
     let savings: SavingsContract;
     let savingsFactory: SavingsContract__factory;
 
+    let isConstructorTest = false;
+
+    const initializeMStableYieldSource = async (savingsAddress: string) => {
+        const mStableYieldSourceFactory = await ethers.getContractFactory('MStableYieldSourceHarness');
+        const hardhatMStableYieldSource = await mStableYieldSourceFactory.deploy(savingsAddress);
+
+        return (mStableYieldSource = ((await ethers.getContractAt(
+            'MStableYieldSourceHarness',
+            hardhatMStableYieldSource.address,
+            contractsOwner,
+        )) as unknown) as MStableYieldSourceHarness);
+    };
+
     const createNewSavingsContract = async (): Promise<void> => {
         savingsFactory = new SavingsContract__factory(sa.default.signer);
 
         const impl = await savingsFactory.deploy(nexus.address, mUSD.address);
-        const data = impl.interface.encodeFunctionData("initialize", [sa.default.address, "Savings Credit", "imUSD"]);
+        const data = impl.interface.encodeFunctionData('initialize', [sa.default.address, 'Savings Credit', 'imUSD']);
         const proxy = await new AssetProxy__factory(sa.default.signer).deploy(impl.address, sa.dummy4.address, data);
 
         savings = savingsFactory.attach(proxy.address);
@@ -60,35 +73,53 @@ describe("MStableYieldSource", () => {
 
         provider = waffle.provider;
 
-        debug("Deploying MStableYieldSource instance...");
+        debug('Deploying MStableYieldSource instance...');
 
         mAssetMachine = await new MassetMachine().initAccounts(accounts);
 
         sa = mAssetMachine.sa;
         nexus = await new MockNexus__factory(sa.default.signer).deploy(sa.governor.address, sa.governor.address, sa.dummy1.address);
 
         mUSD = await new MockMasset__factory(sa.default.signer).deploy(
-            "mStable USD",
-            "mUSD",
+            'mStable USD',
+            'mUSD',
             18,
             sa.fundManager.address,
-            toWei("100000000"),
+            toWei('100000000'),
         );
-        bAsset = await new MockERC20__factory(sa.default.signer).deploy("Mock1", "MK1", 18, sa.fundManager.address, toWei("100000000"));
-        bAsset2 = await new MockERC20__factory(sa.default.signer).deploy("Mock2", "MK2", 18, sa.fundManager.address, toWei("100000000"));
+
+        bAsset = await new MockERC20__factory(sa.default.signer).deploy('Mock1', 'MK1', 18, sa.fundManager.address, toWei('100000000'));
+        bAsset2 = await new MockERC20__factory(sa.default.signer).deploy('Mock2', 'MK2', 18, sa.fundManager.address, toWei('100000000'));
 
         bAssets = [bAsset, bAsset2];
 
-        createNewSavingsContract();
+        await createNewSavingsContract();
 
-        const mStableYieldSourceFactory = await ethers.getContractFactory("MStableYieldSourceHarness");
-        const hardhatMStableYieldSource = await mStableYieldSourceFactory.deploy(savings.address);
+        if (!isConstructorTest) {
+            mStableYieldSource = await initializeMStableYieldSource(savings.address);
+        }
+    });
 
-        mStableYieldSource = (await ethers.getContractAt(
-            "MStableYieldSourceHarness",
-            hardhatMStableYieldSource.address,
-            contractsOwner,
-        )) as unknown as MStableYieldSourceHarness;
+    describe('constructor()', () => {
+        before(() => {
+            isConstructorTest = true;
+        });
+
+        after(() => {
+            isConstructorTest = false;
+        });
+
+        it('should initialize MStableYieldSource', async () => {
+            const mStableYieldSource = await initializeMStableYieldSource(savings.address);
+
+            expect(await mStableYieldSource.owner()).to.equal(contractsOwner.address);
+            expect(await mStableYieldSource.savings()).to.equal(savings.address);
+            expect(await mStableYieldSource.mAsset()).to.equal(mUSD.address);
+        });
+
+        it('should fail if savings is address zero', async () => {
+            await expect(initializeMStableYieldSource(AddressZero)).to.be.revertedWith('MStableYieldSource/savings-not-zero-address');
+        });
     });
 
     describe('approveMaxAmount()', () => {
@@ -102,12 +133,14 @@ describe("MStableYieldSource", () => {
         });
 
         it('should fail if not owner', async () => {
-            await expect(mStableYieldSource.connect(wallet2).callStatic.approveMaxAmount()).to.be.revertedWith('Ownable: caller is not the owner');
+            await expect(mStableYieldSource.connect(wallet2).callStatic.approveMaxAmount()).to.be.revertedWith(
+                'Ownable: caller is not the owner',
+            );
         });
     });
 
-    describe("depositToken()", () => {
-        it("should return mAsset token address", async () => {
+    describe('depositToken()', () => {
+        it('should return mAsset token address', async () => {
             expect(await mStableYieldSource.depositToken()).to.equal(mUSD.address);
         });
     });