Bond Manager Deposit bugfix (#64)

* fix(bond): always deposit requiredCollateral

This means that we have to remove the setRequiredCollateral function to avoid:
1. Publisher deposits, requiredCollateral goes up, publisher withdraws more than they deposited
2. Publisher deposits, fraud happens, requiredCollateral goes up, more than the user's deposit gets
distributed in fraud claims

* fix(bond): remove redundant batchIndex parameter

packages/contracts/contracts/optimistic-ethereum/OVM/verification/OVM_BondManager.sol

@@ -21,13 +21,7 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
     uint256 public constant disputePeriodSeconds = 7 days;
 
     /// The minimum collateral a sequencer must post
-    uint256 public requiredCollateral = 1 ether;
-
-    /// The maximum multiplier for updating the `requiredCollateral`
-    uint256 public constant MAX = 5;
-
-    /// Owner used to bump the security bond size
-    address immutable public owner;
+    uint256 public constant requiredCollateral = 1 ether;
 
 
     /*******************************************
@@ -37,9 +31,6 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
     /// The bond token
     ERC20 immutable public token;
 
-    /// The fraud verifier contract, used to get data about transitioners for a pre-state root
-    address public ovmFraudVerifier;
-
 
     /********************************************
      * Contract Variables: Internal Accounting  *
@@ -62,7 +53,6 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
     constructor(ERC20 _token, address _libAddressManager)
         Lib_AddressResolver(_libAddressManager)
     {
-        owner = msg.sender;
         token = _token;
     }
 
@@ -83,7 +73,7 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
 
     /// Slashes + distributes rewards or frees up the sequencer's bond, only called by
     /// `FraudVerifier.finalizeFraudVerification`
-    function finalize(bytes32 _preStateRoot, uint256 batchIndex, address publisher, uint256 timestamp) override public {
+    function finalize(bytes32 _preStateRoot, address publisher, uint256 timestamp) override public {
         require(msg.sender == resolve("OVM_FraudVerifier"), Errors.ONLY_FRAUD_VERIFIER);
         require(witnessProviders[_preStateRoot].canClaim == false, Errors.ALREADY_FINALIZED);
 
@@ -113,9 +103,9 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
 
     /// Sequencers call this function to post collateral which will be used for
     /// the `appendBatch` call
-    function deposit(uint256 amount) override public {
+    function deposit() override public {
         require(
-            token.transferFrom(msg.sender, address(this), amount),
+            token.transferFrom(msg.sender, address(this), requiredCollateral),
             Errors.ERC20_ERR
         );
 
@@ -171,16 +161,7 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
         require(token.transfer(msg.sender, amount), Errors.ERC20_ERR);
     }
 
-    /// Sets the required collateral for posting a state root
-    /// Callable only by the contract's deployer.
-    function setRequiredCollateral(uint256 newValue) override public {
-        require(newValue > requiredCollateral, Errors.LOW_VALUE);
-        require(newValue < MAX * requiredCollateral, Errors.HIGH_VALUE);
-        require(msg.sender == owner, Errors.ONLY_OWNER);
-        requiredCollateral = newValue;
-    }
-
-    /// Checks if the user is collateralized for the batchIndex
+    /// Checks if the user is collateralized
     function isCollateralized(address who) override public view returns (bool) {
         return bonds[who].state == State.COLLATERALIZED;
     }

packages/contracts/contracts/optimistic-ethereum/OVM/verification/OVM_FraudVerifier.sol

@@ -203,7 +203,6 @@ contract OVM_FraudVerifier is Lib_AddressResolver, OVM_FraudContributor, iOVM_Fr
         // slash the bonds at the bond manager
         ovmBondManager.finalize(
             _preStateRoot,
-            _postStateRootBatchHeader.batchIndex,
             publisher,
             timestamp
         );

packages/contracts/contracts/optimistic-ethereum/iOVM/verification/iOVM_BondManager.sol

@@ -9,8 +9,6 @@ interface ERC20 {
 /// All the errors which may be encountered on the bond manager
 library Errors {
     string constant ERC20_ERR = "BondManager: Could not post bond";
-    string constant LOW_VALUE = "BondManager: New collateral value must be greater than the previous one";
-    string constant HIGH_VALUE = "BondManager: New collateral value cannot be more than 5x of the previous one";
     string constant ALREADY_FINALIZED = "BondManager: Fraud proof for this pre-state root has already been finalized";
     string constant SLASHED = "BondManager: Cannot finalize withdrawal, you probably got slashed";
     string constant WRONG_STATE = "BondManager: Wrong bond state for proposer";
@@ -19,7 +17,6 @@ library Errors {
     string constant WITHDRAWAL_PENDING = "BondManager: Withdrawal already pending";
     string constant TOO_EARLY = "BondManager: Too early to finalize your withdrawal";
 
-    string constant ONLY_OWNER = "BondManager: Only the contract's owner can call this function";
     string constant ONLY_TRANSITIONER = "BondManager: Only the transitioner for this pre-state root may call this function";
     string constant ONLY_FRAUD_VERIFIER = "BondManager: Only the fraud verifier may call this function";
     string constant ONLY_STATE_COMMITMENT_CHAIN = "BondManager: Only the state commitment chain may call this function";
@@ -78,14 +75,11 @@ interface iOVM_BondManager {
 
     function finalize(
         bytes32 _preStateRoot,
-        uint256 _batchIndex,
         address _publisher,
         uint256 _timestamp
     ) external;
 
-    function deposit(
-        uint256 _amount
-    ) external;
+    function deposit() external;
 
     function startWithdrawal() external;
 
@@ -95,10 +89,6 @@ interface iOVM_BondManager {
         bytes32 _preStateRoot
     ) external;
 
-    function setRequiredCollateral(
-        uint256 _newValue
-    ) external;
-
     function isCollateralized(
         address _who
     ) external view returns (bool);

packages/contracts/contracts/test-helpers/Mock_FraudVerifier.sol

@@ -20,7 +20,7 @@ contract Mock_FraudVerifier {
         return transitioners[preStateRoot];
     }
 
-    function finalize(bytes32 _preStateRoot, uint256 batchIndex, address publisher, uint256 timestamp) public {
-        bondManager.finalize(_preStateRoot, batchIndex, publisher, timestamp);
+    function finalize(bytes32 _preStateRoot, address publisher, uint256 timestamp) public {
+        bondManager.finalize(_preStateRoot, publisher, timestamp);
     }
 }

packages/contracts/test/bond.ts

@@ -71,39 +71,13 @@ describe('BondManager', () => {
     await fraudVerifier.setBondManager(bondManager.address)
   })
 
-  describe('required collateral adjustment', () => {
-    it('bumps required collateral', async () => {
-      // sets collateral to 2 eth, which is more than what we have deposited
-      await bondManager.setRequiredCollateral(ethers.utils.parseEther('2'))
-      expect(await bondManager.isCollateralized(sender)).to.be.false
-    })
-
-    it('cannot lower collateral reqs', async () => {
-      await expect(
-        bondManager.setRequiredCollateral(ethers.utils.parseEther('0.99'))
-      ).to.be.revertedWith(Errors.LOW_VALUE)
-    })
-
-    it('cannot increase collateral reqs to more than 5x of current value', async () => {
-      await expect(
-        bondManager.setRequiredCollateral(ethers.utils.parseEther('10.1'))
-      ).to.be.revertedWith(Errors.HIGH_VALUE)
-    })
-
-    it('only owner can adjust collateral', async () => {
-      await expect(
-        bondManager.connect(wallets[2]).setRequiredCollateral(amount.add(1))
-      ).to.be.revertedWith(Errors.ONLY_OWNER)
-    })
-  })
-
   describe('collateral management', () => {
     let balanceBefore: BigNumber
 
     beforeEach(async () => {
       await token.approve(bondManager.address, ethers.constants.MaxUint256)
       balanceBefore = await token.balanceOf(sender)
-      await bondManager.deposit(amount)
+      await bondManager.deposit()
     })
 
     it('can deposit', async () => {
@@ -207,7 +181,7 @@ describe('BondManager', () => {
       beforeEach(async () => {
         // deposit the collateral to be distributed
         await token.approve(bondManager.address, ethers.constants.MaxUint256)
-        await bondManager.deposit(amount)
+        await bondManager.deposit()
 
         // smodify the canClaim value to true to test claiming
         bondManager.smodify.set({
@@ -256,28 +230,26 @@ describe('BondManager', () => {
     })
 
     describe('finalize', () => {
-      const batchIdx = 1
-
       beforeEach(async () => {
         await token.approve(bondManager.address, ethers.constants.MaxUint256)
-        await bondManager.deposit(amount)
+        await bondManager.deposit()
       })
 
       it('only fraud verifier can finalize', async () => {
         await expect(
-          bondManager.finalize(preStateRoot, batchIdx, sender, 0)
+          bondManager.finalize(preStateRoot, sender, 0)
         ).to.be.revertedWith(Errors.ONLY_FRAUD_VERIFIER)
       })
 
       it('proving fraud allows claiming', async () => {
-        await fraudVerifier.finalize(preStateRoot, batchIdx, sender, 0)
+        await fraudVerifier.finalize(preStateRoot, sender, 0)
 
         expect((await bondManager.witnessProviders(preStateRoot)).canClaim).to
           .be.true
 
         // cannot double finalize
         await expect(
-          fraudVerifier.finalize(preStateRoot, batchIdx, sender, 0)
+          fraudVerifier.finalize(preStateRoot, sender, 0)
         ).to.be.revertedWith(Errors.ALREADY_FINALIZED)
       })
 
@@ -290,7 +262,6 @@ describe('BondManager', () => {
         const disputeTimestamp = withdrawalTimestamp - 100
         await fraudVerifier.finalize(
           preStateRoot,
-          batchIdx,
           sender,
           disputeTimestamp
         )
@@ -310,7 +281,6 @@ describe('BondManager', () => {
         const disputeTimestamp = withdrawalTimestamp - ONE_WEEK - 1
         await fraudVerifier.finalize(
           preStateRoot,
-          batchIdx,
           sender,
           disputeTimestamp
         )
@@ -321,7 +291,7 @@ describe('BondManager', () => {
       })
 
       it('proving fraud prevents starting a withdrawal due to slashing', async () => {
-        await fraudVerifier.finalize(preStateRoot, batchIdx, sender, 0)
+        await fraudVerifier.finalize(preStateRoot, sender, 0)
         await expect(bondManager.startWithdrawal()).to.be.revertedWith(
           Errors.WRONG_STATE
         )
@@ -342,8 +312,6 @@ enum State {
 // Errors from the bond manager smart contract
 enum Errors {
   ERC20_ERR = 'BondManager: Could not post bond',
-  LOW_VALUE = 'BondManager: New collateral value must be greater than the previous one',
-  HIGH_VALUE = 'BondManager: New collateral value cannot be more than 5x of the previous one',
   ALREADY_FINALIZED = 'BondManager: Fraud proof for this pre-state root has already been finalized',
   SLASHED = 'BondManager: Cannot finalize withdrawal, you probably got slashed',
   WRONG_STATE = 'BondManager: Wrong bond state for proposer',
@@ -352,7 +320,6 @@ enum Errors {
   WITHDRAWAL_PENDING = 'BondManager: Withdrawal already pending',
   TOO_EARLY = 'BondManager: Too early to finalize your withdrawal',
 
-  ONLY_OWNER = "BondManager: Only the contract's owner can call this function",
   ONLY_TRANSITIONER = 'BondManager: Only the transitioner for this pre-state root may call this function',
   ONLY_FRAUD_VERIFIER = 'BondManager: Only the fraud verifier may call this function',
   ONLY_STATE_COMMITMENT_CHAIN = 'BondManager: Only the state commitment chain may call this function',