Skip to content

Commit 1b7d13a

Browse files
matiasedgeandnodematiasedgeandnode
committed
fixup: remove getAuthorization
1 parent d99d5cf commit 1b7d13a

File tree

6 files changed

+45
-41
lines changed

6 files changed

+45
-41
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ interface IAuthorizable {
145145
/**
146146
* @notice Returns details of the authorization
147147
*/
148-
function getAuthorization(address signer) external view returns (Authorization memory auth);
148+
function getThawEnd(address signer) external view returns (uint256);
149149

150150
/**
151151
* @notice Returns true if the signer is authorized by the authorizer

‎packages/horizon/contracts/utilities/Authorizable.sol‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ contract Authorizable is IAuthorizable {
2525
* @dev Revert if the caller has not authorized the signer
2626
*/
2727
modifier onlyAuthorized(address _signer) {
28-
require(_isAuthorized(msg.sender, _signer), SignerNotAuthorized(msg.sender, _signer));
28+
_requireAuthorized(msg.sender, _signer);
2929
_;
3030
}
3131

@@ -77,13 +77,6 @@ contract Authorizable is IAuthorizable {
7777
return REVOKE_AUTHORIZATION_THAWING_PERIOD;
7878
}
7979

80-
/**
81-
* See {IAuthorizable.getAuthorization}.
82-
*/
83-
function getAuthorization(address signer) external view returns (Authorization memory auth) {
84-
return authorizations[signer];
85-
}
86-
8780
/**
8881
* See {IAuthorizable.isAuthorized}.
8982
*/
@@ -92,7 +85,22 @@ contract Authorizable is IAuthorizable {
9285
}
9386

9487
function _isAuthorized(address _authorizer, address _signer) internal view returns (bool) {
95-
return (authorizations[_signer].authorizer == _authorizer && !authorizations[_signer].revoked);
88+
return (_authorizer != address(0) &&
89+
authorizations[_signer].authorizer == _authorizer &&
90+
!authorizations[_signer].revoked);
91+
}
92+
93+
function _requireAuthorized(address _authorizer, address _signer) internal view {
94+
require(_isAuthorized(_authorizer, _signer), SignerNotAuthorized(_authorizer, _signer));
95+
}
96+
97+
/**
98+
* See {IAuthorizable.getThawEnd}.
99+
*/
100+
function getThawEnd(address _signer) external view returns (uint256) {
101+
_requireAuthorized(authorizations[_signer].authorizer, _signer);
102+
103+
return authorizations[_signer].thawEndTimestamp;
96104
}
97105

98106
/**

‎packages/horizon/test/payments/graph-tally-collector/GraphTallyCollector.t.sol‎

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,8 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
7474
emit IAuthorizable.SignerAuthorized(msgSender, _signer);
7575

7676
graphTallyCollector.authorizeSigner(_signer, _proofDeadline, _proof);
77-
78-
IAuthorizable.Authorization memory auth = graphTallyCollector.getAuthorization(_signer);
79-
assertEq(auth.authorizer, msgSender);
80-
assertEq(auth.thawEndTimestamp, 0);
81-
assertEq(auth.revoked, false);
77+
assertTrue(graphTallyCollector.isAuthorized(msgSender, _signer));
78+
assertEq(graphTallyCollector.getThawEnd(_signer), 0);
8279
}
8380

8481
function _thawSigner(address _signer) internal {
@@ -90,10 +87,8 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
9087

9188
graphTallyCollector.thawSigner(_signer);
9289

93-
IAuthorizable.Authorization memory auth = graphTallyCollector.getAuthorization(_signer);
94-
assertEq(auth.authorizer, msgSender);
95-
assertEq(auth.thawEndTimestamp, expectedThawEndTimestamp);
96-
assertEq(auth.revoked, false);
90+
assertTrue(graphTallyCollector.isAuthorized(msgSender, _signer));
91+
assertEq(graphTallyCollector.getThawEnd(_signer), expectedThawEndTimestamp);
9792
}
9893

9994
function _cancelThawSigner(address _signer) internal {
@@ -104,27 +99,22 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
10499

105100
graphTallyCollector.cancelThawSigner(_signer);
106101

107-
IAuthorizable.Authorization memory auth = graphTallyCollector.getAuthorization(_signer);
108-
assertEq(auth.authorizer, msgSender);
109-
assertEq(auth.thawEndTimestamp, 0);
110-
assertEq(auth.revoked, false);
102+
assertTrue(graphTallyCollector.isAuthorized(msgSender, _signer));
103+
assertEq(graphTallyCollector.getThawEnd(_signer), 0);
111104
}
112105

113106
function _revokeAuthorizedSigner(address _signer) internal {
114107
(, address msgSender, ) = vm.readCallers();
115108

116-
IAuthorizable.Authorization memory beforeAuth = graphTallyCollector.getAuthorization(_signer);
109+
assertTrue(graphTallyCollector.isAuthorized(msgSender, _signer));
110+
assertLt(graphTallyCollector.getThawEnd(_signer), block.timestamp);
117111

118112
vm.expectEmit(address(graphTallyCollector));
119113
emit IAuthorizable.SignerRevoked(msgSender, _signer);
120114

121115
graphTallyCollector.revokeAuthorizedSigner(_signer);
122116

123-
IAuthorizable.Authorization memory afterAuth = graphTallyCollector.getAuthorization(_signer);
124-
125-
assertEq(beforeAuth.authorizer, afterAuth.authorizer);
126-
assertEq(beforeAuth.thawEndTimestamp, afterAuth.thawEndTimestamp);
127-
assertEq(afterAuth.revoked, true);
117+
assertFalse(graphTallyCollector.isAuthorized(msgSender, _signer));
128118
}
129119

130120
function _collect(IGraphPayments.PaymentTypes _paymentType, bytes memory _data) internal {
@@ -144,14 +134,11 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
144134
_data,
145135
(IGraphTallyCollector.SignedRAV, uint256)
146136
);
147-
bytes32 messageHash = graphTallyCollector.encodeRAV(signedRAV.rav);
148-
address _signer = ECDSA.recover(messageHash, signedRAV.signature);
149-
IAuthorizable.Authorization memory auth = graphTallyCollector.getAuthorization(_signer);
150137
uint256 tokensAlreadyCollected = graphTallyCollector.tokensCollected(
151138
signedRAV.rav.dataService,
152139
signedRAV.rav.collectionId,
153140
signedRAV.rav.serviceProvider,
154-
auth.authorizer
141+
signedRAV.rav.payer
155142
);
156143
uint256 tokensToCollect = _tokensToCollect == 0
157144
? signedRAV.rav.valueAggregate - tokensAlreadyCollected
@@ -161,15 +148,15 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
161148
emit IPaymentsCollector.PaymentCollected(
162149
_paymentType,
163150
signedRAV.rav.collectionId,
164-
auth.authorizer,
151+
signedRAV.rav.payer,
165152
signedRAV.rav.serviceProvider,
166153
signedRAV.rav.dataService,
167154
tokensToCollect
168155
);
169156
vm.expectEmit(address(graphTallyCollector));
170157
emit IGraphTallyCollector.RAVCollected(
171158
signedRAV.rav.collectionId,
172-
auth.authorizer,
159+
signedRAV.rav.payer,
173160
signedRAV.rav.serviceProvider,
174161
signedRAV.rav.dataService,
175162
signedRAV.rav.timestampNs,
@@ -185,7 +172,7 @@ contract GraphTallyTest is HorizonStakingSharedTest, PaymentsEscrowSharedTest {
185172
signedRAV.rav.dataService,
186173
signedRAV.rav.collectionId,
187174
signedRAV.rav.serviceProvider,
188-
auth.authorizer
175+
signedRAV.rav.payer
189176
);
190177
assertEq(tokensCollected, tokensToCollect);
191178
assertEq(

‎packages/horizon/test/payments/graph-tally-collector/signer/authorizeSigner.t.sol‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ contract GraphTallyAuthorizeSignerTest is GraphTallyTest {
5353
uint256 proofDeadline = block.timestamp + 1;
5454
bytes memory signerProof = _getSignerProof(proofDeadline, signerPrivateKey);
5555
_authorizeSigner(signer, proofDeadline, signerProof);
56-
5756
// Revoke signer
5857
_thawSigner(signer);
5958
skip(revokeSignerThawingPeriod + 1);

‎packages/horizon/test/payments/graph-tally-collector/signer/thawSigner.t.sol‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ contract GraphTallyThawSignerTest is GraphTallyTest {
4242

4343
function testGraphTally_ThawSigner_AlreadyThawing() public useGateway useSigner {
4444
_thawSigner(signer);
45-
IAuthorizable.Authorization memory originalAuth = graphTallyCollector.getAuthorization(signer);
45+
uint256 originalThawEnd = graphTallyCollector.getThawEnd(signer);
4646
skip(1);
4747

4848
graphTallyCollector.thawSigner(signer);
49-
IAuthorizable.Authorization memory currentAuth = graphTallyCollector.getAuthorization(signer);
50-
vm.assertEq(originalAuth.thawEndTimestamp, block.timestamp + revokeSignerThawingPeriod - 1);
51-
vm.assertEq(currentAuth.thawEndTimestamp, block.timestamp + revokeSignerThawingPeriod);
49+
uint256 currentThawEnd = graphTallyCollector.getThawEnd(signer);
50+
vm.assertEq(originalThawEnd, block.timestamp + revokeSignerThawingPeriod - 1);
51+
vm.assertEq(currentThawEnd, block.timestamp + revokeSignerThawingPeriod);
5252
}
5353
}

‎packages/horizon/test/utilities/Authorizable.t.sol‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,16 @@ contract AuthorizableTest is Test, Bounder {
310310
vm.prank(_authorizer);
311311
authorizable.revokeAuthorizedSigner(signer);
312312
}
313+
314+
function test_IsAuthorized_Revert_WhenZero(uint256 _unboundedKey, address _authorizer) public {
315+
vm.assume(_authorizer != address(0));
316+
(uint256 signerKey, address signer) = boundAddrAndKey(_unboundedKey);
317+
318+
authHelper.authorizeSignerWithChecks(_authorizer, signerKey);
319+
320+
vm.prank(_authorizer);
321+
assertNotAuthorized(address(0), signer);
322+
}
313323
}
314324

315325
contract AuthorizableHelper is Test {

0 commit comments

Comments
 (0)