Skip to content

Commit

Permalink
fix: prevent payers from being able to bypass escrow mechanism (TRST-…
Browse files Browse the repository at this point in the history 
…H01)

Signed-off-by: Tomás Migone <[email protected]>
  • Loading branch information
@tmigone
tmigone committed Nov 26, 2024
1 parent 0c0d090 commit 65e99f9
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 27 deletions.
6 changes: 6 additions & 0 deletions packages/horizon/contracts/interfaces/ITAPCollector.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ interface ITAPCollector is IPaymentsCollector {
*/
error TAPCollectorInvalidRAVSigner();

/**
* Thrown when the RAV is for a data service the service provider has no provision for
* @param dataService The address of the data service
*/
error TAPCollectorUnauthorizedDataService(address dataService);

/**
* Thrown when the caller is not the data service the RAV was issued to
* @param caller The address of the caller
Expand Down
11 changes: 11 additions & 0 deletions packages/horizon/contracts/payments/collectors/TAPCollector.sol
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.27;

import { IHorizonStaking } from "../../interfaces/IHorizonStaking.sol";
import { IGraphPayments } from "../../interfaces/IGraphPayments.sol";
import { ITAPCollector } from "../../interfaces/ITAPCollector.sol";

Expand Down Expand Up @@ -118,6 +119,7 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector {
* @notice Initiate a payment collection through the payments protocol
* See {IGraphPayments.collect}.
* @dev Caller must be the data service the RAV was issued to.
* @dev Service provider must have an active provision with the data service to collect payments
* @notice REVERT: This function may revert if ECDSA.recover fails, check ECDSA library for details.
*/
function collect(IGraphPayments.PaymentTypes paymentType, bytes memory data) external override returns (uint256) {
Expand All @@ -130,6 +132,15 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector {
address signer = _recoverRAVSigner(signedRAV);
require(authorizedSigners[signer].payer != address(0), TAPCollectorInvalidRAVSigner());

// Check the service provider has an active provision with the data service
// This prevents an attack where the payer can deny the service provider from collecting payments
// by using a signer as data service to syphon off the tokens in the escrow to an account they control
uint256 tokensAvailable = _graphStaking().getProviderTokensAvailable(
signedRAV.rav.serviceProvider,
signedRAV.rav.dataService
);
require(tokensAvailable > 0, TAPCollectorUnauthorizedDataService(signedRAV.rav.dataService));

return _collect(paymentType, authorizedSigners[signer].payer, signedRAV, dataServiceCut);
}

Expand Down
141 changes: 114 additions & 27 deletions packages/horizon/test/payments/tap-collector/collect/collect.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ import { IGraphPayments } from "../../../../contracts/interfaces/IGraphPayments.
import { TAPCollectorTest } from "../TAPCollector.t.sol";

contract TAPCollectorCollectTest is TAPCollectorTest {

/*
* HELPERS
*/
* HELPERS
*/

function _getQueryFeeEncodedData(
uint256 _signerPrivateKey,
Expand Down Expand Up @@ -47,38 +46,122 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
* TESTS
*/

function testTAPCollector_Collect(uint256 tokens) public useGateway useSigner {
function testTAPCollector_Collect(
uint256 tokens
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_Multiple(uint256 tokens, uint8 steps) public useGateway useSigner {
function testTAPCollector_Collect_Multiple(
uint256 tokens,
uint8 steps
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
steps = uint8(bound(steps, 1, 100));
tokens = bound(tokens, steps, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

resetPrank(users.verifier);
uint256 payed = 0;
uint256 tokensPerStep = tokens / steps;
for (uint256 i = 0; i < steps; i++) {
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(payed + tokensPerStep));
bytes memory data = _getQueryFeeEncodedData(
signerPrivateKey,
users.indexer,
users.verifier,
uint128(payed + tokensPerStep)
);
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
payed += tokensPerStep;
}
}

function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
bytes memory expectedError = abi.encodeWithSelector(
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
users.verifier
);
vm.expectRevert(expectedError);
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_RevertWhen_ProvisionEmpty(
uint256 tokens
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
// thaw tokens from the provision
resetPrank(users.indexer);
staking.thaw(users.indexer, users.verifier, 100);

tokens = bound(tokens, 1, type(uint128).max);

resetPrank(users.gateway);
_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
bytes memory expectedError = abi.encodeWithSelector(
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
users.verifier
);
vm.expectRevert(expectedError);
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_PreventSignerAttack(
uint256 tokens
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

resetPrank(users.gateway);
_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

// The sender authorizes another signer
(address anotherSigner, uint256 anotherSignerPrivateKey) = makeAddrAndKey("anotherSigner");
uint256 proofDeadline = block.timestamp + 1;
bytes memory anotherSignerProof = _getSignerProof(proofDeadline, anotherSignerPrivateKey);
_authorizeSigner(anotherSigner, proofDeadline, anotherSignerProof);

// And crafts a RAV using the new signer as the data service
bytes memory data = _getQueryFeeEncodedData(
anotherSignerPrivateKey,
users.indexer,
anotherSigner,
uint128(tokens)
);

// the call should revert because the service provider has no provision with the "data service"
resetPrank(anotherSigner);
bytes memory expectedError = abi.encodeWithSelector(
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
anotherSigner
);
vm.expectRevert(expectedError);
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_RevertWhen_CallerNotDataService(uint256 tokens) public useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

resetPrank(users.gateway);
_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);
Expand All @@ -95,9 +178,11 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(uint256 tokens) public useGateway useSigner {
function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(
uint256 tokens
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
Expand All @@ -106,37 +191,37 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
_collect(IGraphPayments.PaymentTypes.QueryFee, data);

// Attempt to collect again
vm.expectRevert(abi.encodeWithSelector(
ITAPCollector.TAPCollectorInconsistentRAVTokens.selector,
tokens,
tokens
));
vm.expectRevert(
abi.encodeWithSelector(ITAPCollector.TAPCollectorInconsistentRAVTokens.selector, tokens, tokens)
);
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_RevertWhen_SignerNotAuthorized(uint256 tokens) public useGateway {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
vm.expectRevert(abi.encodeWithSelector(ITAPCollector.TAPCollectorInvalidRAVSigner.selector));
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_ThawingSigner(uint256 tokens) public useGateway useSigner {
function testTAPCollector_Collect_ThawingSigner(
uint256 tokens
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

// Start thawing signer
_thawSigner(signer);
skip(revokeSignerThawingPeriod + 1);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
Expand All @@ -145,33 +230,35 @@ contract TAPCollectorCollectTest is TAPCollectorTest {

function testTAPCollector_Collect_RevertIf_SignerWasRevoked(uint256 tokens) public useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

// Start thawing signer
_thawSigner(signer);
skip(revokeSignerThawingPeriod + 1);
_revokeAuthorizedSigner(signer);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
vm.expectRevert(abi.encodeWithSelector(ITAPCollector.TAPCollectorInvalidRAVSigner.selector));
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
}

function testTAPCollector_Collect_ThawingSignerCanceled(uint256 tokens) public useGateway useSigner {
function testTAPCollector_Collect_ThawingSignerCanceled(
uint256 tokens
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
tokens = bound(tokens, 1, type(uint128).max);

_approveCollector(address(tapCollector), tokens);
_depositTokens(address(tapCollector), users.indexer, tokens);

// Start thawing signer
_thawSigner(signer);
skip(revokeSignerThawingPeriod + 1);
_cancelThawSigner(signer);

bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));

resetPrank(users.verifier);
Expand Down

0 comments on commit 65e99f9

Please sign in to comment.