diff --git a/packages/horizon/contracts/interfaces/IRecurringCollector.sol b/packages/horizon/contracts/interfaces/IRecurringCollector.sol index 19388062b..704515aa7 100644 --- a/packages/horizon/contracts/interfaces/IRecurringCollector.sol +++ b/packages/horizon/contracts/interfaces/IRecurringCollector.sol @@ -40,7 +40,6 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector { /** * @notice The Recurring Collection Agreement (RCA) - * @param agreementId The agreement ID of the RCA * @param deadline The deadline for accepting the RCA * @param endsAt The timestamp when the agreement ends * @param payer The address of the payer the RCA was issued by @@ -52,11 +51,11 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector { * except for the first collection * @param minSecondsPerCollection The minimum amount of seconds that must pass between collections * @param maxSecondsPerCollection The maximum amount of seconds that can pass between collections + * @param nonce A unique nonce for preventing collisions (user-chosen) * @param metadata Arbitrary metadata to extend functionality if a data service requires it * */ struct RecurringCollectionAgreement { - bytes16 agreementId; uint64 deadline; uint64 endsAt; address payer; @@ -66,6 +65,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector { uint256 maxOngoingTokensPerSecond; uint32 minSecondsPerCollection; uint32 maxSecondsPerCollection; + uint256 nonce; bytes metadata; } @@ -372,8 +372,9 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector { /** * @dev Accept an indexing agreement. * @param signedRCA The signed Recurring Collection Agreement which is to be accepted. + * @return agreementId The deterministically generated agreement ID */ - function accept(SignedRCA calldata signedRCA) external; + function accept(SignedRCA calldata signedRCA) external returns (bytes16 agreementId); /** * @dev Cancel an indexing agreement. @@ -433,4 +434,21 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector { function getCollectionInfo( AgreementData memory agreement ) external view returns (bool isCollectable, uint256 collectionSeconds); + + /** + * @notice Generate a deterministic agreement ID from agreement parameters + * @param payer The address of the payer + * @param dataService The address of the data service + * @param serviceProvider The address of the service provider + * @param deadline The deadline for accepting the agreement + * @param nonce A unique nonce for preventing collisions + * @return agreementId The deterministically generated agreement ID + */ + function generateAgreementId( + address payer, + address dataService, + address serviceProvider, + uint64 deadline, + uint256 nonce + ) external pure returns (bytes16 agreementId); } diff --git a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol index e16db74db..56d2b9d5b 100644 --- a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol +++ b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol @@ -29,7 +29,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC /// @notice The EIP712 typehash for the RecurringCollectionAgreement struct bytes32 public constant EIP712_RCA_TYPEHASH = keccak256( - "RecurringCollectionAgreement(bytes16 agreementId,uint256 deadline,uint256 endsAt,address payer,address dataService,address serviceProvider,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,bytes metadata)" + "RecurringCollectionAgreement(uint64 deadline,uint64 endsAt,address payer,address dataService,address serviceProvider,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,uint256 nonce,bytes metadata)" ); /// @notice The EIP712 typehash for the RecurringCollectionAgreementUpdate struct @@ -75,8 +75,16 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC * See {IRecurringCollector.accept}. * @dev Caller must be the data service the RCA was issued to. */ - function accept(SignedRCA calldata signedRCA) external { - require(signedRCA.rca.agreementId != bytes16(0), RecurringCollectorAgreementIdZero()); + function accept(SignedRCA calldata signedRCA) external returns (bytes16) { + bytes16 agreementId = _generateAgreementId( + signedRCA.rca.payer, + signedRCA.rca.dataService, + signedRCA.rca.serviceProvider, + signedRCA.rca.deadline, + signedRCA.rca.nonce + ); + + require(agreementId != bytes16(0), RecurringCollectorAgreementIdZero()); require( msg.sender == signedRCA.rca.dataService, RecurringCollectorUnauthorizedCaller(msg.sender, signedRCA.rca.dataService) @@ -102,11 +110,11 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC signedRCA.rca.maxSecondsPerCollection ); - AgreementData storage agreement = _getAgreementStorage(signedRCA.rca.agreementId); + AgreementData storage agreement = _getAgreementStorage(agreementId); // check that the agreement is not already accepted require( agreement.state == AgreementState.NotAccepted, - RecurringCollectorAgreementIncorrectState(signedRCA.rca.agreementId, agreement.state) + RecurringCollectorAgreementIncorrectState(agreementId, agreement.state) ); // accept the agreement @@ -126,7 +134,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC agreement.dataService, agreement.payer, agreement.serviceProvider, - signedRCA.rca.agreementId, + agreementId, agreement.acceptedAt, agreement.endsAt, agreement.maxInitialTokens, @@ -134,6 +142,8 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC agreement.minSecondsPerCollection, agreement.maxSecondsPerCollection ); + + return agreementId; } /** @@ -261,6 +271,17 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC return _getCollectionInfo(agreement); } + /// @inheritdoc IRecurringCollector + function generateAgreementId( + address payer, + address dataService, + address serviceProvider, + uint64 deadline, + uint256 nonce + ) external pure returns (bytes16) { + return _generateAgreementId(payer, dataService, serviceProvider, deadline, nonce); + } + /** * @notice Decodes the collect data. * @param data The encoded collect parameters. @@ -457,7 +478,6 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC keccak256( abi.encode( EIP712_RCA_TYPEHASH, - _rca.agreementId, _rca.deadline, _rca.endsAt, _rca.payer, @@ -467,6 +487,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC _rca.maxOngoingTokensPerSecond, _rca.minSecondsPerCollection, _rca.maxSecondsPerCollection, + _rca.nonce, keccak256(_rca.metadata) ) ) @@ -590,4 +611,23 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC function _agreementCollectionStartAt(AgreementData memory _agreement) private pure returns (uint256) { return _agreement.lastCollectionAt > 0 ? _agreement.lastCollectionAt : _agreement.acceptedAt; } + + /** + * @notice Internal function to generate deterministic agreement ID + * @param _payer The address of the payer + * @param _dataService The address of the data service + * @param _serviceProvider The address of the service provider + * @param _deadline The deadline for accepting the agreement + * @param _nonce A unique nonce for preventing collisions + * @return agreementId The deterministically generated agreement ID + */ + function _generateAgreementId( + address _payer, + address _dataService, + address _serviceProvider, + uint64 _deadline, + uint256 _nonce + ) private pure returns (bytes16) { + return bytes16(keccak256(abi.encode(_payer, _dataService, _serviceProvider, _deadline, _nonce))); + } } diff --git a/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorHelper.t.sol b/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorHelper.t.sol index 611f554e7..6ddbdfa0b 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorHelper.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/RecurringCollectorHelper.t.sol @@ -45,17 +45,47 @@ contract RecurringCollectorHelper is AuthorizableHelper, Bounder { return signedRCAU; } - function generateSignedRCAUWithCorrectNonce( + function generateSignedRCAUForAgreement( + bytes16 agreementId, IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau, uint256 signerPrivateKey ) public view returns (IRecurringCollector.SignedRCAU memory) { // Automatically set the correct nonce based on current agreement state - IRecurringCollector.AgreementData memory agreement = collector.getAgreement(rcau.agreementId); + IRecurringCollector.AgreementData memory agreement = collector.getAgreement(agreementId); rcau.nonce = agreement.updateNonce + 1; return generateSignedRCAU(rcau, signerPrivateKey); } + function generateSignedRCAUWithCorrectNonce( + IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau, + uint256 signerPrivateKey + ) public view returns (IRecurringCollector.SignedRCAU memory) { + // This is kept for backwards compatibility but should not be used with new interface + // since we can't determine agreementId without it being passed separately + return generateSignedRCAU(rcau, signerPrivateKey); + } + + function generateSignedRCAWithCalculatedId( + IRecurringCollector.RecurringCollectionAgreement memory rca, + uint256 signerPrivateKey + ) public view returns (IRecurringCollector.SignedRCA memory, bytes16) { + // Ensure we have sensible values + rca = sensibleRCA(rca); + + // Calculate the agreement ID + bytes16 agreementId = collector.generateAgreementId( + rca.payer, + rca.dataService, + rca.serviceProvider, + rca.deadline, + rca.nonce + ); + + IRecurringCollector.SignedRCA memory signedRCA = generateSignedRCA(rca, signerPrivateKey); + return (signedRCA, agreementId); + } + function withElapsedAcceptDeadline( IRecurringCollector.RecurringCollectionAgreement memory rca ) public view returns (IRecurringCollector.RecurringCollectionAgreement memory) { @@ -76,11 +106,15 @@ contract RecurringCollectorHelper is AuthorizableHelper, Bounder { function sensibleRCA( IRecurringCollector.RecurringCollectionAgreement memory rca ) public view returns (IRecurringCollector.RecurringCollectionAgreement memory) { - vm.assume(rca.agreementId != bytes16(0)); vm.assume(rca.dataService != address(0)); vm.assume(rca.payer != address(0)); vm.assume(rca.serviceProvider != address(0)); + // Ensure we have a nonce if it's zero + if (rca.nonce == 0) { + rca.nonce = 1; + } + rca.minSecondsPerCollection = _sensibleMinSecondsPerCollection(rca.minSecondsPerCollection); rca.maxSecondsPerCollection = _sensibleMaxSecondsPerCollection( rca.maxSecondsPerCollection, diff --git a/packages/horizon/test/unit/payments/recurring-collector/accept.t.sol b/packages/horizon/test/unit/payments/recurring-collector/accept.t.sol index d9479b955..f7a4c3823 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/accept.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/accept.t.sol @@ -20,7 +20,15 @@ contract RecurringCollectorAcceptTest is RecurringCollectorSharedTest { IRecurringCollector.SignedRCA memory fuzzySignedRCA, uint256 unboundedSkip ) public { - vm.assume(fuzzySignedRCA.rca.agreementId != bytes16(0)); + // Generate deterministic agreement ID for validation + bytes16 agreementId = _recurringCollector.generateAgreementId( + fuzzySignedRCA.rca.payer, + fuzzySignedRCA.rca.dataService, + fuzzySignedRCA.rca.serviceProvider, + fuzzySignedRCA.rca.deadline, + fuzzySignedRCA.rca.nonce + ); + vm.assume(agreementId != bytes16(0)); skip(boundSkip(unboundedSkip, 1, type(uint64).max - block.timestamp)); fuzzySignedRCA.rca = _recurringCollectorHelper.withElapsedAcceptDeadline(fuzzySignedRCA.rca); @@ -35,11 +43,13 @@ contract RecurringCollectorAcceptTest is RecurringCollectorSharedTest { } function test_Accept_Revert_WhenAlreadyAccepted(FuzzyTestAccept calldata fuzzyTestAccept) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzyTestAccept); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzyTestAccept + ); bytes memory expectedErr = abi.encodeWithSelector( IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector, - accepted.rca.agreementId, + agreementId, IRecurringCollector.AgreementState.Accepted ); vm.expectRevert(expectedErr); diff --git a/packages/horizon/test/unit/payments/recurring-collector/cancel.t.sol b/packages/horizon/test/unit/payments/recurring-collector/cancel.t.sol index fe938c825..fa3b595a0 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/cancel.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/cancel.t.sol @@ -13,22 +13,34 @@ contract RecurringCollectorCancelTest is RecurringCollectorSharedTest { /* solhint-disable graph/func-name-mixedcase */ function test_Cancel(FuzzyTestAccept calldata fuzzyTestAccept, uint8 unboundedCanceler) public { - _sensibleAuthorizeAndAccept(fuzzyTestAccept); - _cancel(fuzzyTestAccept.rca, _fuzzyCancelAgreementBy(unboundedCanceler)); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzyTestAccept + ); + + _cancel(accepted.rca, agreementId, _fuzzyCancelAgreementBy(unboundedCanceler)); } function test_Cancel_Revert_WhenNotAccepted( IRecurringCollector.RecurringCollectionAgreement memory fuzzyRCA, uint8 unboundedCanceler ) public { + // Generate deterministic agreement ID + bytes16 agreementId = _recurringCollector.generateAgreementId( + fuzzyRCA.payer, + fuzzyRCA.dataService, + fuzzyRCA.serviceProvider, + fuzzyRCA.deadline, + fuzzyRCA.nonce + ); + bytes memory expectedErr = abi.encodeWithSelector( IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector, - fuzzyRCA.agreementId, + agreementId, IRecurringCollector.AgreementState.NotAccepted ); vm.expectRevert(expectedErr); vm.prank(fuzzyRCA.dataService); - _recurringCollector.cancel(fuzzyRCA.agreementId, _fuzzyCancelAgreementBy(unboundedCanceler)); + _recurringCollector.cancel(agreementId, _fuzzyCancelAgreementBy(unboundedCanceler)); } function test_Cancel_Revert_WhenNotDataService( @@ -38,16 +50,16 @@ contract RecurringCollectorCancelTest is RecurringCollectorSharedTest { ) public { vm.assume(fuzzyTestAccept.rca.dataService != notDataService); - _sensibleAuthorizeAndAccept(fuzzyTestAccept); + (, , bytes16 agreementId) = _sensibleAuthorizeAndAccept(fuzzyTestAccept); bytes memory expectedErr = abi.encodeWithSelector( IRecurringCollector.RecurringCollectorDataServiceNotAuthorized.selector, - fuzzyTestAccept.rca.agreementId, + agreementId, notDataService ); vm.expectRevert(expectedErr); vm.prank(notDataService); - _recurringCollector.cancel(fuzzyTestAccept.rca.agreementId, _fuzzyCancelAgreementBy(unboundedCanceler)); + _recurringCollector.cancel(agreementId, _fuzzyCancelAgreementBy(unboundedCanceler)); } /* solhint-enable graph/func-name-mixedcase */ } diff --git a/packages/horizon/test/unit/payments/recurring-collector/collect.t.sol b/packages/horizon/test/unit/payments/recurring-collector/collect.t.sol index c99098c7b..a972734a6 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/collect.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/collect.t.sol @@ -29,12 +29,11 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { ) public { vm.assume(fuzzy.fuzzyTestAccept.rca.dataService != notDataService); - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); + (, , bytes16 agreementId) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); IRecurringCollector.CollectParams memory collectParams = fuzzy.collectParams; skip(1); - - collectParams.agreementId = accepted.rca.agreementId; + collectParams.agreementId = agreementId; bytes memory data = _generateCollectData(collectParams); bytes memory expectedErr = abi.encodeWithSelector( @@ -48,10 +47,11 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { } function test_Collect_Revert_WhenUnauthorizedDataService(FuzzyTestCollect calldata fuzzy) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzy.fuzzyTestAccept + ); IRecurringCollector.CollectParams memory collectParams = fuzzy.collectParams; - - collectParams.agreementId = accepted.rca.agreementId; + collectParams.agreementId = agreementId; collectParams.tokens = bound(collectParams.tokens, 1, type(uint256).max); bytes memory data = _generateCollectData(collectParams); @@ -99,12 +99,15 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { } function test_Collect_Revert_WhenCanceledAgreementByServiceProvider(FuzzyTestCollect calldata fuzzy) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); - _cancel(accepted.rca, IRecurringCollector.CancelAgreementBy.ServiceProvider); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzy.fuzzyTestAccept + ); + _cancel(accepted.rca, agreementId, IRecurringCollector.CancelAgreementBy.ServiceProvider); IRecurringCollector.CollectParams memory collectData = fuzzy.collectParams; collectData.tokens = bound(collectData.tokens, 1, type(uint256).max); IRecurringCollector.CollectParams memory collectParams = _generateCollectParams( accepted.rca, + agreementId, collectData.collectionId, collectData.tokens, collectData.dataServiceCut @@ -125,12 +128,15 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { FuzzyTestCollect calldata fuzzy, uint256 unboundedCollectionSeconds ) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzy.fuzzyTestAccept + ); skip(accepted.rca.minSecondsPerCollection); bytes memory data = _generateCollectData( _generateCollectParams( accepted.rca, + agreementId, fuzzy.collectParams.collectionId, 1, fuzzy.collectParams.dataServiceCut @@ -144,6 +150,7 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { IRecurringCollector.CollectParams memory collectParams = _generateCollectParams( accepted.rca, + agreementId, fuzzy.collectParams.collectionId, bound(fuzzy.collectParams.tokens, 1, type(uint256).max), fuzzy.collectParams.dataServiceCut @@ -165,9 +172,12 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { uint256 unboundedFirstCollectionSeconds, uint256 unboundedSecondCollectionSeconds ) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzy.fuzzyTestAccept + ); // skip to collectable time + skip( boundSkip( unboundedFirstCollectionSeconds, @@ -178,6 +188,7 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { bytes memory data = _generateCollectData( _generateCollectParams( accepted.rca, + agreementId, fuzzy.collectParams.collectionId, 1, fuzzy.collectParams.dataServiceCut @@ -197,6 +208,7 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { data = _generateCollectData( _generateCollectParams( accepted.rca, + agreementId, fuzzy.collectParams.collectionId, bound(fuzzy.collectParams.tokens, 1, type(uint256).max), fuzzy.collectParams.dataServiceCut @@ -204,7 +216,7 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { ); bytes memory expectedErr = abi.encodeWithSelector( IRecurringCollector.RecurringCollectorCollectionTooLate.selector, - accepted.rca.agreementId, + agreementId, collectionSeconds, accepted.rca.maxSecondsPerCollection ); @@ -220,7 +232,9 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { uint256 unboundedTokens, bool testInitialCollection ) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzy.fuzzyTestAccept + ); if (!testInitialCollection) { // skip to collectable time @@ -234,6 +248,7 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { bytes memory initialData = _generateCollectData( _generateCollectParams( accepted.rca, + agreementId, fuzzy.collectParams.collectionId, 1, fuzzy.collectParams.dataServiceCut @@ -255,6 +270,7 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { uint256 tokens = bound(unboundedTokens, maxTokens + 1, type(uint256).max); IRecurringCollector.CollectParams memory collectParams = _generateCollectParams( accepted.rca, + agreementId, fuzzy.collectParams.collectionId, tokens, fuzzy.collectParams.dataServiceCut @@ -270,7 +286,9 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { uint256 unboundedCollectionSeconds, uint256 unboundedTokens ) public { - (IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept); + (IRecurringCollector.SignedRCA memory accepted, , bytes16 agreementId) = _sensibleAuthorizeAndAccept( + fuzzy.fuzzyTestAccept + ); (bytes memory data, uint256 collectionSeconds, uint256 tokens) = _generateValidCollection( accepted.rca, @@ -278,8 +296,15 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest { unboundedCollectionSeconds, unboundedTokens ); + skip(collectionSeconds); - _expectCollectCallAndEmit(accepted.rca, _paymentType(fuzzy.unboundedPaymentType), fuzzy.collectParams, tokens); + _expectCollectCallAndEmit( + accepted.rca, + agreementId, + _paymentType(fuzzy.unboundedPaymentType), + fuzzy.collectParams, + tokens + ); vm.prank(accepted.rca.dataService); uint256 collected = _recurringCollector.collect(_paymentType(fuzzy.unboundedPaymentType), data); assertEq(collected, tokens); diff --git a/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol b/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol index d8d9483e7..9a564086e 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/shared.t.sol @@ -54,37 +54,47 @@ contract RecurringCollectorSharedTest is Test, Bounder { function _sensibleAuthorizeAndAccept( FuzzyTestAccept calldata _fuzzyTestAccept - ) internal returns (IRecurringCollector.SignedRCA memory, uint256 key) { + ) internal returns (IRecurringCollector.SignedRCA memory, uint256 key, bytes16 agreementId) { IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA( _fuzzyTestAccept.rca ); key = boundKey(_fuzzyTestAccept.unboundedSignerKey); - return (_authorizeAndAccept(rca, key), key); + IRecurringCollector.SignedRCA memory signedRCA; + (signedRCA, agreementId) = _authorizeAndAccept(rca, key); + return (signedRCA, key, agreementId); } // authorizes signer, signs the RCA, and accepts it function _authorizeAndAccept( IRecurringCollector.RecurringCollectionAgreement memory _rca, uint256 _signerKey - ) internal returns (IRecurringCollector.SignedRCA memory) { + ) internal returns (IRecurringCollector.SignedRCA memory, bytes16 agreementId) { _recurringCollectorHelper.authorizeSignerWithChecks(_rca.payer, _signerKey); IRecurringCollector.SignedRCA memory signedRCA = _recurringCollectorHelper.generateSignedRCA(_rca, _signerKey); - _accept(signedRCA); - - return signedRCA; + agreementId = _accept(signedRCA); + return (signedRCA, agreementId); } - function _accept(IRecurringCollector.SignedRCA memory _signedRCA) internal { + function _accept(IRecurringCollector.SignedRCA memory _signedRCA) internal returns (bytes16) { // Set up valid staking provision by default to allow collections to succeed _setupValidProvision(_signedRCA.rca.serviceProvider, _signedRCA.rca.dataService); + // Calculate the expected agreement ID for verification + bytes16 expectedAgreementId = _recurringCollector.generateAgreementId( + _signedRCA.rca.payer, + _signedRCA.rca.dataService, + _signedRCA.rca.serviceProvider, + _signedRCA.rca.deadline, + _signedRCA.rca.nonce + ); + vm.expectEmit(address(_recurringCollector)); emit IRecurringCollector.AgreementAccepted( _signedRCA.rca.dataService, _signedRCA.rca.payer, _signedRCA.rca.serviceProvider, - _signedRCA.rca.agreementId, + expectedAgreementId, uint64(block.timestamp), _signedRCA.rca.endsAt, _signedRCA.rca.maxInitialTokens, @@ -93,7 +103,11 @@ contract RecurringCollectorSharedTest is Test, Bounder { _signedRCA.rca.maxSecondsPerCollection ); vm.prank(_signedRCA.rca.dataService); - _recurringCollector.accept(_signedRCA); + bytes16 actualAgreementId = _recurringCollector.accept(_signedRCA); + + // Verify the agreement ID matches expectation + assertEq(actualAgreementId, expectedAgreementId); + return actualAgreementId; } function _setupValidProvision(address _serviceProvider, address _dataService) internal { @@ -117,6 +131,7 @@ contract RecurringCollectorSharedTest is Test, Bounder { function _cancel( IRecurringCollector.RecurringCollectionAgreement memory _rca, + bytes16 _agreementId, IRecurringCollector.CancelAgreementBy _by ) internal { vm.expectEmit(address(_recurringCollector)); @@ -124,16 +139,17 @@ contract RecurringCollectorSharedTest is Test, Bounder { _rca.dataService, _rca.payer, _rca.serviceProvider, - _rca.agreementId, + _agreementId, uint64(block.timestamp), _by ); vm.prank(_rca.dataService); - _recurringCollector.cancel(_rca.agreementId, _by); + _recurringCollector.cancel(_agreementId, _by); } function _expectCollectCallAndEmit( IRecurringCollector.RecurringCollectionAgreement memory _rca, + bytes16 _agreementId, IGraphPayments.PaymentTypes __paymentType, IRecurringCollector.CollectParams memory _fuzzyParams, uint256 _tokens @@ -168,7 +184,7 @@ contract RecurringCollectorSharedTest is Test, Bounder { _rca.dataService, _rca.payer, _rca.serviceProvider, - _rca.agreementId, + _agreementId, _fuzzyParams.collectionId, _tokens, _fuzzyParams.dataServiceCut @@ -187,8 +203,18 @@ contract RecurringCollectorSharedTest is Test, Bounder { _rca.maxSecondsPerCollection ); uint256 tokens = bound(_unboundedTokens, 1, _rca.maxOngoingTokensPerSecond * collectionSeconds); + + // Generate the agreement ID deterministically + bytes16 agreementId = _recurringCollector.generateAgreementId( + _rca.payer, + _rca.dataService, + _rca.serviceProvider, + _rca.deadline, + _rca.nonce + ); + bytes memory data = _generateCollectData( - _generateCollectParams(_rca, _fuzzyParams.collectionId, tokens, _fuzzyParams.dataServiceCut) + _generateCollectParams(_rca, agreementId, _fuzzyParams.collectionId, tokens, _fuzzyParams.dataServiceCut) ); return (data, collectionSeconds, tokens); @@ -196,13 +222,14 @@ contract RecurringCollectorSharedTest is Test, Bounder { function _generateCollectParams( IRecurringCollector.RecurringCollectionAgreement memory _rca, + bytes16 _agreementId, bytes32 _collectionId, uint256 _tokens, uint256 _dataServiceCut ) internal pure returns (IRecurringCollector.CollectParams memory) { return IRecurringCollector.CollectParams({ - agreementId: _rca.agreementId, + agreementId: _agreementId, collectionId: _collectionId, tokens: _tokens, dataServiceCut: _dataServiceCut, diff --git a/packages/horizon/test/unit/payments/recurring-collector/update.t.sol b/packages/horizon/test/unit/payments/recurring-collector/update.t.sol index 1676fc0bc..ea34f895b 100644 --- a/packages/horizon/test/unit/payments/recurring-collector/update.t.sol +++ b/packages/horizon/test/unit/payments/recurring-collector/update.t.sol @@ -19,7 +19,15 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { ) public { rca = _recurringCollectorHelper.sensibleRCA(rca); rcau = _recurringCollectorHelper.sensibleRCAU(rcau); - rcau.agreementId = rca.agreementId; + // Generate deterministic agreement ID + bytes16 agreementId = _recurringCollector.generateAgreementId( + rca.payer, + rca.dataService, + rca.serviceProvider, + rca.deadline, + rca.nonce + ); + rcau.agreementId = agreementId; boundSkipCeil(unboundedUpdateSkip, type(uint64).max); rcau.deadline = uint64(bound(rcau.deadline, 0, block.timestamp - 1)); @@ -44,7 +52,15 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { ) public { rca = _recurringCollectorHelper.sensibleRCA(rca); rcau = _recurringCollectorHelper.sensibleRCAU(rcau); - rcau.agreementId = rca.agreementId; + // Generate deterministic agreement ID + bytes16 agreementId = _recurringCollector.generateAgreementId( + rca.payer, + rca.dataService, + rca.serviceProvider, + rca.deadline, + rca.nonce + ); + rcau.agreementId = agreementId; rcau.deadline = uint64(block.timestamp); IRecurringCollector.SignedRCAU memory signedRCAU = IRecurringCollector.SignedRCAU({ @@ -67,14 +83,12 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { address notDataService ) public { vm.assume(fuzzyTestUpdate.fuzzyTestAccept.rca.dataService != notDataService); - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + (, uint256 signerKey, bytes16 agreementId) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau.agreementId = accepted.rca.agreementId; + rcau.agreementId = agreementId; IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAUWithCorrectNonce( rcau, @@ -95,16 +109,18 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { FuzzyTestUpdate calldata fuzzyTestUpdate, uint256 unboundedInvalidSignerKey ) public { - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + ( + IRecurringCollector.SignedRCA memory accepted, + uint256 signerKey, + bytes16 agreementId + ) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); uint256 invalidSignerKey = boundKey(unboundedInvalidSignerKey); vm.assume(signerKey != invalidSignerKey); IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau.agreementId = accepted.rca.agreementId; + rcau.agreementId = agreementId; IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU( rcau, @@ -117,13 +133,15 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { } function test_Update_OK(FuzzyTestUpdate calldata fuzzyTestUpdate) public { - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + ( + IRecurringCollector.SignedRCA memory accepted, + uint256 signerKey, + bytes16 agreementId + ) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau.agreementId = accepted.rca.agreementId; + rcau.agreementId = agreementId; // Don't use fuzzed nonce - use correct nonce for first update rcau.nonce = 1; IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU( @@ -147,7 +165,7 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { vm.prank(accepted.rca.dataService); _recurringCollector.update(signedRCAU); - IRecurringCollector.AgreementData memory agreement = _recurringCollector.getAgreement(accepted.rca.agreementId); + IRecurringCollector.AgreementData memory agreement = _recurringCollector.getAgreement(agreementId); assertEq(rcau.endsAt, agreement.endsAt); assertEq(rcau.maxInitialTokens, agreement.maxInitialTokens); assertEq(rcau.maxOngoingTokensPerSecond, agreement.maxOngoingTokensPerSecond); @@ -157,13 +175,15 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { } function test_Update_Revert_WhenInvalidNonce_TooLow(FuzzyTestUpdate calldata fuzzyTestUpdate) public { - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + ( + IRecurringCollector.SignedRCA memory accepted, + uint256 signerKey, + bytes16 agreementId + ) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau.agreementId = accepted.rca.agreementId; + rcau.agreementId = agreementId; rcau.nonce = 0; // Invalid: should be 1 for first update IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU( @@ -183,13 +203,15 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { } function test_Update_Revert_WhenInvalidNonce_TooHigh(FuzzyTestUpdate calldata fuzzyTestUpdate) public { - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + ( + IRecurringCollector.SignedRCA memory accepted, + uint256 signerKey, + bytes16 agreementId + ) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau.agreementId = accepted.rca.agreementId; + rcau.agreementId = agreementId; rcau.nonce = 5; // Invalid: should be 1 for first update IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU( @@ -209,13 +231,15 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { } function test_Update_Revert_WhenReplayAttack(FuzzyTestUpdate calldata fuzzyTestUpdate) public { - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + ( + IRecurringCollector.SignedRCA memory accepted, + uint256 signerKey, + bytes16 agreementId + ) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau1 = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau1.agreementId = accepted.rca.agreementId; + rcau1.agreementId = agreementId; rcau1.nonce = 1; // First update succeeds @@ -251,21 +275,21 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { } function test_Update_OK_NonceIncrementsCorrectly(FuzzyTestUpdate calldata fuzzyTestUpdate) public { - (IRecurringCollector.SignedRCA memory accepted, uint256 signerKey) = _sensibleAuthorizeAndAccept( - fuzzyTestUpdate.fuzzyTestAccept - ); + ( + IRecurringCollector.SignedRCA memory accepted, + uint256 signerKey, + bytes16 agreementId + ) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept); // Initial nonce should be 0 - IRecurringCollector.AgreementData memory initialAgreement = _recurringCollector.getAgreement( - accepted.rca.agreementId - ); + IRecurringCollector.AgreementData memory initialAgreement = _recurringCollector.getAgreement(agreementId); assertEq(initialAgreement.updateNonce, 0); // First update with nonce 1 IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau1 = _recurringCollectorHelper.sensibleRCAU( fuzzyTestUpdate.rcau ); - rcau1.agreementId = accepted.rca.agreementId; + rcau1.agreementId = agreementId; rcau1.nonce = 1; IRecurringCollector.SignedRCAU memory signedRCAU1 = _recurringCollectorHelper.generateSignedRCAU( @@ -276,9 +300,7 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { _recurringCollector.update(signedRCAU1); // Verify nonce incremented to 1 - IRecurringCollector.AgreementData memory updatedAgreement1 = _recurringCollector.getAgreement( - accepted.rca.agreementId - ); + IRecurringCollector.AgreementData memory updatedAgreement1 = _recurringCollector.getAgreement(agreementId); assertEq(updatedAgreement1.updateNonce, 1); // Second update with nonce 2 @@ -294,9 +316,7 @@ contract RecurringCollectorUpdateTest is RecurringCollectorSharedTest { _recurringCollector.update(signedRCAU2); // Verify nonce incremented to 2 - IRecurringCollector.AgreementData memory updatedAgreement2 = _recurringCollector.getAgreement( - accepted.rca.agreementId - ); + IRecurringCollector.AgreementData memory updatedAgreement2 = _recurringCollector.getAgreement(agreementId); assertEq(updatedAgreement2.updateNonce, 2); } diff --git a/packages/subgraph-service/contracts/SubgraphService.sol b/packages/subgraph-service/contracts/SubgraphService.sol index 6e77e66f9..d311c6c62 100644 --- a/packages/subgraph-service/contracts/SubgraphService.sol +++ b/packages/subgraph-service/contracts/SubgraphService.sol @@ -420,6 +420,7 @@ contract SubgraphService is * * @param allocationId The id of the allocation * @param signedRCA The signed Recurring Collection Agreement + * @return agreementId The ID of the accepted indexing agreement */ function acceptIndexingAgreement( address allocationId, @@ -430,8 +431,9 @@ contract SubgraphService is onlyAuthorizedForProvision(signedRCA.rca.serviceProvider) onlyValidProvision(signedRCA.rca.serviceProvider) onlyRegisteredIndexer(signedRCA.rca.serviceProvider) + returns (bytes16) { - IndexingAgreement._getStorageManager().accept(_allocations, allocationId, signedRCA); + return IndexingAgreement._getStorageManager().accept(_allocations, allocationId, signedRCA); } /** diff --git a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol index 2a852ffce..17ff4cbd0 100644 --- a/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol +++ b/packages/subgraph-service/contracts/interfaces/ISubgraphService.sol @@ -263,8 +263,12 @@ interface ISubgraphService is IDataServiceFees { * @notice Accept an indexing agreement. * @param allocationId The id of the allocation * @param signedRCA The signed recurring collector agreement (RCA) that the indexer accepts + * @return agreementId The ID of the accepted indexing agreement */ - function acceptIndexingAgreement(address allocationId, IRecurringCollector.SignedRCA calldata signedRCA) external; + function acceptIndexingAgreement( + address allocationId, + IRecurringCollector.SignedRCA calldata signedRCA + ) external returns (bytes16); /** * @notice Update an indexing agreement. diff --git a/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol b/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol index ce94418ac..d1f42f2b5 100644 --- a/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol +++ b/packages/subgraph-service/contracts/libraries/IndexingAgreement.sol @@ -293,7 +293,7 @@ library IndexingAgreement { mapping(address allocationId => Allocation.State allocation) storage allocations, address allocationId, IRecurringCollector.SignedRCA calldata signedRCA - ) external { + ) external returns (bytes16) { Allocation.State memory allocation = _requireValidAllocation( allocations, allocationId, @@ -309,9 +309,17 @@ library IndexingAgreement { signedRCA.rca.metadata ); - State storage agreement = self.agreements[signedRCA.rca.agreementId]; + bytes16 agreementId = _directory().recurringCollector().generateAgreementId( + signedRCA.rca.payer, + signedRCA.rca.dataService, + signedRCA.rca.serviceProvider, + signedRCA.rca.deadline, + signedRCA.rca.nonce + ); + + State storage agreement = self.agreements[agreementId]; - require(agreement.allocationId == address(0), IndexingAgreementAlreadyAccepted(signedRCA.rca.agreementId)); + require(agreement.allocationId == address(0), IndexingAgreementAlreadyAccepted(agreementId)); require( allocation.subgraphDeploymentId == metadata.subgraphDeploymentId, @@ -327,25 +335,26 @@ library IndexingAgreement { self.allocationToActiveAgreementId[allocationId] == bytes16(0), AllocationAlreadyHasIndexingAgreement(allocationId) ); - self.allocationToActiveAgreementId[allocationId] = signedRCA.rca.agreementId; + self.allocationToActiveAgreementId[allocationId] = agreementId; agreement.version = metadata.version; agreement.allocationId = allocationId; require(metadata.version == IndexingAgreementVersion.V1, IndexingAgreementInvalidVersion(metadata.version)); - _setTermsV1(self, signedRCA.rca.agreementId, metadata.terms); + _setTermsV1(self, agreementId, metadata.terms); emit IndexingAgreementAccepted( signedRCA.rca.serviceProvider, signedRCA.rca.payer, - signedRCA.rca.agreementId, + agreementId, allocationId, metadata.subgraphDeploymentId, metadata.version, metadata.terms ); - _directory().recurringCollector().accept(signedRCA); + require(_directory().recurringCollector().accept(signedRCA) == agreementId, "internal: agreement ID mismatch"); + return agreementId; } /** diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol index 77b18308c..db2a859dd 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/accept.t.sol @@ -212,11 +212,14 @@ contract SubgraphServiceIndexingAgreementAcceptTest is SubgraphServiceIndexingAg function test_SubgraphService_AcceptIndexingAgreement_Revert_WhenAgreementAlreadyAccepted(Seed memory seed) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (IRecurringCollector.SignedRCA memory accepted, bytes16 agreementId) = _withAcceptedIndexingAgreement( + ctx, + indexerState + ); bytes memory expectedErr = abi.encodeWithSelector( IndexingAgreement.IndexingAgreementAlreadyAccepted.selector, - accepted.rca.agreementId + agreementId ); vm.expectRevert(expectedErr); resetPrank(ctx.indexers[0].addr); @@ -229,10 +232,15 @@ contract SubgraphServiceIndexingAgreementAcceptTest is SubgraphServiceIndexingAg Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); IRecurringCollector.SignedRCA memory acceptable = _generateAcceptableSignedRCA(ctx, indexerState.addr); + IRecurringCollector.RecurringCollectionAgreement memory notAcceptableRCA = acceptable.rca; bytes memory invalidTermsData = bytes("invalid terms data"); - acceptable.rca.metadata = abi.encode( + notAcceptableRCA.metadata = abi.encode( _newAcceptIndexingAgreementMetadataV1Terms(indexerState.subgraphDeploymentId, invalidTermsData) ); + IRecurringCollector.SignedRCA memory notAcceptable = _recurringCollectorHelper.generateSignedRCA( + notAcceptableRCA, + ctx.payer.signerPrivateKey + ); bytes memory expectedErr = abi.encodeWithSelector( IndexingAgreementDecoder.IndexingAgreementDecoderInvalidData.selector, @@ -241,7 +249,7 @@ contract SubgraphServiceIndexingAgreementAcceptTest is SubgraphServiceIndexingAg ); vm.expectRevert(expectedErr); resetPrank(indexerState.addr); - subgraphService.acceptIndexingAgreement(indexerState.allocationId, acceptable); + subgraphService.acceptIndexingAgreement(indexerState.allocationId, notAcceptable); } function test_SubgraphService_AcceptIndexingAgreement(Seed memory seed) public { @@ -252,11 +260,20 @@ contract SubgraphServiceIndexingAgreementAcceptTest is SubgraphServiceIndexingAg acceptable.rca.metadata, (IndexingAgreement.AcceptIndexingAgreementMetadata) ); + // Generate deterministic agreement ID for event expectation + bytes16 expectedAgreementId = recurringCollector.generateAgreementId( + acceptable.rca.payer, + acceptable.rca.dataService, + acceptable.rca.serviceProvider, + acceptable.rca.deadline, + acceptable.rca.nonce + ); + vm.expectEmit(address(subgraphService)); emit IndexingAgreement.IndexingAgreementAccepted( acceptable.rca.serviceProvider, acceptable.rca.payer, - acceptable.rca.agreementId, + expectedAgreementId, indexerState.allocationId, metadata.subgraphDeploymentId, metadata.version, diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol index 2eda9dfc0..5b7aba56f 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/base.t.sol @@ -13,22 +13,29 @@ contract SubgraphServiceIndexingAgreementBaseTest is SubgraphServiceIndexingAgre */ /* solhint-disable graph/func-name-mixedcase */ - function test_SubgraphService_GetIndexingAgreement(Seed memory seed, address operator, bytes16 agreementId) public { + function test_SubgraphService_GetIndexingAgreement( + Seed memory seed, + address operator, + bytes16 fuzzyAgreementId + ) public { vm.assume(_isSafeSubgraphServiceCaller(operator)); resetPrank(address(operator)); // Get unkown indexing agreement - vm.expectRevert(abi.encodeWithSelector(IndexingAgreement.IndexingAgreementNotActive.selector, agreementId)); - subgraphService.getIndexingAgreement(agreementId); + vm.expectRevert( + abi.encodeWithSelector(IndexingAgreement.IndexingAgreementNotActive.selector, fuzzyAgreementId) + ); + subgraphService.getIndexingAgreement(fuzzyAgreementId); // Accept an indexing agreement Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); - IndexingAgreement.AgreementWrapper memory agreement = subgraphService.getIndexingAgreement( - accepted.rca.agreementId + (IRecurringCollector.SignedRCA memory accepted, bytes16 agreementId) = _withAcceptedIndexingAgreement( + ctx, + indexerState ); + IndexingAgreement.AgreementWrapper memory agreement = subgraphService.getIndexingAgreement(agreementId); _assertEqualAgreement(accepted.rca, agreement); } diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/cancel.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/cancel.t.sol index 60a28169c..2c904f156 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/cancel.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/cancel.t.sol @@ -33,7 +33,10 @@ contract SubgraphServiceIndexingAgreementCancelTest is SubgraphServiceIndexingAg address rando ) public withSafeIndexerOrOperator(rando) { Context storage ctx = _newCtx(seed); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, _withIndexer(ctx)); + (IRecurringCollector.SignedRCA memory accepted, bytes16 agreementId) = _withAcceptedIndexingAgreement( + ctx, + _withIndexer(ctx) + ); bytes memory expectedErr = abi.encodeWithSelector( IndexingAgreement.IndexingAgreementNonCancelableBy.selector, @@ -42,7 +45,7 @@ contract SubgraphServiceIndexingAgreementCancelTest is SubgraphServiceIndexingAg ); vm.expectRevert(expectedErr); resetPrank(rando); - subgraphService.cancelIndexingAgreementByPayer(accepted.rca.agreementId); + subgraphService.cancelIndexingAgreementByPayer(agreementId); } function test_SubgraphService_CancelIndexingAgreementByPayer_Revert_WhenNotAccepted( @@ -67,28 +70,34 @@ contract SubgraphServiceIndexingAgreementCancelTest is SubgraphServiceIndexingAg ) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (IRecurringCollector.SignedRCA memory accepted, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement( + ctx, + indexerState + ); IRecurringCollector.CancelAgreementBy by = cancelSource ? IRecurringCollector.CancelAgreementBy.ServiceProvider : IRecurringCollector.CancelAgreementBy.Payer; - _cancelAgreement(ctx, accepted.rca.agreementId, indexerState.addr, accepted.rca.payer, by); + _cancelAgreement(ctx, acceptedAgreementId, indexerState.addr, accepted.rca.payer, by); resetPrank(indexerState.addr); bytes memory expectedErr = abi.encodeWithSelector( IndexingAgreement.IndexingAgreementNotActive.selector, - accepted.rca.agreementId + acceptedAgreementId ); vm.expectRevert(expectedErr); - subgraphService.cancelIndexingAgreementByPayer(accepted.rca.agreementId); + subgraphService.cancelIndexingAgreementByPayer(acceptedAgreementId); } function test_SubgraphService_CancelIndexingAgreementByPayer(Seed memory seed) public { Context storage ctx = _newCtx(seed); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, _withIndexer(ctx)); + (IRecurringCollector.SignedRCA memory accepted, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement( + ctx, + _withIndexer(ctx) + ); _cancelAgreement( ctx, - accepted.rca.agreementId, + acceptedAgreementId, accepted.rca.serviceProvider, accepted.rca.payer, IRecurringCollector.CancelAgreementBy.Payer @@ -184,28 +193,34 @@ contract SubgraphServiceIndexingAgreementCancelTest is SubgraphServiceIndexingAg ) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (IRecurringCollector.SignedRCA memory accepted, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement( + ctx, + indexerState + ); IRecurringCollector.CancelAgreementBy by = cancelSource ? IRecurringCollector.CancelAgreementBy.ServiceProvider : IRecurringCollector.CancelAgreementBy.Payer; - _cancelAgreement(ctx, accepted.rca.agreementId, accepted.rca.serviceProvider, accepted.rca.payer, by); + _cancelAgreement(ctx, acceptedAgreementId, accepted.rca.serviceProvider, accepted.rca.payer, by); resetPrank(indexerState.addr); bytes memory expectedErr = abi.encodeWithSelector( IndexingAgreement.IndexingAgreementNotActive.selector, - accepted.rca.agreementId + acceptedAgreementId ); vm.expectRevert(expectedErr); - subgraphService.cancelIndexingAgreement(indexerState.addr, accepted.rca.agreementId); + subgraphService.cancelIndexingAgreement(indexerState.addr, acceptedAgreementId); } function test_SubgraphService_CancelIndexingAgreement_OK(Seed memory seed) public { Context storage ctx = _newCtx(seed); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, _withIndexer(ctx)); + (IRecurringCollector.SignedRCA memory accepted, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement( + ctx, + _withIndexer(ctx) + ); _cancelAgreement( ctx, - accepted.rca.agreementId, + acceptedAgreementId, accepted.rca.serviceProvider, accepted.rca.payer, IRecurringCollector.CancelAgreementBy.ServiceProvider diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol index 6f9c2563d..711154be5 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/collect.t.sol @@ -29,7 +29,10 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA ) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (IRecurringCollector.SignedRCA memory accepted, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement( + ctx, + indexerState + ); assertEq(subgraphService.feesProvisionTracker(indexerState.addr), 0, "Should be 0 before collect"); @@ -38,7 +41,7 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA bytes memory data = abi.encode( IRecurringCollector.CollectParams({ - agreementId: accepted.rca.agreementId, + agreementId: acceptedAgreementId, collectionId: bytes32(uint256(uint160(indexerState.allocationId))), tokens: 0, dataServiceCut: 0, @@ -46,33 +49,18 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA }) ); uint256 tokensCollected = bound(unboundedTokensCollected, 1, indexerState.tokens / stakeToFeesRatio); + vm.mockCall( address(recurringCollector), abi.encodeWithSelector(IPaymentsCollector.collect.selector, IGraphPayments.PaymentTypes.IndexingFee, data), abi.encode(tokensCollected) ); - vm.expectCall( - address(recurringCollector), - abi.encodeCall(IPaymentsCollector.collect, (IGraphPayments.PaymentTypes.IndexingFee, data)) - ); - vm.expectEmit(address(subgraphService)); - emit IndexingAgreement.IndexingFeesCollectedV1( - indexerState.addr, - accepted.rca.payer, - accepted.rca.agreementId, - indexerState.allocationId, - indexerState.subgraphDeploymentId, - epochManager.currentEpoch(), - tokensCollected, - entities, - poi, - epochManager.currentEpochBlock(), - bytes("") - ); + _expectCollectCallAndEmit(data, indexerState, accepted, acceptedAgreementId, tokensCollected, entities, poi); + subgraphService.collect( indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, - _encodeCollectDataV1(accepted.rca.agreementId, entities, poi, epochManager.currentEpochBlock(), bytes("")) + _encodeCollectDataV1(acceptedAgreementId, entities, poi, epochManager.currentEpochBlock(), bytes("")) ); assertEq( @@ -214,7 +202,7 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA function test_SubgraphService_CollectIndexingFees_Reverts_WhenInvalidNestedData(Seed memory seed) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement(ctx, indexerState); resetPrank(indexerState.addr); @@ -225,10 +213,11 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA invalidNestedData ); vm.expectRevert(expectedErr); + subgraphService.collect( indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, - _encodeCollectData(accepted.rca.agreementId, invalidNestedData) + _encodeCollectData(acceptedAgreementId, invalidNestedData) ); } @@ -240,7 +229,7 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); IndexerState memory otherIndexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement(ctx, indexerState); vm.assume(otherIndexerState.addr != indexerState.addr); @@ -250,14 +239,14 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA bytes memory expectedErr = abi.encodeWithSelector( IndexingAgreement.IndexingAgreementNotAuthorized.selector, - accepted.rca.agreementId, + acceptedAgreementId, otherIndexerState.addr ); vm.expectRevert(expectedErr); subgraphService.collect( otherIndexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, - _encodeCollectDataV1(accepted.rca.agreementId, entities, poi, currentEpochBlock, bytes("")) + _encodeCollectDataV1(acceptedAgreementId, entities, poi, currentEpochBlock, bytes("")) ); } @@ -268,7 +257,7 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA ) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement(ctx, indexerState); resetPrank(indexerState.addr); subgraphService.stopService(indexerState.addr, abi.encode(indexerState.allocationId)); @@ -283,7 +272,7 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA subgraphService.collect( indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, - _encodeCollectDataV1(accepted.rca.agreementId, entities, poi, currentEpochBlock, bytes("")) + _encodeCollectDataV1(acceptedAgreementId, entities, poi, currentEpochBlock, bytes("")) ); } @@ -294,7 +283,7 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA ) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (, bytes16 acceptedAgreementId) = _withAcceptedIndexingAgreement(ctx, indexerState); skip(maxPOIStaleness + 1); resetPrank(indexerState.addr); @@ -310,8 +299,38 @@ contract SubgraphServiceIndexingAgreementCollectTest is SubgraphServiceIndexingA subgraphService.collect( indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, - _encodeCollectDataV1(accepted.rca.agreementId, entities, poi, currentEpochBlock, bytes("")) + _encodeCollectDataV1(acceptedAgreementId, entities, poi, currentEpochBlock, bytes("")) ); } + /* solhint-enable graph/func-name-mixedcase */ + + function _expectCollectCallAndEmit( + bytes memory _data, + IndexerState memory _indexerState, + IRecurringCollector.SignedRCA memory _accepted, + bytes16 _acceptedAgreementId, + uint256 _tokensCollected, + uint256 _entities, + bytes32 _poi + ) private { + vm.expectCall( + address(recurringCollector), + abi.encodeCall(IPaymentsCollector.collect, (IGraphPayments.PaymentTypes.IndexingFee, _data)) + ); + vm.expectEmit(address(subgraphService)); + emit IndexingAgreement.IndexingFeesCollectedV1( + _indexerState.addr, + _accepted.rca.payer, + _acceptedAgreementId, + _indexerState.allocationId, + _indexerState.subgraphDeploymentId, + epochManager.currentEpoch(), + _tokensCollected, + _entities, + _poi, + epochManager.currentEpochBlock(), + bytes("") + ); + } } diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol index 5c8758370..45b7db8d8 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/integration.t.sol @@ -42,7 +42,7 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA( ctx.ctxInternal.seed.rca ); - _sharedSetup(ctx, rca, indexerState, expectedTokens); + bytes16 acceptedAgreementId = _sharedSetup(ctx, rca, indexerState, expectedTokens); TestState memory beforeCollect = _getState(rca.payer, indexerState.addr); @@ -52,7 +52,7 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, _encodeCollectDataV1( - rca.agreementId, + acceptedAgreementId, 1, keccak256(abi.encodePacked("poi")), epochManager.currentEpochBlock(), @@ -75,11 +75,11 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA( ctx.ctxInternal.seed.rca ); - _sharedSetup(ctx, rca, indexerState, expectedTokens); + bytes16 acceptedAgreementId = _sharedSetup(ctx, rca, indexerState, expectedTokens); // Cancel the indexing agreement by the payer resetPrank(ctx.payer.signer); - subgraphService.cancelIndexingAgreementByPayer(rca.agreementId); + subgraphService.cancelIndexingAgreementByPayer(acceptedAgreementId); TestState memory beforeCollect = _getState(rca.payer, indexerState.addr); @@ -89,7 +89,7 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex indexerState.addr, IGraphPayments.PaymentTypes.IndexingFee, _encodeCollectDataV1( - rca.agreementId, + acceptedAgreementId, 1, keccak256(abi.encodePacked("poi")), epochManager.currentEpochBlock(), @@ -108,7 +108,7 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex IRecurringCollector.RecurringCollectionAgreement memory _rca, IndexerState memory _indexerState, ExpectedTokens memory _expectedTokens - ) internal { + ) internal returns (bytes16) { _addTokensToProvision(_indexerState, _expectedTokens.expectedTokensLocked); IndexingAgreement.IndexingAgreementTermsV1 memory terms = IndexingAgreement.IndexingAgreementTermsV1({ @@ -137,13 +137,15 @@ contract SubgraphServiceIndexingAgreementIntegrationTest is SubgraphServiceIndex subgraphService.setPaymentsDestination(_indexerState.addr); // Accept the Indexing Agreement - subgraphService.acceptIndexingAgreement( + bytes16 agreementId = subgraphService.acceptIndexingAgreement( _indexerState.allocationId, _recurringCollectorHelper.generateSignedRCA(_rca, _ctx.payer.signerPrivateKey) ); // Skip ahead to collection point skip(_expectedTokens.expectedTotalTokensCollected / terms.tokensPerSecond); + + return agreementId; } function _newExpectedTokens(uint256 _fuzzyTokensCollected) internal view returns (ExpectedTokens memory) { diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol index c23727f20..f04be267b 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/shared.t.sol @@ -169,7 +169,7 @@ contract SubgraphServiceIndexingAgreementSharedTest is SubgraphServiceTest, Boun function _withAcceptedIndexingAgreement( Context storage _ctx, IndexerState memory _indexerState - ) internal returns (IRecurringCollector.SignedRCA memory) { + ) internal returns (IRecurringCollector.SignedRCA memory, bytes16 agreementId) { IRecurringCollector.RecurringCollectionAgreement memory rca = _ctx.ctxInternal.seed.rca; IndexingAgreement.AcceptIndexingAgreementMetadata memory metadata = _newAcceptIndexingAgreementMetadataV1( @@ -187,20 +187,31 @@ contract SubgraphServiceIndexingAgreementSharedTest is SubgraphServiceTest, Boun ); _recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, _ctx.payer.signerPrivateKey); + // Generate deterministic agreement ID for event expectation + agreementId = recurringCollector.generateAgreementId( + rca.payer, + rca.dataService, + rca.serviceProvider, + rca.deadline, + rca.nonce + ); + vm.expectEmit(address(subgraphService)); emit IndexingAgreement.IndexingAgreementAccepted( rca.serviceProvider, rca.payer, - rca.agreementId, + agreementId, _indexerState.allocationId, metadata.subgraphDeploymentId, metadata.version, metadata.terms ); _subgraphServiceSafePrank(_indexerState.addr); - subgraphService.acceptIndexingAgreement(_indexerState.allocationId, signedRCA); + bytes16 actualAgreementId = subgraphService.acceptIndexingAgreement(_indexerState.allocationId, signedRCA); - return signedRCA; + // Verify the agreement ID matches expectation + assertEq(actualAgreementId, agreementId); + return (signedRCA, agreementId); } function _newCtx(Seed memory _seed) internal returns (Context storage) { @@ -267,7 +278,14 @@ contract SubgraphServiceIndexingAgreementSharedTest is SubgraphServiceTest, Boun IRecurringCollector.RecurringCollectionAgreement memory _rca ) internal view returns (IRecurringCollector.RecurringCollectionAgreementUpdate memory) { IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _ctx.ctxInternal.seed.rcau; - rcau.agreementId = _rca.agreementId; + // Generate deterministic agreement ID for the update + rcau.agreementId = recurringCollector.generateAgreementId( + _rca.payer, + _rca.dataService, + _rca.serviceProvider, + _rca.deadline, + _rca.nonce + ); rcau.metadata = _encodeUpdateIndexingAgreementMetadataV1( _newUpdateIndexingAgreementMetadataV1( _ctx.ctxInternal.seed.termsV1.tokensPerSecond, diff --git a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/update.t.sol b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/update.t.sol index ebd9200d1..ba14d8e55 100644 --- a/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/update.t.sol +++ b/packages/subgraph-service/test/unit/subgraphService/indexing-agreement/update.t.sol @@ -107,7 +107,7 @@ contract SubgraphServiceIndexingAgreementUpgradeTest is SubgraphServiceIndexingA Context storage ctx = _newCtx(seed); IndexerState memory indexerStateA = _withIndexer(ctx); IndexerState memory indexerStateB = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerStateA); + (IRecurringCollector.SignedRCA memory accepted, ) = _withAcceptedIndexingAgreement(ctx, indexerStateA); IRecurringCollector.SignedRCAU memory acceptableUpdate = _generateAcceptableSignedRCAU(ctx, accepted.rca); bytes memory expectedErr = abi.encodeWithSelector( @@ -123,7 +123,7 @@ contract SubgraphServiceIndexingAgreementUpgradeTest is SubgraphServiceIndexingA function test_SubgraphService_UpdateIndexingAgreement_Revert_WhenInvalidMetadata(Seed memory seed) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (IRecurringCollector.SignedRCA memory accepted, ) = _withAcceptedIndexingAgreement(ctx, indexerState); IRecurringCollector.RecurringCollectionAgreementUpdate memory acceptableUpdate = _generateAcceptableRecurringCollectionAgreementUpdate(ctx, accepted.rca); acceptableUpdate.metadata = bytes("invalid"); @@ -147,7 +147,7 @@ contract SubgraphServiceIndexingAgreementUpgradeTest is SubgraphServiceIndexingA function test_SubgraphService_UpdateIndexingAgreement_OK(Seed memory seed) public { Context storage ctx = _newCtx(seed); IndexerState memory indexerState = _withIndexer(ctx); - IRecurringCollector.SignedRCA memory accepted = _withAcceptedIndexingAgreement(ctx, indexerState); + (IRecurringCollector.SignedRCA memory accepted, ) = _withAcceptedIndexingAgreement(ctx, indexerState); IRecurringCollector.SignedRCAU memory acceptableUpdate = _generateAcceptableSignedRCAU(ctx, accepted.rca); IndexingAgreement.UpdateIndexingAgreementMetadata memory metadata = abi.decode(