Reward earliest fraud proof only (#65)

* 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

* feat(bond): claim from Publisher instead of state root

This forces a 1:1 connection between a publisher's bond and a state root being claimed

* feat(bond): set the earliest state root
Co-authored-by: ben-chain <ben@pseudonym.party>

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

@@ -17,6 +17,9 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
      * Constants and Parameters *
      ****************************/
 
+    /// The period to find the earliest fraud proof for a publisher
+    uint256 public constant multiFraudProofPeriod = 7 days;
+
     /// The dispute period
     uint256 public constant disputePeriodSeconds = 7 days;
 
@@ -82,6 +85,19 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
         witnessProviders[_preStateRoot].canClaim = true;
 
         Bond storage bond = bonds[publisher];
+        if (bond.firstDisputeAt == 0) {
+            bond.firstDisputeAt = block.timestamp;
+            bond.earliestDisputedStateRoot = _preStateRoot;
+            bond.earliestTimestamp = timestamp;
+        } else if (
+            // only update the disputed state root for the publisher if it's within
+            // the dispute period _and_ if it's before the previous one
+            block.timestamp < bond.firstDisputeAt + multiFraudProofPeriod &&
+            timestamp < bond.earliestTimestamp
+        ) {
+            bond.earliestDisputedStateRoot = _preStateRoot;
+            bond.earliestTimestamp = timestamp;
+        }
 
         // if the fraud proof's dispute period does not intersect with the 
         // withdrawal's timestamp, then the user should not be slashed
@@ -143,8 +159,17 @@ contract OVM_BondManager is iOVM_BondManager, Lib_AddressResolver {
         );
     }
 
-    /// Claims the user's reward for the witnesses they provided
-    function claim(bytes32 _preStateRoot) override public {
+    /// Claims the user's reward for the witnesses they provided for the earliest
+    /// disputed state root of the designated publisher
+    function claim(address who) override public {
+        Bond storage bond = bonds[who];
+        require(
+            block.timestamp >= bond.firstDisputeAt + multiFraudProofPeriod,
+            Errors.WAIT_FOR_DISPUTES
+        );
+
+        // reward the earliest state root for this publisher
+        bytes32 _preStateRoot = bond.earliestDisputedStateRoot;
         Rewards storage rewards = witnessProviders[_preStateRoot];
 
         // only allow claiming if fraud was proven in `finalize`

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

@@ -20,6 +20,7 @@ library Errors {
     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";
+    string constant WAIT_FOR_DISPUTES = "BondManager: Wait for other potential disputes";
 }
 
 /**
@@ -47,6 +48,12 @@ interface iOVM_BondManager {
         State state;
         // The timestamp at which a proposer issued their withdrawal request
         uint32 withdrawalTimestamp;
+        // The time when the first disputed was initiated for this bond
+        uint256 firstDisputeAt;
+        // The earliest observed state root for this bond which has had fraud
+        bytes32 earliestDisputedStateRoot;
+        // The state root's timestamp
+        uint256 earliestTimestamp;
     }
 
     // Per pre-state root, store the number of state provisions that were made
@@ -86,7 +93,7 @@ interface iOVM_BondManager {
     function finalizeWithdrawal() external;
 
     function claim(
-        bytes32 _preStateRoot
+        address who
     ) external;
 
     function isCollateralized(

packages/contracts/test/bond.ts

@@ -16,6 +16,7 @@ describe('BondManager', () => {
   let manager: Contract
   let fraudVerifier: Contract
 
+  const publisher = wallets[0].address
   const stateTransitioner = wallets[3]
   const witnessProvider = wallets[4]
   const witnessProvider2 = wallets[5]
@@ -172,36 +173,54 @@ describe('BondManager', () => {
     })
 
     it('cannot claim before canClaim is set', async () => {
-      await expect(bondManager.claim(preStateRoot)).to.be.revertedWith(
+      await expect(bondManager.claim(publisher)).to.be.revertedWith(
         Errors.CANNOT_CLAIM
       )
     })
 
     describe('claims', () => {
+      let timestamp: number
+      // prepare by setting the claim flag and linking the publisher to the state root
       beforeEach(async () => {
         // deposit the collateral to be distributed
         await token.approve(bondManager.address, ethers.constants.MaxUint256)
         await bondManager.deposit()
 
         // smodify the canClaim value to true to test claiming
+        const block = await provider.getBlock('latest')
+        timestamp = block.timestamp
         bondManager.smodify.set({
           witnessProviders: {
             [preStateRoot]: {
               canClaim: true,
             },
           },
+          bonds: {
+            [publisher]: {
+              earliestDisputedStateRoot: preStateRoot,
+              firstDisputeAt: timestamp,
+            },
+          },
         })
         const reward = await bondManager.witnessProviders(preStateRoot)
         expect(reward.canClaim).to.be.true
       })
 
+      it('cannot claim before time for other disputes has passed', async () => {
+        await expect(
+          bondManager.connect(witnessProvider).claim(publisher)
+        ).to.be.revertedWith(Errors.WAIT_FOR_DISPUTES)
+      })
+
       it('rewards get paid out proportionally', async () => {
+        await mineBlock(deployer.provider, timestamp + ONE_WEEK)
+
         // One will get 2/3rds of the bond, the other will get 1/3rd
         const balanceBefore1 = await token.balanceOf(witnessProvider.address)
         const balanceBefore2 = await token.balanceOf(witnessProvider2.address)
 
-        await bondManager.connect(witnessProvider).claim(preStateRoot)
-        await bondManager.connect(witnessProvider2).claim(preStateRoot)
+        await bondManager.connect(witnessProvider).claim(publisher)
+        await bondManager.connect(witnessProvider2).claim(publisher)
 
         const balanceAfter1 = await token.balanceOf(witnessProvider.address)
         const balanceAfter2 = await token.balanceOf(witnessProvider2.address)
@@ -215,15 +234,16 @@ describe('BondManager', () => {
       })
 
       it('cannot double claim', async () => {
+        await mineBlock(deployer.provider, timestamp + ONE_WEEK)
         const balance1 = await token.balanceOf(witnessProvider.address)
-        await bondManager.connect(witnessProvider).claim(preStateRoot)
+        await bondManager.connect(witnessProvider).claim(publisher)
         const balance2 = await token.balanceOf(witnessProvider.address)
         expect(balance2).to.be.eq(
           balance1.add(half.mul(totalUser1Gas).div(totalGas))
         )
 
         // re-claiming does not give the user any extra funds
-        await bondManager.connect(witnessProvider).claim(preStateRoot)
+        await bondManager.connect(witnessProvider).claim(publisher)
         const balance3 = await token.balanceOf(witnessProvider.address)
         expect(balance3).to.be.eq(balance2)
       })
@@ -296,6 +316,54 @@ describe('BondManager', () => {
           Errors.WRONG_STATE
         )
       })
+
+      describe('same publisher commits fraud multiple times', async () => {
+        let timestamp: number
+        let root1 =
+          '0x0000000000000000000000000000000000000000000000000000000000000000'
+        let ts1 = 100
+        let root2 =
+          '0x0000000000000000000000000000000000000000000000000000000000000001'
+        let ts2 = 110
+
+        beforeEach(async () => {
+          await fraudVerifier.finalize(root2, sender, ts2)
+          const block = await provider.getBlock('latest')
+          timestamp = block.timestamp
+        })
+
+        it('initial dispute data is stored', async () => {
+          const bond = await bondManager.bonds(sender)
+          expect(bond.firstDisputeAt).to.be.equal(timestamp)
+          expect(bond.earliestTimestamp).to.be.equal(ts2)
+          expect(bond.earliestDisputedStateRoot).to.be.equal(root2)
+        })
+
+        it('earlier dispute replaces initial data', async () => {
+          await fraudVerifier.finalize(root1, sender, ts1)
+          const bond = await bondManager.bonds(sender)
+          expect(bond.firstDisputeAt).to.be.equal(timestamp)
+          expect(bond.earliestTimestamp).to.be.equal(ts1)
+          expect(bond.earliestDisputedStateRoot).to.be.equal(root1)
+        })
+
+        it('earlier dispute does not replace initial data if not in time', async () => {
+          await mineBlock(deployer.provider, timestamp + ONE_WEEK)
+          await fraudVerifier.finalize(root1, sender, ts1)
+          const bond = await bondManager.bonds(sender)
+          expect(bond.firstDisputeAt).to.be.equal(timestamp)
+          expect(bond.earliestTimestamp).to.be.equal(ts2)
+          expect(bond.earliestDisputedStateRoot).to.be.equal(root2)
+        })
+
+        it('later dispute does not replace initial data', async () => {
+          await fraudVerifier.finalize(root1, sender, ts2 + 1)
+          const bond = await bondManager.bonds(sender)
+          expect(bond.firstDisputeAt).to.be.equal(timestamp)
+          expect(bond.earliestTimestamp).to.be.equal(ts2)
+          expect(bond.earliestDisputedStateRoot).to.be.equal(root2)
+        })
+      })
     })
   })
 })
@@ -323,4 +391,5 @@ enum Errors {
   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',
+  WAIT_FOR_DISPUTES = 'BondManager: Wait for other potential disputes',
 }