Skip to content

Commit 76865a4

Browse files
tmigonetmigone
committed
fix(c4-117-g05): require()/revert() strings longer than 32 bytes cost extra gas
Signed-off-by: Tomás Migone <[email protected]>
1 parent 2c4ac9d commit 76865a4

15 files changed

+57
-57
lines changed

‎contracts/gateway/GraphTokenGateway.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok
2121
modifier onlyGovernorOrGuardian() {
2222
require(
2323
msg.sender == controller.getGovernor() || msg.sender == pauseGuardian,
24-
"Only Governor or Guardian can call"
24+
"Only Governor or Guardian"
2525
);
2626
_;
2727
}

‎contracts/governance/Managed.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ contract Managed {
5050
}
5151

5252
function _onlyGovernor() internal view {
53-
require(msg.sender == controller.getGovernor(), "Caller must be Controller governor");
53+
require(msg.sender == controller.getGovernor(), "Only Controller governor");
5454
}
5555

5656
function _onlyController() internal view {

‎contracts/upgrades/GraphProxy.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ contract GraphProxy is GraphProxyStorage {
102102
* NOTE: Only the admin can call this function.
103103
*/
104104
function setAdmin(address _newAdmin) external ifAdmin {
105-
require(_newAdmin != address(0), "Cannot change the admin of a proxy to the zero address");
105+
require(_newAdmin != address(0), "Admin cant be the zero address");
106106
_setAdmin(_newAdmin);
107107
}
108108

@@ -138,10 +138,10 @@ contract GraphProxy is GraphProxyStorage {
138138
*/
139139
function _acceptUpgrade() internal {
140140
address _pendingImplementation = _pendingImplementation();
141-
require(Address.isContract(_pendingImplementation), "Implementation must be a contract");
141+
require(Address.isContract(_pendingImplementation), "Impl must be a contract");
142142
require(
143143
_pendingImplementation != address(0) && msg.sender == _pendingImplementation,
144-
"Caller must be the pending implementation"
144+
"Only pending implementation"
145145
);
146146

147147
_setImplementation(_pendingImplementation);

‎contracts/upgrades/GraphUpgradeable.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ contract GraphUpgradeable {
2929
* @dev Check if the caller is the implementation.
3030
*/
3131
modifier onlyImpl() {
32-
require(msg.sender == _implementation(), "Caller must be the implementation");
32+
require(msg.sender == _implementation(), "Only implementation");
3333
_;
3434
}
3535

‎e2e/deployment/config/l1/l1GraphTokenGateway.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,38 +34,38 @@ describe('[L1] L1GraphTokenGateway configuration', function () {
3434
unauthorized.address,
3535
unauthorized.address,
3636
)
37-
await expect(tx).revertedWith('Caller must be Controller governor')
37+
await expect(tx).revertedWith('Only Controller governor')
3838
})
3939

4040
it('setL2TokenAddress should revert', async function () {
4141
const tx = L1GraphTokenGateway.connect(unauthorized).setL2TokenAddress(unauthorized.address)
42-
await expect(tx).revertedWith('Caller must be Controller governor')
42+
await expect(tx).revertedWith('Only Controller governor')
4343
})
4444

4545
it('setL2CounterpartAddress should revert', async function () {
4646
const tx = L1GraphTokenGateway.connect(unauthorized).setL2CounterpartAddress(
4747
unauthorized.address,
4848
)
49-
await expect(tx).revertedWith('Caller must be Controller governor')
49+
await expect(tx).revertedWith('Only Controller governor')
5050
})
5151

5252
it('setEscrowAddress should revert', async function () {
5353
const tx = L1GraphTokenGateway.connect(unauthorized).setEscrowAddress(unauthorized.address)
54-
await expect(tx).revertedWith('Caller must be Controller governor')
54+
await expect(tx).revertedWith('Only Controller governor')
5555
})
5656

5757
it('addToCallhookWhitelist should revert', async function () {
5858
const tx = L1GraphTokenGateway.connect(unauthorized).addToCallhookWhitelist(
5959
unauthorized.address,
6060
)
61-
await expect(tx).revertedWith('Caller must be Controller governor')
61+
await expect(tx).revertedWith('Only Controller governor')
6262
})
6363

6464
it('removeFromCallhookWhitelist should revert', async function () {
6565
const tx = L1GraphTokenGateway.connect(unauthorized).removeFromCallhookWhitelist(
6666
unauthorized.address,
6767
)
68-
await expect(tx).revertedWith('Caller must be Controller governor')
68+
await expect(tx).revertedWith('Only Controller governor')
6969
})
7070

7171
it('finalizeInboundTransfer should revert', async function () {

‎e2e/deployment/config/l2/l2GraphTokenGateway.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,19 @@ describe('[L2] L2GraphTokenGateway configuration', function () {
3131

3232
it('setL2Router should revert', async function () {
3333
const tx = L2GraphTokenGateway.connect(unauthorized).setL2Router(unauthorized.address)
34-
await expect(tx).revertedWith('Caller must be Controller governor')
34+
await expect(tx).revertedWith('Only Controller governor')
3535
})
3636

3737
it('setL1TokenAddress should revert', async function () {
3838
const tx = L2GraphTokenGateway.connect(unauthorized).setL1TokenAddress(unauthorized.address)
39-
await expect(tx).revertedWith('Caller must be Controller governor')
39+
await expect(tx).revertedWith('Only Controller governor')
4040
})
4141

4242
it('setL1CounterpartAddress should revert', async function () {
4343
const tx = L2GraphTokenGateway.connect(unauthorized).setL1CounterpartAddress(
4444
unauthorized.address,
4545
)
46-
await expect(tx).revertedWith('Caller must be Controller governor')
46+
await expect(tx).revertedWith('Only Controller governor')
4747
})
4848

4949
it('finalizeInboundTransfer should revert', async function () {

‎test/curation/configuration.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('Curation:Config', () => {
5555

5656
it('reject set `defaultReserveRatio` if not allowed', async function () {
5757
const tx = curation.connect(me.signer).setDefaultReserveRatio(defaults.curation.reserveRatio)
58-
await expect(tx).revertedWith('Caller must be Controller governor')
58+
await expect(tx).revertedWith('Only Controller governor')
5959
})
6060
})
6161

@@ -79,7 +79,7 @@ describe('Curation:Config', () => {
7979
const tx = curation
8080
.connect(me.signer)
8181
.setMinimumCurationDeposit(defaults.curation.minimumCurationDeposit)
82-
await expect(tx).revertedWith('Caller must be Controller governor')
82+
await expect(tx).revertedWith('Only Controller governor')
8383
})
8484
})
8585

@@ -99,7 +99,7 @@ describe('Curation:Config', () => {
9999

100100
it('reject set `curationTaxPercentage` if not allowed', async function () {
101101
const tx = curation.connect(me.signer).setCurationTaxPercentage(0)
102-
await expect(tx).revertedWith('Caller must be Controller governor')
102+
await expect(tx).revertedWith('Only Controller governor')
103103
})
104104
})
105105

@@ -124,7 +124,7 @@ describe('Curation:Config', () => {
124124
it('reject set `curationTokenMaster` if not allowed', async function () {
125125
const newCurationTokenMaster = curation.address
126126
const tx = curation.connect(me.signer).setCurationTokenMaster(newCurationTokenMaster)
127-
await expect(tx).revertedWith('Caller must be Controller governor')
127+
await expect(tx).revertedWith('Only Controller governor')
128128
})
129129
})
130130
})

‎test/disputes/configuration.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ describe('DisputeManager:Config', () => {
4949

5050
it('reject set `arbitrator` if not allowed', async function () {
5151
const tx = disputeManager.connect(me.signer).setArbitrator(arbitrator.address)
52-
await expect(tx).revertedWith('Caller must be Controller governor')
52+
await expect(tx).revertedWith('Only Controller governor')
5353
})
5454

5555
it('reject set `arbitrator` to address zero', async function () {
@@ -74,7 +74,7 @@ describe('DisputeManager:Config', () => {
7474
it('reject set `minimumDeposit` if not allowed', async function () {
7575
const newValue = toBN('1')
7676
const tx = disputeManager.connect(me.signer).setMinimumDeposit(newValue)
77-
await expect(tx).revertedWith('Caller must be Controller governor')
77+
await expect(tx).revertedWith('Only Controller governor')
7878
})
7979
})
8080

@@ -97,7 +97,7 @@ describe('DisputeManager:Config', () => {
9797

9898
it('reject set `fishermanRewardPercentage` if not allowed', async function () {
9999
const tx = disputeManager.connect(me.signer).setFishermanRewardPercentage(50)
100-
await expect(tx).revertedWith('Caller must be Controller governor')
100+
await expect(tx).revertedWith('Only Controller governor')
101101
})
102102
})
103103

@@ -127,7 +127,7 @@ describe('DisputeManager:Config', () => {
127127

128128
it('reject set `slashingPercentage` if not allowed', async function () {
129129
const tx = disputeManager.connect(me.signer).setSlashingPercentage(50, 50)
130-
await expect(tx).revertedWith('Caller must be Controller governor')
130+
await expect(tx).revertedWith('Only Controller governor')
131131
})
132132
})
133133
})

‎test/gateway/bridgeEscrow.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ describe('BridgeEscrow', () => {
4141
describe('approveAll', function () {
4242
it('cannot be called by someone other than the governor', async function () {
4343
const tx = bridgeEscrow.connect(tokenReceiver.signer).approveAll(spender.address)
44-
await expect(tx).revertedWith('Caller must be Controller governor')
44+
await expect(tx).revertedWith('Only Controller governor')
4545
})
4646
it('allows a spender to transfer GRT held by the contract', async function () {
4747
expect(await grt.allowance(bridgeEscrow.address, spender.address)).eq(0)
@@ -62,7 +62,7 @@ describe('BridgeEscrow', () => {
6262
describe('revokeAll', function () {
6363
it('cannot be called by someone other than the governor', async function () {
6464
const tx = bridgeEscrow.connect(tokenReceiver.signer).revokeAll(spender.address)
65-
await expect(tx).revertedWith('Caller must be Controller governor')
65+
await expect(tx).revertedWith('Only Controller governor')
6666
})
6767
it("revokes a spender's permission to transfer GRT held by the contract", async function () {
6868
await bridgeEscrow.connect(governor.signer).approveAll(spender.address)

‎test/gateway/l1GraphTokenGateway.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ describe('L1GraphTokenGateway', () => {
133133
const tx = l1GraphTokenGateway
134134
.connect(tokenSender.signer)
135135
.setArbitrumAddresses(inboxMock.address, mockRouter.address)
136-
await expect(tx).revertedWith('Caller must be Controller governor')
136+
await expect(tx).revertedWith('Only Controller governor')
137137
})
138138
it('rejects setting an EOA as router or inbox', async function () {
139139
let tx = l1GraphTokenGateway
@@ -162,7 +162,7 @@ describe('L1GraphTokenGateway', () => {
162162
const tx = l1GraphTokenGateway
163163
.connect(tokenSender.signer)
164164
.setL2TokenAddress(mockL2GRT.address)
165-
await expect(tx).revertedWith('Caller must be Controller governor')
165+
await expect(tx).revertedWith('Only Controller governor')
166166
})
167167
it('sets l2GRT', async function () {
168168
const tx = l1GraphTokenGateway.connect(governor.signer).setL2TokenAddress(mockL2GRT.address)
@@ -176,7 +176,7 @@ describe('L1GraphTokenGateway', () => {
176176
const tx = l1GraphTokenGateway
177177
.connect(tokenSender.signer)
178178
.setL2CounterpartAddress(mockL2Gateway.address)
179-
await expect(tx).revertedWith('Caller must be Controller governor')
179+
await expect(tx).revertedWith('Only Controller governor')
180180
})
181181
it('sets L2Counterpart', async function () {
182182
const tx = l1GraphTokenGateway
@@ -193,7 +193,7 @@ describe('L1GraphTokenGateway', () => {
193193
const tx = l1GraphTokenGateway
194194
.connect(tokenSender.signer)
195195
.setEscrowAddress(bridgeEscrow.address)
196-
await expect(tx).revertedWith('Caller must be Controller governor')
196+
await expect(tx).revertedWith('Only Controller governor')
197197
})
198198
it('sets escrow', async function () {
199199
const tx = l1GraphTokenGateway
@@ -210,7 +210,7 @@ describe('L1GraphTokenGateway', () => {
210210
const tx = l1GraphTokenGateway
211211
.connect(tokenSender.signer)
212212
.addToCallhookWhitelist(fixtureContracts.rewardsManager.address)
213-
await expect(tx).revertedWith('Caller must be Controller governor')
213+
await expect(tx).revertedWith('Only Controller governor')
214214
expect(
215215
await l1GraphTokenGateway.callhookWhitelist(fixtureContracts.rewardsManager.address),
216216
).eq(false)
@@ -241,7 +241,7 @@ describe('L1GraphTokenGateway', () => {
241241
const tx = l1GraphTokenGateway
242242
.connect(tokenSender.signer)
243243
.removeFromCallhookWhitelist(fixtureContracts.rewardsManager.address)
244-
await expect(tx).revertedWith('Caller must be Controller governor')
244+
await expect(tx).revertedWith('Only Controller governor')
245245
expect(
246246
await l1GraphTokenGateway.callhookWhitelist(fixtureContracts.rewardsManager.address),
247247
).eq(true)
@@ -264,9 +264,9 @@ describe('L1GraphTokenGateway', () => {
264264
describe('Pausable behavior', () => {
265265
it('cannot be paused or unpaused by someone other than governor or pauseGuardian', async () => {
266266
let tx = l1GraphTokenGateway.connect(tokenSender.signer).setPaused(false)
267-
await expect(tx).revertedWith('Only Governor or Guardian can call')
267+
await expect(tx).revertedWith('Only Governor or Guardian')
268268
tx = l1GraphTokenGateway.connect(tokenSender.signer).setPaused(true)
269-
await expect(tx).revertedWith('Only Governor or Guardian can call')
269+
await expect(tx).revertedWith('Only Governor or Guardian')
270270
})
271271
it('cannot be unpaused if some state variables are not set', async function () {
272272
let tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
@@ -303,7 +303,7 @@ describe('L1GraphTokenGateway', () => {
303303
const tx = l1GraphTokenGateway
304304
.connect(tokenSender.signer)
305305
.setPauseGuardian(pauseGuardian.address)
306-
await expect(tx).revertedWith('Caller must be Controller governor')
306+
await expect(tx).revertedWith('Only Controller governor')
307307
})
308308
it('sets a new pause guardian', async function () {
309309
const tx = l1GraphTokenGateway

0 commit comments

Comments
 (0)