Merge pull request #514 from keep-network/requestNewKeep-acl

ACL for RequestNewKeep

Only Deposit instances should be able to call requestNewKeep; implement
using the TDT id as a proxy for authorization.

implementation/contracts/proxy/DepositFactory.sol

@@ -70,6 +70,8 @@ contract DepositFactory is CloneFactory, TBTCSystemAuthority{
     function createDeposit (uint256 _lotSizeSatoshis) public payable returns(address) {
         address cloneAddress = createClone(masterDepositAddress);
 
+        TBTCDepositToken(tbtcDepositToken).mint(msg.sender, uint256(cloneAddress));
+
         Deposit deposit = Deposit(address(uint160(cloneAddress)));
         deposit.initialize(address(this));
         deposit.createNewDeposit.value(msg.value)(
@@ -83,8 +85,6 @@ contract DepositFactory is CloneFactory, TBTCSystemAuthority{
                 _lotSizeSatoshis
             );
 
-        TBTCDepositToken(tbtcDepositToken).mint(msg.sender, uint256(cloneAddress));
-
         emit DepositCloneCreated(cloneAddress);
 
         return cloneAddress;

implementation/contracts/system/TBTCSystem.sol

@@ -13,6 +13,7 @@ import {ITBTCSystem} from "../interfaces/ITBTCSystem.sol";
 import {IBTCETHPriceFeed} from "../interfaces/IBTCETHPriceFeed.sol";
 import {DepositLog} from "../DepositLog.sol";
 
+import {TBTCDepositToken} from "./TBTCDepositToken.sol";
 import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
 import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 
@@ -37,6 +38,7 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
     uint256 pausedTimestamp;
     uint256 constant pausedDuration = 10 days;
 
+    TBTCDepositToken tbtcDepositToken;
     address public keepVendor;
     address public priceFeed;
     address public relay;
@@ -77,7 +79,7 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
         uint256 _keepSize
     ) external onlyOwner {
         require(!_initialized, "already initialized");
-
+        tbtcDepositToken = TBTCDepositToken(_tbtcDepositToken);
         keepVendor = _keepVendor;
         VendingMachine(_vendingMachine).setExternalAddresses(
             _tbtcToken,
@@ -267,6 +269,7 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
         payable
         returns (address)
     {
+        require(tbtcDepositToken.exists(uint256(msg.sender)), "Caller must be a Deposit contract");
         IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor);
         IBondedECDSAKeepFactory _keepFactory = IBondedECDSAKeepFactory(_keepVendor.selectFactory());
         return _keepFactory.openKeep.value(msg.value)(_n, _m, msg.sender, _bond);

implementation/test/TBTCSystemTest.js

@@ -10,9 +10,14 @@ const TBTCSystem = contract.fromArtifact("TBTCSystem")
 describe("TBTCSystem", async function() {
   let tbtcSystem
   let ecdsaKeepFactory
+  let tdt
 
   before(async () => {
-    const {tbtcSystemStub, ecdsaKeepFactoryStub} = await deployAndLinkAll(
+    const {
+      tbtcSystemStub,
+      ecdsaKeepFactoryStub,
+      tbtcDepositToken,
+    } = await deployAndLinkAll(
       [],
       // Though deployTestDeposit deploys a TBTCSystemStub for us, we want to
       // test TBTCSystem itself.
@@ -21,24 +26,28 @@ describe("TBTCSystem", async function() {
     // Refer to this correctly throughout the rest of the test.
     tbtcSystem = tbtcSystemStub
     ecdsaKeepFactory = ecdsaKeepFactoryStub
+    tdt = tbtcDepositToken
   })
 
   describe("requestNewKeep()", async () => {
     let openKeepFee
+    const tdtOwner = accounts[1]
+    const keepOwner = accounts[2]
+
     before(async () => {
       openKeepFee = await ecdsaKeepFactory.openKeepFeeEstimate.call()
+      await tdt.forceMint(tdtOwner, web3.utils.toBN(keepOwner))
     })
-    it("sends caller as owner to open new keep", async () => {
-      const expectedKeepOwner = accounts[2]
 
+    it("sends caller as owner to open new keep", async () => {
       await tbtcSystem.requestNewKeep(5, 10, 0, {
-        from: expectedKeepOwner,
+        from: keepOwner,
         value: openKeepFee,
       })
-      const keepOwner = await ecdsaKeepFactory.keepOwner.call()
+      const actualKeepOwner = await ecdsaKeepFactory.keepOwner.call()
 
-      expect(expectedKeepOwner, "incorrect keep owner address").to.equal(
-        keepOwner,
+      expect(keepOwner, "incorrect keep owner address").to.equal(
+        actualKeepOwner,
       )
     })
 
@@ -47,6 +56,7 @@ describe("TBTCSystem", async function() {
 
       const result = await tbtcSystem.requestNewKeep.call(5, 10, 0, {
         value: openKeepFee,
+        from: keepOwner,
       })
 
       expect(expectedKeepAddress, "incorrect keep address").to.equal(result)
@@ -55,7 +65,10 @@ describe("TBTCSystem", async function() {
     it("forwards value to keep factory", async () => {
       const initialBalance = await web3.eth.getBalance(ecdsaKeepFactory.address)
 
-      await tbtcSystem.requestNewKeep(5, 10, 0, {value: openKeepFee})
+      await tbtcSystem.requestNewKeep(5, 10, 0, {
+        value: openKeepFee,
+        from: keepOwner,
+      })
 
       const finalBalance = await web3.eth.getBalance(ecdsaKeepFactory.address)
       const balanceCheck = new BN(finalBalance).sub(new BN(initialBalance))
@@ -64,6 +77,26 @@ describe("TBTCSystem", async function() {
         "TBTCSystem did not correctly forward value to keep factory",
       ).to.eq.BN(openKeepFee)
     })
+
+    it("reverts if caller does not match a valid TDT", async () => {
+      await expectRevert(
+        tbtcSystem.requestNewKeep(5, 10, 0, {
+          value: openKeepFee,
+          from: accounts[0],
+        }),
+        "Caller must be a Deposit contract",
+      )
+    })
+
+    it("reverts if caller is the owner of a valid TDT", async () => {
+      await expectRevert(
+        tbtcSystem.requestNewKeep(5, 10, 0, {
+          value: openKeepFee,
+          from: tdtOwner,
+        }),
+        "Caller must be a Deposit contract",
+      )
+    })
   })
 
   describe("setSignerFeeDivisor", async () => {