Skip to content

[audit-09] fix: [TRST-L-6] Proper agreement version check #1206

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

Open
wants to merge 1 commit into
base: ma/indexing-payments-audit-fixes-08-L-5
Choose a base branch
from
+33 −3
Open

[audit-09] fix: [TRST-L-6] Proper agreement version check #1206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions packages/subgraph-service/contracts/libraries/IndexingAgreement.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,7 @@ library IndexingAgreement {
signedRCAU.rcau.metadata
);

wrapper.agreement.version = metadata.version;

require(wrapper.agreement.version == IndexingAgreementVersion.V1, "internal: invalid version");
require(metadata.version == IndexingAgreementVersion.V1, IndexingAgreementInvalidVersion(metadata.version));
_setTermsV1(self, signedRCAU.rcau.agreementId, metadata.terms);

Expand Down
33 changes: 32 additions & 1 deletion packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,38 @@ contract SubgraphServiceIndexingAgreementAcceptTest is SubgraphServiceIndexingAg
subgraphService.acceptIndexingAgreement(ctx.indexers[0].allocationId, accepted);
}

function test_SubgraphService_AcceptIndexingAgreement_Revert_WhenAgreementAlreadyAllocated() public {}
function test_SubgraphService_AcceptIndexingAgreement_Revert_WhenAgreementAlreadyAllocated(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems unrelated to the fix, right? In which case it would be good to include a test for when an invalid version is supplied

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way the code is structured, I don't think we can test for that right now. i.e. only V1 is declared in the enum.

Seed memory seed,
uint256 alternativeNonce
) public {
Context storage ctx = _newCtx(seed);
IndexerState memory indexerState = _withIndexer(ctx);

// First, accept an indexing agreement on the allocation
(IRecurringCollector.SignedRCA memory accepted, ) = _withAcceptedIndexingAgreement(ctx, indexerState);
vm.assume(accepted.rca.nonce != alternativeNonce);

// Now try to accept a different agreement on the same allocation
// Create a new agreement with different nonce to ensure different agreement ID
IRecurringCollector.RecurringCollectionAgreement
memory newRCA = _generateAcceptableRecurringCollectionAgreement(ctx, indexerState.addr);
newRCA.nonce = alternativeNonce; // Different nonce to ensure different agreement ID

// Sign the new agreement
IRecurringCollector.SignedRCA memory newSignedRCA = _recurringCollectorHelper.generateSignedRCA(
newRCA,
ctx.payer.signerPrivateKey
);

// Expect the error when trying to accept a second agreement on the same allocation
bytes memory expectedErr = abi.encodeWithSelector(
IndexingAgreement.AllocationAlreadyHasIndexingAgreement.selector,
indexerState.allocationId
);
vm.expectRevert(expectedErr);
resetPrank(indexerState.addr);
subgraphService.acceptIndexingAgreement(indexerState.allocationId, newSignedRCA);
}

function test_SubgraphService_AcceptIndexingAgreement_Revert_WhenInvalidTermsData(Seed memory seed) public {
Context storage ctx = _newCtx(seed);
Expand Down