Skip to content

Commit

Permalink
fix: make fisherman deposit a fixed value (OZ L-01)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Maikol
Maikol committed Aug 24, 2024
1 parent b2bf070 commit b56567e
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 119 deletions.
75 changes: 38 additions & 37 deletions packages/subgraph-service/contracts/DisputeManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ contract DisputeManager is
* @notice Initialize this contract.
* @param arbitrator Arbitrator role
* @param disputePeriod Dispute period in seconds
* @param minimumDeposit Minimum deposit required to create a Dispute
* @param disputeDeposit Deposit required to create a Dispute
* @param fishermanRewardCut_ Percent of slashed funds for fisherman (ppm)
* @param maxSlashingCut_ Maximum percentage of indexer stake that can be slashed (ppm)
*/
function initialize(
address arbitrator,
uint64 disputePeriod,
uint256 minimumDeposit,
uint256 disputeDeposit,
uint32 fishermanRewardCut_,
uint32 maxSlashingCut_
) external override initializer {
Expand All @@ -113,7 +113,7 @@ contract DisputeManager is

_setArbitrator(arbitrator);
_setDisputePeriod(disputePeriod);
_setMinimumDeposit(minimumDeposit);
_setDisputeDeposit(disputeDeposit);
_setFishermanRewardCut(fishermanRewardCut_);
_setMaxSlashingCut(maxSlashingCut_);
}
Expand All @@ -122,40 +122,45 @@ contract DisputeManager is
* @notice Create an indexing dispute for the arbitrator to resolve.
* The disputes are created in reference to an allocationId and specifically
* a POI for that allocation.
* This function is called by a challenger that will need to `_deposit` at
* least `minimumDeposit` GRT tokens.
* This function is called by a challenger and it will pull `disputeDeposit` GRT tokens.
*
* Requirements:
* - Challenger must have previously approved this contract to pull `disputeDeposit` amount
* of tokens from their balance.
*
* @param allocationId The allocation to dispute
* @param poi The Proof of Indexing (POI) being disputed
* @param deposit Amount of tokens staked as deposit
*/
function createIndexingDispute(
address allocationId,
bytes32 poi,
uint256 deposit
bytes32 poi
) external override returns (bytes32) {
// Get funds from submitter
_pullSubmitterDeposit(deposit);
_pullSubmitterDeposit();

// Create a dispute
return _createIndexingDisputeWithAllocation(msg.sender, deposit, allocationId, poi);
return _createIndexingDisputeWithAllocation(msg.sender, disputeDeposit, allocationId, poi);
}

/**
* @notice Create a query dispute for the arbitrator to resolve.
* This function is called by a fisherman that will need to `_deposit` at
* least `minimumDeposit` GRT tokens.
* @param attestationData Attestation bytes submitted by the fisherman
* @param deposit Amount of tokens staked as deposit
*/
function createQueryDispute(bytes calldata attestationData, uint256 deposit) external override returns (bytes32) {
* This function is called by a challenger and it will pull `disputeDeposit` GRT tokens.
*
* * Requirements:
* - Challenger must have previously approved this contract to pull `disputeDeposit` amount
* of tokens from their balance.
*
* @param attestationData Attestation bytes submitted by the challenger
*/
function createQueryDispute(bytes calldata attestationData) external override returns (bytes32) {
// Get funds from submitter
_pullSubmitterDeposit(deposit);
_pullSubmitterDeposit();

// Create a dispute
return
_createQueryDisputeWithAttestation(
msg.sender,
deposit,
disputeDeposit,
Attestation.parse(attestationData),
attestationData
);
Expand Down Expand Up @@ -303,12 +308,12 @@ contract DisputeManager is
}

/**
* @notice Set the minimum deposit required to create a dispute.
* @dev Update the minimum deposit to `_minimumDeposit` Graph Tokens
* @param minimumDeposit The minimum deposit in Graph Tokens
* @notice Set the dispute deposit required to create a dispute.
* @dev Update the dispute deposit to `_disputeDeposit` Graph Tokens
* @param disputeDeposit The dispute deposit in Graph Tokens
*/
function setMinimumDeposit(uint256 minimumDeposit) external override onlyOwner {
_setMinimumDeposit(minimumDeposit);
function setDisputeDeposit(uint256 disputeDeposit) external override onlyOwner {
_setDisputeDeposit(disputeDeposit);
}

/**
Expand Down Expand Up @@ -577,15 +582,11 @@ contract DisputeManager is
}

/**
* @notice Pull deposit from submitter account.
* @param _deposit Amount of tokens to deposit
* @notice Pull `disputeDeposit` from submitter account.
*/
function _pullSubmitterDeposit(uint256 _deposit) private {
// Ensure that fisherman has staked at least the minimum amount
require(_deposit >= minimumDeposit, DisputeManagerInsufficientDeposit(_deposit, minimumDeposit));

function _pullSubmitterDeposit() private {
// Transfer tokens to deposit from fisherman to this contract
_graphToken().pullTokens(msg.sender, _deposit);
_graphToken().pullTokens(msg.sender, disputeDeposit);
}

/**
Expand Down Expand Up @@ -642,14 +643,14 @@ contract DisputeManager is
}

/**
* @notice Internal: Set the minimum deposit required to create a dispute.
* @dev Update the minimum deposit to `_minimumDeposit` Graph Tokens
* @param _minimumDeposit The minimum deposit in Graph Tokens
* @notice Internal: Set the dispute deposit required to create a dispute.
* @dev Update the dispute deposit to `_disputeDeposit` Graph Tokens
* @param _disputeDeposit The dispute deposit in Graph Tokens
*/
function _setMinimumDeposit(uint256 _minimumDeposit) private {
require(_minimumDeposit != 0, DisputeManagerInvalidMinimumDeposit(_minimumDeposit));
minimumDeposit = _minimumDeposit;
emit MinimumDepositSet(minimumDeposit);
function _setDisputeDeposit(uint256 _disputeDeposit) private {
require(_disputeDeposit != 0, DisputeManagerInvalidDisputeDeposit(_disputeDeposit));
disputeDeposit = _disputeDeposit;
emit DisputeDepositSet(disputeDeposit);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/subgraph-service/contracts/DisputeManagerStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ abstract contract DisputeManagerV1Storage {
/// @notice dispute period in seconds
uint64 public disputePeriod;

/// @notice Minimum deposit required to create a Dispute
uint256 public minimumDeposit;
/// @notice Deposit required to create a Dispute
uint256 public disputeDeposit;

/// @notice Percentage of indexer slashed funds to assign as a reward to fisherman in successful dispute. In PPM.
uint32 public fishermanRewardCut;
Expand Down
17 changes: 8 additions & 9 deletions packages/subgraph-service/contracts/interfaces/IDisputeManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ interface IDisputeManager {
event DisputePeriodSet(uint64 disputePeriod);

/**
* @notice Emitted when minimum deposit is set.
* @param minimumDeposit The minimum deposit required to create a dispute.
* @notice Emitted when dispute deposit is set.
* @param disputeDeposit The dispute deposit required to create a dispute.
*/
event MinimumDepositSet(uint256 minimumDeposit);
event DisputeDepositSet(uint256 disputeDeposit);

/**
* @notice Emitted when max slashing cut is set.
Expand Down Expand Up @@ -151,12 +151,11 @@ interface IDisputeManager {
error DisputeManagerDisputePeriodZero();
error DisputeManagerZeroTokens();
error DisputeManagerInvalidDispute(bytes32 disputeId);
error DisputeManagerInvalidMinimumDeposit(uint256 minimumDeposit);
error DisputeManagerInvalidDisputeDeposit(uint256 disputeDeposit);
error DisputeManagerInvalidFishermanReward(uint32 cut);
error DisputeManagerInvalidMaxSlashingCut(uint32 maxSlashingCut);
error DisputeManagerInvalidTokensSlash(uint256 tokensSlash, uint256 maxTokensSlash);
error DisputeManagerDisputeNotPending(IDisputeManager.DisputeStatus status);
error DisputeManagerInsufficientDeposit(uint256 deposit, uint256 minimumDeposit);
error DisputeManagerDisputeAlreadyCreated(bytes32 disputeId);
error DisputeManagerDisputePeriodNotFinished();
error DisputeManagerMustAcceptRelatedDispute(bytes32 disputeId, bytes32 relatedDisputeId);
Expand All @@ -174,7 +173,7 @@ interface IDisputeManager {
function initialize(
address arbitrator,
uint64 disputePeriod,
uint256 minimumDeposit,
uint256 disputeDeposit,
uint32 fishermanRewardCut,
uint32 maxSlashingCut
) external;
Expand All @@ -183,22 +182,22 @@ interface IDisputeManager {

function setArbitrator(address arbitrator) external;

function setMinimumDeposit(uint256 minimumDeposit) external;
function setDisputeDeposit(uint256 disputeDeposit) external;

function setFishermanRewardCut(uint32 cut) external;

function setMaxSlashingCut(uint32 maxCut) external;

// -- Dispute --

function createQueryDispute(bytes calldata attestationData, uint256 deposit) external returns (bytes32);
function createQueryDispute(bytes calldata attestationData) external returns (bytes32);

function createQueryDisputeConflict(
bytes calldata attestationData1,
bytes calldata attestationData2
) external returns (bytes32, bytes32);

function createIndexingDispute(address allocationId, bytes32 poi, uint256 deposit) external returns (bytes32);
function createIndexingDispute(address allocationId, bytes32 poi) external returns (bytes32);

function acceptDispute(bytes32 disputeId, uint256 tokensSlash) external;

Expand Down
2 changes: 1 addition & 1 deletion packages/subgraph-service/test/SubgraphBaseTest.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ abstract contract SubgraphBaseTest is Utils, Constants {
users.governor,
abi.encodeCall(
DisputeManager.initialize,
(users.arbitrator, disputePeriod, minimumDeposit, fishermanRewardPercentage, maxSlashingPercentage)
(users.arbitrator, disputePeriod, disputeDeposit, fishermanRewardPercentage, maxSlashingPercentage)
)
);
disputeManager = DisputeManager(disputeManagerProxy);
Expand Down
18 changes: 12 additions & 6 deletions packages/subgraph-service/test/disputes/DisputeManager.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,20 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
* HELPERS
*/

function _createIndexingDispute(address _allocationID, bytes32 _poi, uint256 tokens) internal returns (bytes32 disputeID) {
function _createIndexingDispute(address _allocationID, bytes32 _poi) internal returns (bytes32 disputeID) {
address msgSender;
(, msgSender,) = vm.readCallers();
resetPrank(users.fisherman);
token.approve(address(disputeManager), tokens);
bytes32 _disputeID = disputeManager.createIndexingDispute(_allocationID, _poi, tokens);
uint256 beforeFishermanBalance = token.balanceOf(users.fisherman);
token.approve(address(disputeManager), disputeDeposit);
bytes32 _disputeID = disputeManager.createIndexingDispute(_allocationID, _poi);
uint256 afterFishermanBalance = token.balanceOf(users.fisherman);
assertEq(afterFishermanBalance, beforeFishermanBalance - disputeDeposit, "Fisherman should be charged the dispute deposit");
resetPrank(msgSender);
return _disputeID;
}

function _createQueryDispute(uint256 tokens) internal returns (bytes32 disputeID) {
function _createQueryDispute() internal returns (bytes32 disputeID) {
address msgSender;
(, msgSender,) = vm.readCallers();
resetPrank(users.fisherman);
Expand All @@ -49,8 +52,11 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
});
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);

token.approve(address(disputeManager), tokens);
bytes32 _disputeID = disputeManager.createQueryDispute(attestationData, tokens);
uint256 beforeFishermanBalance = token.balanceOf(users.fisherman);
token.approve(address(disputeManager), disputeDeposit);
bytes32 _disputeID = disputeManager.createQueryDispute(attestationData);
uint256 afterFishermanBalance = token.balanceOf(users.fisherman);
assertEq(afterFishermanBalance, beforeFishermanBalance - disputeDeposit, "Fisherman should be charged the dispute deposit");
resetPrank(msgSender);
return _disputeID;
}
Expand Down
22 changes: 5 additions & 17 deletions packages/subgraph-service/test/disputes/accept.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {

function testAccept_IndexingDispute(
uint256 tokens,
uint256 tokensDispute,
uint256 tokensSlash,
uint256 delegationAmount
) public useIndexer useAllocation(tokens) {
Expand All @@ -29,10 +28,9 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {
uint256 stakeSnapshot = disputeManager.getStakeSnapshot(users.indexer);
uint256 tokensSlashCap = stakeSnapshot.mulPPM(maxSlashingPercentage);
tokensSlash = bound(tokensSlash, 1, tokensSlashCap);
tokensDispute = bound(tokensDispute, minimumDeposit, tokens);

uint256 fishermanPreviousBalance = token.balanceOf(users.fisherman);
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), tokensDispute);
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));

resetPrank(users.arbitrator);
disputeManager.acceptDispute(disputeID, tokensSlash);
Expand All @@ -44,7 +42,6 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {

function testAccept_QueryDispute(
uint256 tokens,
uint256 tokensDispute,
uint256 tokensSlash,
uint256 delegationAmount
) public useIndexer useAllocation(tokens) {
Expand All @@ -56,10 +53,9 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {
uint256 stakeSnapshot = disputeManager.getStakeSnapshot(users.indexer);
uint256 tokensSlashCap = stakeSnapshot.mulPPM(maxSlashingPercentage);
tokensSlash = bound(tokensSlash, 1, tokensSlashCap);
tokensDispute = bound(tokensDispute, minimumDeposit, tokens);

uint256 fishermanPreviousBalance = token.balanceOf(users.fisherman);
bytes32 disputeID = _createQueryDispute(tokensDispute);
bytes32 disputeID = _createQueryDispute();

resetPrank(users.arbitrator);
disputeManager.acceptDispute(disputeID, tokensSlash);
Expand Down Expand Up @@ -120,7 +116,6 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {

function testAccept_IndexingDispute_RevertIf_SlashAmountTooHigh(
uint256 tokens,
uint256 tokensDispute,
uint256 tokensSlash,
uint256 delegationAmount
) public useIndexer useAllocation(tokens) {
Expand All @@ -132,9 +127,8 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {
uint256 stakeSnapshot = disputeManager.getStakeSnapshot(users.indexer);
uint256 tokensSlashCap = stakeSnapshot.mulPPM(maxSlashingPercentage);
tokensSlash = bound(tokensSlash, tokensSlashCap + 1 wei, type(uint256).max);
tokensDispute = bound(tokensDispute, minimumDeposit, tokens);

bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), tokensDispute);
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));

resetPrank(users.arbitrator);
bytes memory expectedError = abi.encodeWithSelector(
Expand All @@ -148,7 +142,6 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {

function testAccept_QueryDispute_RevertIf_SlashAmountTooHigh(
uint256 tokens,
uint256 tokensDispute,
uint256 tokensSlash,
uint256 delegationAmount
) public useIndexer useAllocation(tokens) {
Expand All @@ -160,9 +153,8 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {
uint256 stakeSnapshot = disputeManager.getStakeSnapshot(users.indexer);
uint256 tokensSlashCap = stakeSnapshot.mulPPM(maxSlashingPercentage);
tokensSlash = bound(tokensSlash, tokensSlashCap + 1 wei, type(uint256).max);
tokensDispute = bound(tokensDispute, minimumDeposit, tokens);

bytes32 disputeID = _createQueryDispute(tokensDispute);
bytes32 disputeID = _createQueryDispute();

resetPrank(users.arbitrator);
bytes memory expectedError = abi.encodeWithSelector(
Expand All @@ -176,7 +168,6 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {

function testAccept_ConflictingQueryDispute_RevertIf_SlashAmountTooHigh(
uint256 tokens,
uint256 tokensDispute,
uint256 tokensSlash,
uint256 delegationAmount
) public useIndexer useAllocation(tokens) {
Expand All @@ -188,7 +179,6 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {
uint256 stakeSnapshot = disputeManager.getStakeSnapshot(users.indexer);
uint256 tokensSlashCap = stakeSnapshot.mulPPM(maxSlashingPercentage);
tokensSlash = bound(tokensSlash, tokensSlashCap + 1 wei, type(uint256).max);
tokensDispute = bound(tokensDispute, minimumDeposit, tokens);

bytes32 responseCID1 = keccak256(abi.encodePacked("Response CID 1"));
bytes32 responseCID2 = keccak256(abi.encodePacked("Response CID 2"));
Expand Down Expand Up @@ -219,13 +209,11 @@ contract DisputeManagerAcceptDisputeTest is DisputeManagerTest {

function testAccept_RevertIf_CallerIsNotArbitrator(
uint256 tokens,
uint256 tokensDispute,
uint256 tokensSlash
) public useIndexer useAllocation(tokens) {
tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
tokensDispute = bound(tokensDispute, minimumDeposit, tokens);

bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"), tokensDispute);
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));

// attempt to accept dispute as fisherman
resetPrank(users.fisherman);
Expand Down
Loading

0 comments on commit b56567e

Please sign in to comment.