From 12c436d4ffa21a04456d92b11efa4fd165e344e9 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 1 Dec 2020 22:32:02 +0200 Subject: [PATCH] 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 --- .../OVM/verification/OVM_BondManager.sol | 29 ++++++- .../iOVM/verification/iOVM_BondManager.sol | 9 ++- packages/contracts/test/bond.ts | 79 +++++++++++++++++-- 3 files changed, 109 insertions(+), 8 deletions(-) diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/verification/OVM_BondManager.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/verification/OVM_BondManager.sol index 2965e167bfaf..548fd317b695 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/verification/OVM_BondManager.sol +++ b/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` diff --git a/packages/contracts/contracts/optimistic-ethereum/iOVM/verification/iOVM_BondManager.sol b/packages/contracts/contracts/optimistic-ethereum/iOVM/verification/iOVM_BondManager.sol index d3e5e17e1aa5..8cfbd759b48c 100644 --- a/packages/contracts/contracts/optimistic-ethereum/iOVM/verification/iOVM_BondManager.sol +++ b/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( diff --git a/packages/contracts/test/bond.ts b/packages/contracts/test/bond.ts index 43a39c5434be..97f418f36c36 100644 --- a/packages/contracts/test/bond.ts +++ b/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', }