Skip to content

Commit da0427a

Browse files
tmigonetmigone
committed
fix: prevent payers from being able to bypass escrow mechanism (TRST-H01)
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
1 parent 0c0d090 commit da0427a

3 files changed

Lines changed: 96 additions & 27 deletions

File tree

‎packages/horizon/contracts/interfaces/ITAPCollector.sol‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ interface ITAPCollector is IPaymentsCollector {
137137
*/
138138
error TAPCollectorInvalidRAVSigner();
139139

140+
/**
141+
* Thrown when the RAV is for a data service the service provider has no provision for
142+
* @param dataService The address of the data service
143+
*/
144+
error TAPCollectorUnauthorizedDataService(address dataService);
145+
140146
/**
141147
* Thrown when the caller is not the data service the RAV was issued to
142148
* @param caller The address of the caller

‎packages/horizon/contracts/payments/collectors/TAPCollector.sol‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: GPL-3.0-or-later
22
pragma solidity 0.8.27;
33

4+
import { IHorizonStaking } from "../../interfaces/IHorizonStaking.sol";
45
import { IGraphPayments } from "../../interfaces/IGraphPayments.sol";
56
import { ITAPCollector } from "../../interfaces/ITAPCollector.sol";
67

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

135+
// Check the service provider has an active provision with the data service
136+
// This prevents an attack where the payer can deny the service provider from collecting payments
137+
// by using a signer as data service to syphon off the tokens in the escrow to an account they control
138+
uint256 tokensAvailable = _graphStaking().getProviderTokensAvailable(
139+
signedRAV.rav.serviceProvider,
140+
signedRAV.rav.dataService
141+
);
142+
require(tokensAvailable > 0, TAPCollectorUnauthorizedDataService(signedRAV.rav.dataService));
143+
133144
return _collect(paymentType, authorizedSigners[signer].payer, signedRAV, dataServiceCut);
134145
}
135146

‎packages/horizon/test/payments/tap-collector/collect/collect.t.sol‎

Lines changed: 79 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import { IGraphPayments } from "../../../../contracts/interfaces/IGraphPayments.
99
import { TAPCollectorTest } from "../TAPCollector.t.sol";
1010

1111
contract TAPCollectorCollectTest is TAPCollectorTest {
12-
1312
/*
14-
* HELPERS
15-
*/
13+
* HELPERS
14+
*/
1615

1716
function _getQueryFeeEncodedData(
1817
uint256 _signerPrivateKey,
@@ -47,38 +46,87 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
4746
* TESTS
4847
*/
4948

50-
function testTAPCollector_Collect(uint256 tokens) public useGateway useSigner {
49+
function testTAPCollector_Collect(
50+
uint256 tokens
51+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
5152
tokens = bound(tokens, 1, type(uint128).max);
52-
53+
5354
_approveCollector(address(tapCollector), tokens);
5455
_depositTokens(address(tapCollector), users.indexer, tokens);
55-
56+
5657
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
5758

5859
resetPrank(users.verifier);
5960
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
6061
}
6162

62-
function testTAPCollector_Collect_Multiple(uint256 tokens, uint8 steps) public useGateway useSigner {
63+
function testTAPCollector_Collect_Multiple(
64+
uint256 tokens,
65+
uint8 steps
66+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
6367
steps = uint8(bound(steps, 1, 100));
6468
tokens = bound(tokens, steps, type(uint128).max);
65-
69+
6670
_approveCollector(address(tapCollector), tokens);
6771
_depositTokens(address(tapCollector), users.indexer, tokens);
6872

6973
resetPrank(users.verifier);
7074
uint256 payed = 0;
7175
uint256 tokensPerStep = tokens / steps;
7276
for (uint256 i = 0; i < steps; i++) {
73-
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(payed + tokensPerStep));
77+
bytes memory data = _getQueryFeeEncodedData(
78+
signerPrivateKey,
79+
users.indexer,
80+
users.verifier,
81+
uint128(payed + tokensPerStep)
82+
);
7483
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
7584
payed += tokensPerStep;
7685
}
7786
}
7887

88+
function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner {
89+
tokens = bound(tokens, 1, type(uint128).max);
90+
91+
_approveCollector(address(tapCollector), tokens);
92+
_depositTokens(address(tapCollector), users.indexer, tokens);
93+
94+
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
95+
96+
resetPrank(users.verifier);
97+
bytes memory expectedError = abi.encodeWithSelector(
98+
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
99+
users.verifier
100+
);
101+
vm.expectRevert(expectedError);
102+
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
103+
}
104+
105+
function testTAPCollector_Collect_RevertWhen_ProvisionEmpty(uint256 tokens) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
106+
// thaw tokens from the provision
107+
resetPrank(users.indexer);
108+
staking.thaw(users.indexer, users.verifier, 100);
109+
110+
tokens = bound(tokens, 1, type(uint128).max);
111+
112+
resetPrank(users.gateway);
113+
_approveCollector(address(tapCollector), tokens);
114+
_depositTokens(address(tapCollector), users.indexer, tokens);
115+
116+
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
117+
118+
resetPrank(users.verifier);
119+
bytes memory expectedError = abi.encodeWithSelector(
120+
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
121+
users.verifier
122+
);
123+
vm.expectRevert(expectedError);
124+
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
125+
}
126+
79127
function testTAPCollector_Collect_RevertWhen_CallerNotDataService(uint256 tokens) public useGateway useSigner {
80128
tokens = bound(tokens, 1, type(uint128).max);
81-
129+
82130
resetPrank(users.gateway);
83131
_approveCollector(address(tapCollector), tokens);
84132
_depositTokens(address(tapCollector), users.indexer, tokens);
@@ -95,9 +143,11 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
95143
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
96144
}
97145

98-
function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(uint256 tokens) public useGateway useSigner {
146+
function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(
147+
uint256 tokens
148+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
99149
tokens = bound(tokens, 1, type(uint128).max);
100-
150+
101151
_approveCollector(address(tapCollector), tokens);
102152
_depositTokens(address(tapCollector), users.indexer, tokens);
103153
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
@@ -106,37 +156,37 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
106156
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
107157

108158
// Attempt to collect again
109-
vm.expectRevert(abi.encodeWithSelector(
110-
ITAPCollector.TAPCollectorInconsistentRAVTokens.selector,
111-
tokens,
112-
tokens
113-
));
159+
vm.expectRevert(
160+
abi.encodeWithSelector(ITAPCollector.TAPCollectorInconsistentRAVTokens.selector, tokens, tokens)
161+
);
114162
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
115163
}
116164

117165
function testTAPCollector_Collect_RevertWhen_SignerNotAuthorized(uint256 tokens) public useGateway {
118166
tokens = bound(tokens, 1, type(uint128).max);
119-
167+
120168
_approveCollector(address(tapCollector), tokens);
121169
_depositTokens(address(tapCollector), users.indexer, tokens);
122-
170+
123171
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
124172

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

130-
function testTAPCollector_Collect_ThawingSigner(uint256 tokens) public useGateway useSigner {
178+
function testTAPCollector_Collect_ThawingSigner(
179+
uint256 tokens
180+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
131181
tokens = bound(tokens, 1, type(uint128).max);
132-
182+
133183
_approveCollector(address(tapCollector), tokens);
134184
_depositTokens(address(tapCollector), users.indexer, tokens);
135185

136186
// Start thawing signer
137187
_thawSigner(signer);
138188
skip(revokeSignerThawingPeriod + 1);
139-
189+
140190
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
141191

142192
resetPrank(users.verifier);
@@ -145,33 +195,35 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
145195

146196
function testTAPCollector_Collect_RevertIf_SignerWasRevoked(uint256 tokens) public useGateway useSigner {
147197
tokens = bound(tokens, 1, type(uint128).max);
148-
198+
149199
_approveCollector(address(tapCollector), tokens);
150200
_depositTokens(address(tapCollector), users.indexer, tokens);
151201

152202
// Start thawing signer
153203
_thawSigner(signer);
154204
skip(revokeSignerThawingPeriod + 1);
155205
_revokeAuthorizedSigner(signer);
156-
206+
157207
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
158208

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

164-
function testTAPCollector_Collect_ThawingSignerCanceled(uint256 tokens) public useGateway useSigner {
214+
function testTAPCollector_Collect_ThawingSignerCanceled(
215+
uint256 tokens
216+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
165217
tokens = bound(tokens, 1, type(uint128).max);
166-
218+
167219
_approveCollector(address(tapCollector), tokens);
168220
_depositTokens(address(tapCollector), users.indexer, tokens);
169221

170222
// Start thawing signer
171223
_thawSigner(signer);
172224
skip(revokeSignerThawingPeriod + 1);
173225
_cancelThawSigner(signer);
174-
226+
175227
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
176228

177229
resetPrank(users.verifier);

0 commit comments

Comments
 (0)