Certik Audit 2021 05 14

stakewithus · May 14, 2021 · 8e72012 · 8e72012
1 parent 057d59a
commit 8e72012
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 24 deletions.
diff --git a/audit/certik-2021-05-14.pdf b/audit/certik-2021-05-14.pdf
diff --git a/contracts/StrategyERC20_V3.sol b/contracts/StrategyERC20_V3.sol
@@ -22,6 +22,7 @@ abstract contract StrategyERC20_V3 is IStrategyERC20_V3 {
     using SafeMath for uint;
 
     address public override admin;
+    address public nextAdmin;
     address public override controller;
     address public immutable override vault;
     address public immutable override underlying;
@@ -74,9 +75,16 @@ abstract contract StrategyERC20_V3 is IStrategyERC20_V3 {
         _;
     }
 
-    function setAdmin(address _admin) external onlyAdmin {
-        require(_admin != address(0), "admin = zero address");
-        admin = _admin;
+    function setNextAdmin(address _nextAdmin) external onlyAdmin {
+        require(_nextAdmin != admin, "next admin = current");
+        // allow next admin = zero address (cancel next admin)
+        nextAdmin = _nextAdmin;
+    }
+
+    function acceptAdmin() external {
+        require(msg.sender == nextAdmin, "!next admin");
+        admin = msg.sender;
+        nextAdmin = address(0);
     }
 
     function setController(address _controller) external onlyAdmin {

diff --git a/contracts/StrategyETH_V3.sol b/contracts/StrategyETH_V3.sol
@@ -22,6 +22,7 @@ abstract contract StrategyETH_V3 is IStrategyETH_V3 {
     using SafeMath for uint;
 
     address public override admin;
+    address public nextAdmin;
     address public override controller;
     address public immutable override vault;
     // Placeholder address to indicate that this is ETH strategy
@@ -73,9 +74,16 @@ abstract contract StrategyETH_V3 is IStrategyETH_V3 {
         _;
     }
 
-    function setAdmin(address _admin) external onlyAdmin {
-        require(_admin != address(0), "admin = zero address");
-        admin = _admin;
+    function setNextAdmin(address _nextAdmin) external onlyAdmin {
+        require(_nextAdmin != admin, "next admin = current");
+        // allow next admin = zero address (cancel next admin)
+        nextAdmin = _nextAdmin;
+    }
+
+    function acceptAdmin() external {
+        require(msg.sender == nextAdmin, "!next admin");
+        admin = msg.sender;
+        nextAdmin = address(0);
     }
 
     function setController(address _controller) external onlyAdmin {

diff --git a/test/unit/StrategyERC20_V3/test-accept-admin.ts b/test/unit/StrategyERC20_V3/test-accept-admin.ts
@@ -0,0 +1,29 @@
+import chai from "chai"
+import { StrategyERC20V3TestInstance } from "../../../types"
+import { ZERO_ADDRESS } from "../../util"
+import _setup from "./setup"
+
+contract("StrategyERC20_V3", (accounts) => {
+  const refs = _setup(accounts)
+  const { admin } = refs
+
+  let strategy: StrategyERC20V3TestInstance
+  beforeEach(async () => {
+    strategy = refs.strategy
+    await strategy.setNextAdmin(accounts[1], { from: admin })
+  })
+
+  describe("acceptAdmin", () => {
+    it("should set admin", async () => {
+      await strategy.acceptAdmin({ from: accounts[1] })
+      assert.equal(await strategy.admin(), accounts[1])
+      assert.equal(await strategy.nextAdmin(), ZERO_ADDRESS)
+    })
+
+    it("should reject if caller not next admin", async () => {
+      await chai
+        .expect(strategy.acceptAdmin({ from: accounts[2] }))
+        .to.be.rejectedWith("!next admin")
+    })
+  })
+})
diff --git a/test/unit/StrategyERC20_V3/test-set-admin.ts → ...t/StrategyERC20_V3/test-set-next-admin.ts b/test/unit/StrategyERC20_V3/test-set-admin.ts → ...t/StrategyERC20_V3/test-set-next-admin.ts
@@ -1,6 +1,5 @@
 import chai from "chai"
 import { StrategyERC20V3TestInstance } from "../../../types"
-import { ZERO_ADDRESS } from "../../util"
 import _setup from "./setup"
 
 contract("StrategyERC20_V3", (accounts) => {
@@ -12,23 +11,23 @@ contract("StrategyERC20_V3", (accounts) => {
     strategy = refs.strategy
   })
 
-  describe("setAdmin", () => {
-    it("should set admin", async () => {
-      await strategy.setAdmin(accounts[1], { from: admin })
+  describe("setNextAdmin", () => {
+    it("should set next admin", async () => {
+      await strategy.setNextAdmin(accounts[1], { from: admin })
 
-      assert.equal(await strategy.admin(), accounts[1])
+      assert.equal(await strategy.nextAdmin(), accounts[1])
     })
 
     it("should reject if caller not admin", async () => {
       await chai
-        .expect(strategy.setAdmin(accounts[1], { from: accounts[1] }))
+        .expect(strategy.setNextAdmin(accounts[1], { from: accounts[1] }))
         .to.be.rejectedWith("!admin")
     })
 
-    it("should reject zero address", async () => {
+    it("should reject if next admin = current", async () => {
       await chai
-        .expect(strategy.setAdmin(ZERO_ADDRESS, { from: admin }))
-        .to.be.rejectedWith("admin = zero address")
+        .expect(strategy.setNextAdmin(admin, { from: admin }))
+        .to.be.rejectedWith("next admin = current")
     })
   })
 })
diff --git a/test/unit/StrategyETH_V3/test-accept-admin.ts b/test/unit/StrategyETH_V3/test-accept-admin.ts
@@ -0,0 +1,29 @@
+import chai from "chai"
+import { StrategyETHV3TestInstance } from "../../../types"
+import { ZERO_ADDRESS } from "../../util"
+import _setup from "./setup"
+
+contract("StrategyETH_V3", (accounts) => {
+  const refs = _setup(accounts)
+  const { admin } = refs
+
+  let strategy: StrategyETHV3TestInstance
+  beforeEach(async () => {
+    strategy = refs.strategy
+    await strategy.setNextAdmin(accounts[1], { from: admin })
+  })
+
+  describe("acceptAdmin", () => {
+    it("should set admin", async () => {
+      await strategy.acceptAdmin({ from: accounts[1] })
+      assert.equal(await strategy.admin(), accounts[1])
+      assert.equal(await strategy.nextAdmin(), ZERO_ADDRESS)
+    })
+
+    it("should reject if caller not next admin", async () => {
+      await chai
+        .expect(strategy.acceptAdmin({ from: accounts[2] }))
+        .to.be.rejectedWith("!next admin")
+    })
+  })
+})
diff --git a/test/unit/StrategyETH_V3/test-set-admin.ts → ...nit/StrategyETH_V3/test-set-next-admin.ts b/test/unit/StrategyETH_V3/test-set-admin.ts → ...nit/StrategyETH_V3/test-set-next-admin.ts
@@ -1,6 +1,5 @@
 import chai from "chai"
 import { StrategyETHV3TestInstance } from "../../../types"
-import { ZERO_ADDRESS } from "../../util"
 import _setup from "./setup"
 
 contract("StrategyETH_V3", (accounts) => {
@@ -12,23 +11,23 @@ contract("StrategyETH_V3", (accounts) => {
     strategy = refs.strategy
   })
 
-  describe("setAdmin", () => {
-    it("should set admin", async () => {
-      await strategy.setAdmin(accounts[1], { from: admin })
+  describe("setNextAdmin", () => {
+    it("should set next admin", async () => {
+      await strategy.setNextAdmin(accounts[1], { from: admin })
 
-      assert.equal(await strategy.admin(), accounts[1])
+      assert.equal(await strategy.nextAdmin(), accounts[1])
     })
 
     it("should reject if caller not admin", async () => {
       await chai
-        .expect(strategy.setAdmin(accounts[1], { from: accounts[1] }))
+        .expect(strategy.setNextAdmin(accounts[1], { from: accounts[1] }))
         .to.be.rejectedWith("!admin")
     })
 
-    it("should reject zero address", async () => {
+    it("should reject if next admin = current", async () => {
       await chai
-        .expect(strategy.setAdmin(ZERO_ADDRESS, { from: admin }))
-        .to.be.rejectedWith("admin = zero address")
+        .expect(strategy.setNextAdmin(admin, { from: admin }))
+        .to.be.rejectedWith("next admin = current")
     })
   })
 })