Merge pull request #436 from keep-network/forward-value-to-keep

Forward value to keep

we forward initial value to keepFactory. This is part of an effort to get
rid of funder bond and related logic.

implementation/contracts/deposit/DepositFunding.sol

@@ -56,14 +56,13 @@ library DepositFunding {
 
         require(_system.getAllowNewDeposits(), "Opening new deposits is currently disabled.");
         require(_d.inStart(), "Deposit setup already requested");
-        /* solium-disable-next-line value-in-payable */
-        require(msg.value == TBTCConstants.getFunderBondAmount(), "incorrect funder bond amount");
         require(_system.isAllowedLotSize(_lotSize), "provided lot size not supported");
         // TODO: Whole value is stored as funder bond in the deposit, but part
         // of it should be transferred to keep: https://github.com/keep-network/tbtc/issues/297
         _d.lotSizeSatoshis = _lotSize;
         uint256 _bondRequirement = _lotSize.mul(_system.getInitialCollateralizedPercent()).div(100);
-        _d.keepAddress = _system.requestNewKeep(_m, _n, _bondRequirement);
+        /* solium-disable-next-line value-in-payable */
+        _d.keepAddress = _system.requestNewKeep.value(msg.value)(_m, _n, _bondRequirement);
         _d.signerFeeDivisor = _system.getSignerFeeDivisor();
         _d.undercollateralizedThresholdPercent = _system.getUndercollateralizedThresholdPercent();
         _d.severelyUndercollateralizedThresholdPercent = _system.getSeverelyUndercollateralizedThresholdPercent();

implementation/contracts/deposit/DepositLiquidation.sol

@@ -39,7 +39,7 @@ library DepositLiquidation {
         bytes memory _preimage
     ) public returns (bool _isFraud) {
         IBondedECDSAKeep _keep = IBondedECDSAKeep(_d.keepAddress);
-        return _keep.submitSignatureFraud(_d.keepAddress, _v, _r, _s, _signedDigest, _preimage);
+        return _keep.submitSignatureFraud(_v, _r, _s, _signedDigest, _preimage);
     }
 
     /// @notice     Determines the collateralization percentage of the signing group

implementation/contracts/deposit/DepositUtils.sol

@@ -317,7 +317,7 @@ library DepositUtils {
     /// @return     The amount of bonded ETH in wei
     function fetchBondAmount(Deposit storage _d) public view returns (uint256) {
         IBondedECDSAKeep _keep = IBondedECDSAKeep(_d.keepAddress);
-        return _keep.checkBondAmount(_d.keepAddress);
+        return _keep.checkBondAmount();
     }
 
     /// @notice         Convert a LE bytes8 to a uint256
@@ -372,7 +372,7 @@ library DepositUtils {
     function seizeSignerBonds(Deposit storage _d) internal returns (uint256) {
         uint256 _preCallBalance = address(this).balance;
         IBondedECDSAKeep _keep = IBondedECDSAKeep(_d.keepAddress);
-        _keep.seizeSignerBonds(_d.keepAddress);
+        _keep.seizeSignerBonds();
         uint256 _postCallBalance = address(this).balance;
         require(_postCallBalance > _preCallBalance, "No funds received, unexpected");
         return _postCallBalance.sub(_preCallBalance);

implementation/contracts/system/TBTCSystem.sol

@@ -217,6 +217,17 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
         return IRelay(relay).getPrevEpochDifficulty();
     }
 
+    /// @notice Gets a fee estimate for creating a new Deposit.
+    /// @return Uint256 estimate.
+    function createNewDepositFeeEstimate()
+        external
+        returns (uint256)
+    {
+        IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor);
+        IBondedECDSAKeepFactory _keepFactory = IBondedECDSAKeepFactory(_keepVendor.selectFactory());
+        return _keepFactory.openKeepFeeEstimate();
+    }
+
     /// @notice Request a new keep opening.
     /// @param _m Minimum number of honest keep members required to sign.
     /// @param _n Number of members in the keep.
@@ -228,6 +239,6 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
     {
         IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor);
         IBondedECDSAKeepFactory _keepFactory = IBondedECDSAKeepFactory(_keepVendor.selectFactory());
-        return _keepFactory.openKeep(_n, _m, msg.sender, _bond);
+        return _keepFactory.openKeep.value(msg.value)(_n, _m, msg.sender, _bond);
     }
 }

implementation/package.json

@@ -20,7 +20,7 @@
   "author": "James Prestwich",
   "license": "UNLICENSED",
   "dependencies": {
-    "@keep-network/keep-ecdsa": "^0.7.5",
+    "@keep-network/keep-ecdsa": "^0.7.7",
     "@summa-tx/bitcoin-spv-sol": "^2.2.0",
     "@summa-tx/relay-sol": "^1.1.0",
     "@truffle/hdwallet-provider": "^1.0.26",

implementation/test/DepositFactoryTest.js

@@ -1,4 +1,5 @@
 import deployTestDeposit from './helpers/deployTestDeposit'
+import expectThrow from './helpers/expectThrow'
 
 import BN from 'bn.js'
 import utils from './utils'
@@ -10,12 +11,15 @@ const ECDSAKeepStub = artifacts.require('ECDSAKeepStub')
 const Deposit = artifacts.require('Deposit')
 const TestDeposit = artifacts.require('TestDeposit')
 
+const TBTCSystem = artifacts.require('TBTCSystem')
+
 contract('DepositFactory', () => {
-  const funderBondAmount = new BN('10').pow(new BN('5'))
+  const openKeepFee = new BN('123456') // set in ECDAKeepFactory
   const fullBtc = 100000000
 
   describe('createDeposit()', async () => {
     let depositFactory
+    let ecdsaKeepFactoryStub
 
     before(async () => {
       // To properly test createDeposit, we deploy the real Deposit contract and
@@ -30,12 +34,12 @@ contract('DepositFactory', () => {
 
       await depositFactory.createDeposit(
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
 
       await depositFactory.createDeposit(
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
 
       const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' })
@@ -46,24 +50,33 @@ contract('DepositFactory', () => {
       assert.notEqual(eventList[0].returnValues.depositCloneAddress, eventList[1].returnValues.depositCloneAddress, 'clone addresses should not be equal')
     })
 
-    it('correctly forwards value to Deposit', async () => {
-      const blockNumber = await web3.eth.getBlockNumber()
+    it('correctly forwards value to keep factory', async () => {
+      // Use real TBTCSystem contract to validate value forwarding:
+      // DepositFactory -> Deposit -> TBTCSystem -> ECDSAKeepFactory
+      ({
+        ecdsaKeepFactoryStub,
+        depositFactory,
+      } = await deployTestDeposit([], { 'TestDeposit': Deposit, 'TBTCSystemStub': TBTCSystem }))
 
       await depositFactory.createDeposit(
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
-      const eventList = await depositFactory.getPastEvents(
-        'DepositCloneCreated',
-        {
-          fromBlock: blockNumber,
-          toBlock: 'latest',
-        })
-
-      const depositAddress = eventList[eventList.length - 1].returnValues.depositCloneAddress
+      expect(
+        await web3.eth.getBalance(ecdsaKeepFactoryStub.address),
+        'Factory did not correctly forward value on Deposit creation'
+      ).to.eq.BN(openKeepFee)
+    })
 
-      const balance = await web3.eth.getBalance(depositAddress)
-      assert.equal(balance, funderBondAmount, 'Factory did not correctly forward value on Deposit creation')
+    it('reverts if insufficient fee is provided', async () => {
+      const badOpenKeepFee = openKeepFee.sub(new BN(1))
+      await expectThrow(
+        depositFactory.createDeposit(
+          fullBtc,
+          { value: badOpenKeepFee }
+        ),
+        'Insufficient value for new keep creation'
+      )
     })
   })
 
@@ -95,12 +108,12 @@ contract('DepositFactory', () => {
 
       await depositFactory.createDeposit(
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
 
       await depositFactory.createDeposit(
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
 
       const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' })
@@ -155,7 +168,7 @@ contract('DepositFactory', () => {
         1,
         1,
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
 
       await testDeposit.setKeepAddress(keep.address)
@@ -171,7 +184,7 @@ contract('DepositFactory', () => {
 
       await depositFactory.createDeposit(
         fullBtc,
-        { value: funderBondAmount }
+        { value: openKeepFee }
       )
 
       const eventList = await depositFactory.getPastEvents('DepositCloneCreated', { fromBlock: blockNumber, toBlock: 'latest' })

implementation/test/TBTCSystemTest.js

@@ -34,10 +34,14 @@ contract('TBTCSystem', (accounts) => {
   })
 
   describe('requestNewKeep()', async () => {
+    let openKeepFee
+    before(async () => {
+      openKeepFee = await ecdsaKeepFactory.openKeepFeeEstimate.call()
+    })
     it('sends caller as owner to open new keep', async () => {
       const expectedKeepOwner = accounts[2]
 
-      await tbtcSystem.requestNewKeep(5, 10, 0, { from: expectedKeepOwner })
+      await tbtcSystem.requestNewKeep(5, 10, 0, { from: expectedKeepOwner, value: openKeepFee })
       const keepOwner = await ecdsaKeepFactory.keepOwner.call()
 
       assert.equal(expectedKeepOwner, keepOwner, 'incorrect keep owner address')
@@ -46,10 +50,20 @@ contract('TBTCSystem', (accounts) => {
     it('returns keep address', async () => {
       const expectedKeepAddress = await ecdsaKeepFactory.keepAddress.call()
 
-      const result = await tbtcSystem.requestNewKeep.call(5, 10, 0)
+      const result = await tbtcSystem.requestNewKeep.call(5, 10, 0, { value: openKeepFee })
 
       assert.equal(expectedKeepAddress, result, 'incorrect keep address')
     })
+
+    it('forwards value to keep factory', async () => {
+      const initialBalance = await web3.eth.getBalance(ecdsaKeepFactory.address)
+
+      await tbtcSystem.requestNewKeep(5, 10, 0, { value: openKeepFee })
+
+      const finalBalance = await web3.eth.getBalance(ecdsaKeepFactory.address)
+      const balanceCheck = new BN(finalBalance).sub(new BN(initialBalance))
+      expect(balanceCheck, 'TBTCSystem did not correctly forward value to keep factory').to.eq.BN(openKeepFee)
+    })
   })
 
   describe('setSignerFeeDivisor', async () => {

implementation/test/contracts/keep/ECDSAKeepFactoryStub.sol

@@ -5,14 +5,25 @@ import {IBondedECDSAKeepFactory} from "@keep-network/keep-ecdsa/contracts/api/IB
 contract ECDSAKeepFactoryStub is IBondedECDSAKeepFactory {
      address public keepOwner;
      address public keepAddress = address(888);
+     uint256 feeEstimate = 123456;
 
     function openKeep(
         uint256,
         uint256,
         address _owner,
         uint256
     ) external payable returns (address) {
+        require(msg.value >= feeEstimate, "Insufficient value for new keep creation");
         keepOwner = _owner;
         return keepAddress;
     }
+
+    function openKeepFeeEstimate() external returns (uint256){
+        return feeEstimate;
+    }
+
+    function setOpenKeepFeeEstimate(uint256 _fee) external {
+        feeEstimate = _fee;
+    }
+
 }

implementation/test/contracts/keep/ECDSAKeepStub.sol

@@ -55,7 +55,6 @@ contract ECDSAKeepStub is IBondedECDSAKeep {
     // Functions implemented for IBondedECDSAKeep interface.
 
     function submitSignatureFraud(
-        address,
         uint8,
         bytes32,
         bytes32,
@@ -65,11 +64,11 @@ contract ECDSAKeepStub is IBondedECDSAKeep {
        return success;
     }
 
-    function checkBondAmount(address) external view returns (uint256){
+    function checkBondAmount() external view returns (uint256){
         return bondAmount;
     }
 
-    function seizeSignerBonds(address) external returns (bool){
+    function seizeSignerBonds() external returns (bool){
         if (address(this).balance > 0) {
             msg.sender.transfer(address(this).balance);
         }