Skip to content

Commit a065804

Browse files
tmigonetmigone
committed
fix: refactor payments cut distribution (TRST-L12)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 2bf4456 commit a065804

File tree

13 files changed

+227
-165
lines changed

13 files changed

+227
-165
lines changed

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

+22-13
Original file line numberDiff line numberDiff line change
@@ -23,48 +23,57 @@ interface IGraphPayments {
2323
* @param payer The address of the payer
2424
* @param receiver The address of the receiver
2525
* @param dataService The address of the data service
26-
* @param tokensReceiver Amount of tokens for the receiver
27-
* @param tokensDelegationPool Amount of tokens for delegators
28-
* @param tokensDataService Amount of tokens for the data service
26+
* @param tokens The total amount of tokens being collected
2927
* @param tokensProtocol Amount of tokens charged as protocol tax
28+
* @param tokensDataService Amount of tokens for the data service
29+
* @param tokensDelegationPool Amount of tokens for delegators
30+
* @param tokensReceiver Amount of tokens for the receiver
3031
*/
31-
event PaymentCollected(
32+
event GraphPaymentCollected(
3233
address indexed payer,
3334
address indexed receiver,
3435
address indexed dataService,
35-
uint256 tokensReceiver,
36-
uint256 tokensDelegationPool,
36+
uint256 tokens,
37+
uint256 tokensProtocol,
3738
uint256 tokensDataService,
38-
uint256 tokensProtocol
39+
uint256 tokensDelegationPool,
40+
uint256 tokensReceiver
3941
);
4042

4143
/**
42-
* @notice Thrown when there are insufficient tokens to pay the required amount
43-
* @param tokens The amount of tokens available
44-
* @param minTokens The amount of tokens being collected
44+
* @notice Thrown when the calculated amount of tokens to be paid out to all parties is
45+
* not the same as the amount of tokens being collected
46+
* @param tokens The amount of tokens being collected
47+
* @param tokensCalculated The sum of all the tokens to be paid out
4548
*/
46-
error GraphPaymentsInsufficientTokens(uint256 tokens, uint256 minTokens);
49+
error GraphPaymentsBadAccounting(uint256 tokens, uint256 tokensCalculated);
4750

4851
/**
4952
* @notice Thrown when the protocol payment cut is invalid
5053
* @param protocolPaymentCut The protocol payment cut
5154
*/
5255
error GraphPaymentsInvalidProtocolPaymentCut(uint256 protocolPaymentCut);
5356

57+
/**
58+
* @notice Thrown when trying to use a cut that is not expressed in PPM
59+
* @param cut The cut
60+
*/
61+
error GraphPaymentsInvalidCut(uint256 cut);
62+
5463
/**
5564
* @notice Collects funds from a payer.
5665
* It will pay cuts to all relevant parties and forward the rest to the receiver.
5766
* @param paymentType The type of payment as defined in {IGraphPayments}
5867
* @param receiver The address of the receiver
5968
* @param tokens The amount of tokens being collected
6069
* @param dataService The address of the data service
61-
* @param tokensDataService The amount of tokens that should be sent to the data service
70+
* @param dataServiceCut The data service cut in PPM
6271
*/
6372
function collect(
6473
PaymentTypes paymentType,
6574
address receiver,
6675
uint256 tokens,
6776
address dataService,
68-
uint256 tokensDataService
77+
uint256 dataServiceCut
6978
) external;
7079
}

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

+2-4
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,15 @@ interface IPaymentsCollector {
1919
* @param paymentType The payment type collected as defined by {IGraphPayments}
2020
* @param payer The address of the payer
2121
* @param receiver The address of the receiver
22-
* @param tokensReceiver The amount of tokens received by the receiver
2322
* @param dataService The address of the data service
24-
* @param tokensDataService The amount of tokens received by the data service
23+
* @param tokens The amount of tokens being collected
2524
*/
2625
event PaymentCollected(
2726
IGraphPayments.PaymentTypes indexed paymentType,
2827
address indexed payer,
2928
address receiver,
30-
uint256 tokensReceiver,
3129
address indexed dataService,
32-
uint256 tokensDataService
30+
uint256 tokens
3331
);
3432

3533
/**

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,15 @@ interface IPaymentsEscrow {
186186
* @param receiver The address of the receiver
187187
* @param tokens The amount of tokens to collect
188188
* @param dataService The address of the data service
189-
* @param tokensDataService The amount of tokens that {GraphPayments} should send to the data service
189+
* @param dataServiceCut The data service cut in PPM that {GraphPayments} should send
190190
*/
191191
function collect(
192192
IGraphPayments.PaymentTypes paymentType,
193193
address payer,
194194
address receiver,
195195
uint256 tokens,
196196
address dataService,
197-
uint256 tokensDataService
197+
uint256 dataServiceCut
198198
) external;
199199

200200
/**

‎packages/horizon/contracts/payments/GraphPayments.sol

+31-18
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I
3232
* @param protocolPaymentCut The protocol tax in PPM
3333
*/
3434
constructor(address controller, uint256 protocolPaymentCut) GraphDirectory(controller) {
35-
require(PPMMath.isValidPPM(protocolPaymentCut), GraphPaymentsInvalidProtocolPaymentCut(protocolPaymentCut));
35+
require(PPMMath.isValidPPM(protocolPaymentCut), GraphPaymentsInvalidCut(protocolPaymentCut));
3636
PROTOCOL_PAYMENT_CUT = protocolPaymentCut;
3737
_disableInitializers();
3838
}
@@ -52,41 +52,54 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I
5252
address receiver,
5353
uint256 tokens,
5454
address dataService,
55-
uint256 tokensDataService
55+
uint256 dataServiceCut
5656
) external {
57+
require(PPMMath.isValidPPM(dataServiceCut), GraphPaymentsInvalidCut(dataServiceCut));
58+
59+
// Pull tokens from the sender
5760
_graphToken().pullTokens(msg.sender, tokens);
5861

59-
// Calculate cuts
60-
uint256 tokensProtocol = tokens.mulPPM(PROTOCOL_PAYMENT_CUT);
61-
uint256 delegationFeeCut = _graphStaking().getDelegationFeeCut(receiver, dataService, paymentType);
62-
uint256 tokensDelegationPool = tokens.mulPPM(delegationFeeCut);
63-
uint256 totalCut = tokensProtocol + tokensDataService + tokensDelegationPool;
64-
require(tokens >= totalCut, GraphPaymentsInsufficientTokens(tokens, totalCut));
62+
// Calculate token amounts for each party
63+
// Order matters: protocol -> data service -> delegators -> receiver
64+
// Note the substractions should not underflow as we are only deducting a percentage of the remainder
65+
uint256 tokensRemaining = tokens;
66+
67+
uint256 tokensProtocol = tokensRemaining.mulPPMRoundUp(PROTOCOL_PAYMENT_CUT);
68+
tokensRemaining = tokensRemaining - tokensProtocol;
69+
70+
uint256 tokensDataService = tokensRemaining.mulPPMRoundUp(dataServiceCut);
71+
tokensRemaining = tokensRemaining - tokensDataService;
72+
73+
uint256 tokensDelegationPool = tokensRemaining.mulPPMRoundUp(
74+
_graphStaking().getDelegationFeeCut(receiver, dataService, paymentType)
75+
);
76+
tokensRemaining = tokensRemaining - tokensDelegationPool;
77+
78+
// Ensure accounting is correct
79+
uint256 tokensTotal = tokensProtocol + tokensDataService + tokensDelegationPool + tokensRemaining;
80+
require(tokens == tokensTotal, GraphPaymentsBadAccounting(tokens, tokensTotal));
6581

66-
// Pay protocol cut
82+
// Pay all parties
6783
_graphToken().burnTokens(tokensProtocol);
6884

69-
// Pay data service cut
7085
_graphToken().pushTokens(dataService, tokensDataService);
7186

72-
// Pay delegators
7387
if (tokensDelegationPool > 0) {
7488
_graphToken().approve(address(_graphStaking()), tokensDelegationPool);
7589
_graphStaking().addToDelegationPool(receiver, dataService, tokensDelegationPool);
7690
}
7791

78-
// Pay receiver
79-
uint256 tokensReceiverRemaining = tokens - totalCut;
80-
_graphToken().pushTokens(receiver, tokensReceiverRemaining);
92+
_graphToken().pushTokens(receiver, tokensRemaining);
8193

82-
emit PaymentCollected(
94+
emit GraphPaymentCollected(
8395
msg.sender,
8496
receiver,
8597
dataService,
86-
tokensReceiverRemaining,
87-
tokensDelegationPool,
98+
tokens,
99+
tokensProtocol,
88100
tokensDataService,
89-
tokensProtocol
101+
tokensDelegationPool,
102+
tokensRemaining
90103
);
91104
}
92105
}

‎packages/horizon/contracts/payments/PaymentsEscrow.sol

+2-2
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory,
129129
address receiver,
130130
uint256 tokens,
131131
address dataService,
132-
uint256 tokensDataService
132+
uint256 dataServiceCut
133133
) external override notPaused {
134134
// Check if there are enough funds in the escrow account
135135
EscrowAccount storage account = escrowAccounts[payer][msg.sender][receiver];
@@ -141,7 +141,7 @@ contract PaymentsEscrow is Initializable, MulticallUpgradeable, GraphDirectory,
141141
uint256 balanceBefore = _graphToken().balanceOf(address(this));
142142

143143
_graphToken().approve(address(_graphPayments()), tokens);
144-
_graphPayments().collect(paymentType, receiver, tokens, dataService, tokensDataService);
144+
_graphPayments().collect(paymentType, receiver, tokens, dataService, dataServiceCut);
145145

146146
uint256 balanceAfter = _graphToken().balanceOf(address(this));
147147
require(

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

+2-11
Original file line numberDiff line numberDiff line change
@@ -214,21 +214,12 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector {
214214
}
215215
}
216216

217-
uint256 tokensDataService = tokensToCollect.mulPPM(dataServiceCut);
218-
219217
if (tokensToCollect > 0) {
220218
tokensCollected[dataService][receiver][payer] += tokensToCollect;
221-
_graphPaymentsEscrow().collect(
222-
_paymentType,
223-
payer,
224-
receiver,
225-
tokensToCollect,
226-
dataService,
227-
tokensDataService
228-
);
219+
_graphPaymentsEscrow().collect(_paymentType, payer, receiver, tokensToCollect, dataService, dataServiceCut);
229220
}
230221

231-
emit PaymentCollected(_paymentType, payer, receiver, tokensToCollect, dataService, tokensDataService);
222+
emit PaymentCollected(_paymentType, payer, receiver, dataService, tokensToCollect);
232223
emit RAVCollected(
233224
payer,
234225
dataService,

‎packages/horizon/test/escrow/GraphEscrow.t.sol

+33-20
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol";
77

88
import { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStakingShared.t.sol";
99
import { PaymentsEscrowSharedTest } from "../shared/payments-escrow/PaymentsEscrowShared.t.sol";
10+
import { PPMMath } from "../../contracts/libraries/PPMMath.sol";
1011

1112
contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
13+
using PPMMath for uint256;
14+
1215
/*
1316
* MODIFIERS
1417
*/
@@ -59,6 +62,7 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
5962
uint256 receiverBalance;
6063
uint256 delegationPoolBalance;
6164
uint256 dataServiceBalance;
65+
uint256 payerEscrowBalance;
6266
}
6367

6468
function _collectEscrow(
@@ -67,53 +71,62 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
6771
address _receiver,
6872
uint256 _tokens,
6973
address _dataService,
70-
uint256 _tokensDataService
74+
uint256 _dataServiceCut
7175
) internal {
7276
(, address _collector, ) = vm.readCallers();
7377

7478
// Previous balances
75-
(uint256 previousPayerEscrowBalance, , ) = escrow.escrowAccounts(_payer, _collector, _receiver);
7679
CollectPaymentData memory previousBalances = CollectPaymentData({
7780
escrowBalance: token.balanceOf(address(escrow)),
7881
paymentsBalance: token.balanceOf(address(payments)),
7982
receiverBalance: token.balanceOf(_receiver),
8083
delegationPoolBalance: staking.getDelegatedTokensAvailable(_receiver, _dataService),
81-
dataServiceBalance: token.balanceOf(_dataService)
84+
dataServiceBalance: token.balanceOf(_dataService),
85+
payerEscrowBalance: 0
8286
});
8387

88+
{
89+
(uint256 payerEscrowBalance, , ) = escrow.escrowAccounts(_payer, _collector, _receiver);
90+
previousBalances.payerEscrowBalance = payerEscrowBalance;
91+
}
92+
8493
vm.expectEmit(address(escrow));
8594
emit IPaymentsEscrow.EscrowCollected(_payer, _collector, _receiver, _tokens);
86-
escrow.collect(_paymentType, _payer, _receiver, _tokens, _dataService, _tokensDataService);
95+
escrow.collect(_paymentType, _payer, _receiver, _tokens, _dataService, _dataServiceCut);
8796

8897
// Calculate cuts
89-
uint256 protocolPaymentCut = payments.PROTOCOL_PAYMENT_CUT();
90-
uint256 delegatorCut = staking.getDelegationFeeCut(_receiver, _dataService, _paymentType);
98+
// this is nasty but stack is indeed too deep
99+
uint256 tokensDataService = (_tokens - _tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT())).mulPPMRoundUp(
100+
_dataServiceCut
101+
);
102+
uint256 tokensDelegation = (_tokens -
103+
_tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()) -
104+
tokensDataService).mulPPMRoundUp(staking.getDelegationFeeCut(_receiver, _dataService, _paymentType));
105+
uint256 receiverExpectedPayment = _tokens -
106+
_tokens.mulPPMRoundUp(payments.PROTOCOL_PAYMENT_CUT()) -
107+
tokensDataService -
108+
tokensDelegation;
91109

92110
// After balances
93-
(uint256 afterPayerEscrowBalance, , ) = escrow.escrowAccounts(_payer, _collector, _receiver);
94111
CollectPaymentData memory afterBalances = CollectPaymentData({
95112
escrowBalance: token.balanceOf(address(escrow)),
96113
paymentsBalance: token.balanceOf(address(payments)),
97114
receiverBalance: token.balanceOf(_receiver),
98115
delegationPoolBalance: staking.getDelegatedTokensAvailable(_receiver, _dataService),
99-
dataServiceBalance: token.balanceOf(_dataService)
116+
dataServiceBalance: token.balanceOf(_dataService),
117+
payerEscrowBalance: 0
100118
});
119+
{
120+
(uint256 afterPayerEscrowBalance, , ) = escrow.escrowAccounts(_payer, _collector, _receiver);
121+
afterBalances.payerEscrowBalance = afterPayerEscrowBalance;
122+
}
101123

102124
// Check receiver balance after payment
103-
uint256 receiverExpectedPayment = _tokens -
104-
_tokensDataService -
105-
(_tokens * protocolPaymentCut) /
106-
MAX_PPM -
107-
(_tokens * delegatorCut) /
108-
MAX_PPM;
109125
assertEq(afterBalances.receiverBalance - previousBalances.receiverBalance, receiverExpectedPayment);
110126
assertEq(token.balanceOf(address(payments)), 0);
111127

112128
// Check delegation pool balance after payment
113-
assertEq(
114-
afterBalances.delegationPoolBalance - previousBalances.delegationPoolBalance,
115-
(_tokens * delegatorCut) / MAX_PPM
116-
);
129+
assertEq(afterBalances.delegationPoolBalance - previousBalances.delegationPoolBalance, tokensDelegation);
117130

118131
// Check that the escrow account has been updated
119132
assertEq(previousBalances.escrowBalance, afterBalances.escrowBalance + _tokens);
@@ -122,9 +135,9 @@ contract GraphEscrowTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
122135
assertEq(previousBalances.paymentsBalance, afterBalances.paymentsBalance);
123136

124137
// Check data service balance after payment
125-
assertEq(afterBalances.dataServiceBalance - previousBalances.dataServiceBalance, _tokensDataService);
138+
assertEq(afterBalances.dataServiceBalance - previousBalances.dataServiceBalance, tokensDataService);
126139

127140
// Check payers escrow balance after payment
128-
assertEq(previousPayerEscrowBalance - _tokens, afterPayerEscrowBalance);
141+
assertEq(previousBalances.payerEscrowBalance - _tokens, afterBalances.payerEscrowBalance);
129142
}
130143
}

‎packages/horizon/test/escrow/collect.t.sol

+4-6
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,16 @@ contract GraphEscrowCollectTest is GraphEscrowTest {
1616
function testCollect_Tokens(
1717
uint256 tokens,
1818
uint256 delegationTokens,
19-
uint256 tokensDataService
19+
uint256 dataServiceCut
2020
)
2121
public
2222
useIndexer
2323
useProvision(tokens, 0, 0)
2424
useDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, delegationFeeCut)
2525
{
26-
uint256 tokensProtocol = (tokens * protocolPaymentCut) / MAX_PPM;
27-
uint256 tokensDelegatoion = (tokens * delegationFeeCut) / MAX_PPM;
28-
vm.assume(tokensDataService < tokens - tokensProtocol - tokensDelegatoion);
29-
26+
dataServiceCut = bound(dataServiceCut, 0, MAX_PPM);
3027
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
28+
3129
resetPrank(users.delegator);
3230
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);
3331

@@ -41,7 +39,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest {
4139
users.indexer,
4240
tokens,
4341
subgraphDataServiceAddress,
44-
tokensDataService
42+
dataServiceCut
4543
);
4644
}
4745

0 commit comments

Comments
 (0)