Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRST audit fixes for DisputeManager contract #1075

Merged
merged 6 commits into from
Dec 17, 2024
Merged

TRST audit fixes for DisputeManager contract #1075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4d75947
fix: dispute manager refactor (TRST-M07 / TRST-M08)
tmigone Nov 29, 2024
0a31b58
fix: ensure subgraph service is set before using it (TRST-R10)
tmigone Dec 9, 2024
85de5bd
fix: documentation error (TRST-R09)
tmigone Dec 9, 2024
25d22e4
fix: added new function for accept dispute in conflict that includes …
Maikol Dec 13, 2024
4ba215c
fix: accept dispute documentation
Maikol Dec 16, 2024
8aa05c4
Merge branch 'horizon' into tmigone/trust-fixes-dispute-manager
tmigone Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 134 additions & 114 deletions packages/subgraph-service/contracts/DisputeManager.sol
View file
Open in desktop

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions packages/subgraph-service/contracts/interfaces/IDisputeManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ interface IDisputeManager {
error DisputeManagerDisputeNotPending(IDisputeManager.DisputeStatus status);
error DisputeManagerDisputeAlreadyCreated(bytes32 disputeId);
error DisputeManagerDisputePeriodNotFinished();
error DisputeManagerDisputeInConflict(bytes32 disputeId);
error DisputeManagerDisputeNotInConflict(bytes32 disputeId);
error DisputeManagerMustAcceptRelatedDispute(bytes32 disputeId, bytes32 relatedDisputeId);
error DisputeManagerIndexerNotFound(address allocationId);
error DisputeManagerNonMatchingSubgraphDeployment(bytes32 subgraphDeploymentId1, bytes32 subgraphDeploymentId2);
Expand All @@ -180,6 +182,7 @@ interface IDisputeManager {
bytes32 responseCID2,
bytes32 subgraphDeploymentId2
);
error DisputeManagerSubgraphServiceNotSet();

function setDisputePeriod(uint64 disputePeriod) external;

Expand All @@ -204,6 +207,13 @@ interface IDisputeManager {

function acceptDispute(bytes32 disputeId, uint256 tokensSlash) external;

function acceptDisputeConflict(
bytes32 disputeId,
uint256 tokensSlash,
bool acceptDisputeInConflict,
uint256 tokensSlashRelated
) external;

function rejectDispute(bytes32 disputeId) external;

function drawDispute(bytes32 disputeId) external;
Expand Down
396 changes: 333 additions & 63 deletions packages/subgraph-service/test/disputeManager/DisputeManager.t.sol
View file
Open in desktop

Large diffs are not rendered by default.

30 changes: 30 additions & 0 deletions packages/subgraph-service/test/disputeManager/disputes/indexing/accept.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,36 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
_acceptDispute(disputeID, tokensSlash);
}

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

resetPrank(users.fisherman);
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));

resetPrank(users.arbitrator);
// clear subgraph service address from storage
_setStorage_SubgraphService(address(0));

vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
disputeManager.acceptDispute(disputeID, tokensSlash);
}

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

resetPrank(users.fisherman);
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));

resetPrank(users.arbitrator);
_acceptDispute(disputeID, tokensSlash);
}

function test_Indexing_Accept_RevertIf_CallerIsNotArbitrator(
uint256 tokens,
uint256 tokensSlash
Expand Down
35 changes: 24 additions & 11 deletions packages/subgraph-service/test/disputeManager/disputes/indexing/create.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,28 @@ import { IDisputeManager } from "../../../../contracts/interfaces/IDisputeManage
import { DisputeManagerTest } from "../../DisputeManager.t.sol";

contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {

/*
* TESTS
*/

function test_Indexing_Create_Dispute(
function test_Indexing_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) {
resetPrank(users.fisherman);
_createIndexingDispute(allocationID, bytes32("POI1"));
}

function test_Indexing_Create_Dispute_RevertWhen_SubgraphServiceNotSet(
uint256 tokens
) public useIndexer useAllocation(tokens) {
resetPrank(users.fisherman);
_createIndexingDispute(allocationID, bytes32("POI1"));

// clear subgraph service address from storage
_setStorage_SubgraphService(address(0));

// // Approve the dispute deposit
token.approve(address(disputeManager), disputeDeposit);

vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
disputeManager.createIndexingDispute(allocationID, bytes32("POI2"));
}

function test_Indexing_Create_MultipleDisputes() public {
Expand All @@ -33,7 +45,12 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
_createProvision(indexer, tokens, maxSlashingPercentage, disputePeriod);
_register(indexer, abi.encode("url", "geoHash", address(0)));
uint256 allocationIDPrivateKey = uint256(keccak256(abi.encodePacked(i)));
bytes memory data = _createSubgraphAllocationData(indexer, subgraphDeployment, allocationIDPrivateKey, tokens);
bytes memory data = _createSubgraphAllocationData(
indexer,
subgraphDeployment,
allocationIDPrivateKey,
tokens
);
_startService(indexer, data);
allocationIDPrivateKeys[i] = allocationIDPrivateKey;
}
Expand All @@ -48,7 +65,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
uint256 tokens
) public useIndexer useAllocation(tokens) {
resetPrank(users.fisherman);
bytes32 disputeID =_createIndexingDispute(allocationID, bytes32("POI1"));
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));

// Create another dispute with different fisherman
address otherFisherman = makeAddr("otherFisherman");
Expand Down Expand Up @@ -78,9 +95,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
vm.stopPrank();
}

function test_Indexing_Create_RevertIf_AllocationDoesNotExist(
uint256 tokens
) public useFisherman {
function test_Indexing_Create_RevertIf_AllocationDoesNotExist(uint256 tokens) public useFisherman {
tokens = bound(tokens, disputeDeposit, 10_000_000_000 ether);
token.approve(address(disputeManager), tokens);
bytes memory expectedError = abi.encodeWithSelector(
Expand All @@ -92,9 +107,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
vm.stopPrank();
}

function test_Indexing_Create_RevertIf_IndexerIsBelowStake(
uint256 tokens
) public useIndexer useAllocation(tokens) {
function test_Indexing_Create_RevertIf_IndexerIsBelowStake(uint256 tokens) public useIndexer useAllocation(tokens) {
// Close allocation
bytes memory data = abi.encode(allocationID);
_stopService(users.indexer, data);
Expand Down
52 changes: 52 additions & 0 deletions packages/subgraph-service/test/disputeManager/disputes/query/accept.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,39 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest {
_acceptDispute(disputeID, tokensSlash);
}

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

resetPrank(users.fisherman);
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
bytes32 disputeID = _createQueryDispute(attestationData);

resetPrank(users.arbitrator);
// clear subgraph service address from storage
_setStorage_SubgraphService(address(0));
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
disputeManager.acceptDispute(disputeID, tokensSlash);
}

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

resetPrank(users.fisherman);
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
bytes32 disputeID = _createQueryDispute(attestationData);

resetPrank(users.arbitrator);
_acceptDispute(disputeID, tokensSlash);
}

function test_Query_Accept_RevertIf_CallerIsNotArbitrator(
uint256 tokens,
uint256 tokensSlash
Expand Down Expand Up @@ -72,4 +105,23 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest {
vm.expectRevert(expectedError);
disputeManager.acceptDispute(disputeID, tokensSlash);
}

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

resetPrank(users.fisherman);
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
bytes32 disputeID = _createQueryDispute(attestationData);

resetPrank(users.arbitrator);
vm.expectRevert(abi.encodeWithSelector(
IDisputeManager.DisputeManagerDisputeNotInConflict.selector,
disputeID
));
disputeManager.acceptDisputeConflict(disputeID, tokensSlash, true, 0);
}
}
17 changes: 16 additions & 1 deletion packages/subgraph-service/test/disputeManager/disputes/query/create.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,28 @@ contract DisputeManagerQueryCreateDisputeTest is DisputeManagerTest {
* TESTS
*/

function test_Query_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) {
function test_Query_Create_Dispute_Only(uint256 tokens) public useIndexer useAllocation(tokens) {
resetPrank(users.fisherman);
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
_createQueryDispute(attestationData);
}

function test_Query_Create_Dispute_RevertWhen_SubgraphServiceNotSet(uint256 tokens) public useIndexer useAllocation(tokens) {
resetPrank(users.fisherman);
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);

// clear subgraph service address from storage
_setStorage_SubgraphService(address(0));

// // Approve the dispute deposit
token.approve(address(disputeManager), disputeDeposit);

vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
disputeManager.createQueryDispute(attestationData);
}

function test_Query_Create_MultipleDisputes_DifferentFisherman(
uint256 tokens
) public useIndexer useAllocation(tokens) {
Expand Down
Loading
Loading