Skip to content

Commit 161e995

Browse files
ItsNickBarryItsNickBarry
committed
replace AddressUtils string errors with custom errors
1 parent 07bc54f commit 161e995

File tree

6 files changed

+86
-59
lines changed

6 files changed

+86
-59
lines changed

‎contracts/token/non_fungible/_NonFungibleToken.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ abstract contract _NonFungibleToken is _INonFungibleToken, _Introspectable {
240240
tokenId,
241241
data
242242
),
243-
'NonFungibleToken: transfer to non ERC721Receiver implementer'
243+
NonFungibleToken__ERC721ReceiverNotImplemented.selector
244244
);
245245

246246
bytes4 returnValue = abi.decode(returnData, (bytes4));

‎contracts/utils/AddressUtils.sol

+17-7
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ library AddressUtils {
1010
error AddressUtils__InsufficientBalance();
1111
error AddressUtils__NotContract();
1212
error AddressUtils__SendValueFailed();
13+
error AddressUtils__FailedCall();
14+
error AddressUtils__FailedCallWithValue();
15+
16+
// TODO: without this function, custom errors referenced by selector are not available in tests
17+
function __placeholder() internal {
18+
revert AddressUtils__FailedCall();
19+
revert AddressUtils__FailedCallWithValue();
20+
}
1321

1422
function toString(address account) internal pure returns (string memory) {
1523
return uint256(uint160(account)).toHexString(20);
@@ -32,14 +40,13 @@ library AddressUtils {
3240
address target,
3341
bytes memory data
3442
) internal returns (bytes memory) {
35-
return
36-
functionCall(target, data, 'AddressUtils: failed low-level call');
43+
return functionCall(target, data, AddressUtils__FailedCall.selector);
3744
}
3845

3946
function functionCall(
4047
address target,
4148
bytes memory data,
42-
string memory error
49+
bytes4 error
4350
) internal returns (bytes memory) {
4451
return _functionCallWithValue(target, data, 0, error);
4552
}
@@ -54,15 +61,15 @@ library AddressUtils {
5461
target,
5562
data,
5663
value,
57-
'AddressUtils: failed low-level call with value'
64+
AddressUtils__FailedCallWithValue.selector
5865
);
5966
}
6067

6168
function functionCallWithValue(
6269
address target,
6370
bytes memory data,
6471
uint256 value,
65-
string memory error
72+
bytes4 error
6673
) internal returns (bytes memory) {
6774
if (value > address(this).balance)
6875
revert AddressUtils__InsufficientBalance();
@@ -120,7 +127,7 @@ library AddressUtils {
120127
address target,
121128
bytes memory data,
122129
uint256 value,
123-
string memory error
130+
bytes4 error
124131
) private returns (bytes memory) {
125132
if (!isContract(target)) revert AddressUtils__NotContract();
126133

@@ -136,7 +143,10 @@ library AddressUtils {
136143
revert(add(32, returnData), returnData_size)
137144
}
138145
} else {
139-
revert(error);
146+
assembly {
147+
mstore(0, error)
148+
revert(0, 4)
149+
}
140150
}
141151
}
142152
}

‎contracts/utils/SafeERC20.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ library SafeERC20 {
9191
function _callOptionalReturn(IERC20 token, bytes memory data) private {
9292
bytes memory returndata = address(token).functionCall(
9393
data,
94-
'SafeERC20: low-level call failed'
94+
SafeERC20__OperationFailed.selector
9595
);
9696

9797
if (returndata.length > 0) {

‎spec/token/non_fungible/NonFungibleToken.behavior.ts

+6-4
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,9 @@ export function describeBehaviorOfNonFungibleToken(
378378
[
379379
'safeTransferFrom(address,address,uint256)'
380380
](holder.address, await instance.getAddress(), tokenId),
381-
).to.be.revertedWith(
382-
'NonFungibleToken: transfer to non ERC721Receiver implementer',
381+
).to.be.revertedWithCustomError(
382+
instance,
383+
'NonFungibleToken__ERC721ReceiverNotImplemented',
383384
);
384385
});
385386

@@ -510,8 +511,9 @@ export function describeBehaviorOfNonFungibleToken(
510511
[
511512
'safeTransferFrom(address,address,uint256,bytes)'
512513
](holder.address, await instance.getAddress(), tokenId, '0x'),
513-
).to.be.revertedWith(
514-
'NonFungibleToken: transfer to non ERC721Receiver implementer',
514+
).to.be.revertedWithCustomError(
515+
instance,
516+
'NonFungibleToken__ERC721ReceiverNotImplemented',
515517
);
516518
});
517519

‎test/token/non_fungible/NonFungibleToken.ts

+12-8
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,9 @@ describe('NonFungibleToken', () => {
245245
await instance.getAddress(),
246246
2,
247247
),
248-
).to.be.revertedWith(
249-
'NonFungibleToken: transfer to non ERC721Receiver implementer',
248+
).to.be.revertedWithCustomError(
249+
instance,
250+
'NonFungibleToken__ERC721ReceiverNotImplemented',
250251
);
251252
});
252253

@@ -368,8 +369,9 @@ describe('NonFungibleToken', () => {
368369
2,
369370
'0x',
370371
),
371-
).to.be.revertedWith(
372-
'NonFungibleToken: transfer to non ERC721Receiver implementer',
372+
).to.be.revertedWithCustomError(
373+
instance,
374+
'NonFungibleToken__ERC721ReceiverNotImplemented',
373375
);
374376
});
375377

@@ -594,8 +596,9 @@ describe('NonFungibleToken', () => {
594596
tokenId,
595597
'0x',
596598
),
597-
).to.be.revertedWith(
598-
'NonFungibleToken: transfer to non ERC721Receiver implementer',
599+
).to.be.revertedWithCustomError(
600+
instance,
601+
'NonFungibleToken__ERC721ReceiverNotImplemented',
599602
);
600603
});
601604

@@ -685,8 +688,9 @@ describe('NonFungibleToken', () => {
685688
0,
686689
'0x',
687690
),
688-
).to.be.revertedWith(
689-
'NonFungibleToken: transfer to non ERC721Receiver implementer',
691+
).to.be.revertedWithCustomError(
692+
instance,
693+
'NonFungibleToken__ERC721ReceiverNotImplemented',
690694
);
691695
});
692696
});

‎test/utils/AddressUtils.ts

+49-38
Original file line numberDiff line numberDiff line change
@@ -137,18 +137,16 @@ describe('AddressUtils', async () => {
137137
[
138138
'$functionCall(address,bytes)'
139139
](await targetContract.getAddress(), '0x'),
140-
).to.be.revertedWith('AddressUtils: failed low-level call');
140+
).to.be.revertedWithCustomError(instance, 'AddressUtils__FailedCall');
141141
});
142142
});
143143
});
144144

145-
describe('#functionCall(address,bytes,string)', () => {
145+
describe('#functionCall(address,bytes,bytes4)', () => {
146146
it('returns the bytes representation of the return value of the target function', async () => {
147147
const mock = await deployMockContract(deployer, [
148148
'function fn () external payable returns (bool)',
149149
]);
150-
const revertReason = 'REVERT_REASON';
151-
152150
await mock.mock.fn.returns(true);
153151

154152
const target = mock.address;
@@ -160,18 +158,18 @@ describe('AddressUtils', async () => {
160158
await instance
161159
.connect(deployer)
162160
[
163-
'$functionCall(address,bytes,string)'
164-
].staticCall(target, data, revertReason),
161+
'$functionCall(address,bytes,bytes4)'
162+
].staticCall(target, data, ethers.randomBytes(4)),
165163
).to.equal(ethers.zeroPadValue('0x01', 32));
166164
});
167165

168166
describe('reverts if', () => {
169167
it('target is not a contract', async () => {
170168
await expect(
171-
instance['$functionCall(address,bytes,string)'](
169+
instance['$functionCall(address,bytes,bytes4)'](
172170
ethers.ZeroAddress,
173171
'0x',
174-
'',
172+
ethers.randomBytes(4),
175173
),
176174
).to.be.revertedWithCustomError(
177175
instance,
@@ -196,12 +194,17 @@ describe('AddressUtils', async () => {
196194
await expect(
197195
instance
198196
.connect(deployer)
199-
['$functionCall(address,bytes,string)'](target, data, ''),
197+
[
198+
'$functionCall(address,bytes,bytes4)'
199+
](target, data, ethers.randomBytes(4)),
200200
).to.be.revertedWith(revertReason);
201201
});
202202

203-
it('target contract reverts, with provided error message', async () => {
204-
const revertReason = 'REVERT_REASON';
203+
it('target contract reverts, with provided custom error', async () => {
204+
// unrelated custom error, but it must exist on the contract due to limitiations with revertedWithCustomError matcher
205+
const customError = 'AddressUtils__InsufficientBalance';
206+
const revertReason =
207+
instance.interface.getError(customError)?.selector!;
205208

206209
const targetContract = await new AddressUtils__factory(
207210
deployer,
@@ -211,9 +214,9 @@ describe('AddressUtils', async () => {
211214
instance
212215
.connect(deployer)
213216
[
214-
'$functionCall(address,bytes,string)'
217+
'$functionCall(address,bytes,bytes4)'
215218
](await targetContract.getAddress(), '0x', revertReason),
216-
).to.be.revertedWith(revertReason);
219+
).to.be.revertedWithCustomError(instance, customError);
217220
});
218221
});
219222
});
@@ -278,7 +281,7 @@ describe('AddressUtils', async () => {
278281
);
279282
});
280283

281-
it('contract balance is insufficient for the call', async () => {
284+
it('contract balance is insufficient', async () => {
282285
await expect(
283286
instance
284287
.connect(deployer)
@@ -313,8 +316,9 @@ describe('AddressUtils', async () => {
313316
[
314317
'$functionCallWithValue(address,bytes,uint256)'
315318
](await targetContract.getAddress(), data, value),
316-
).to.be.revertedWith(
317-
'AddressUtils: failed low-level call with value',
319+
).to.be.revertedWithCustomError(
320+
instance,
321+
'AddressUtils__FailedCallWithValue',
318322
);
319323
});
320324

@@ -352,14 +356,15 @@ describe('AddressUtils', async () => {
352356
[
353357
'$functionCallWithValue(address,bytes,uint256)'
354358
](await targetContract.getAddress(), '0x', 0),
355-
).to.be.revertedWith(
356-
'AddressUtils: failed low-level call with value',
359+
).to.be.revertedWithCustomError(
360+
instance,
361+
'AddressUtils__FailedCallWithValue',
357362
);
358363
});
359364
});
360365
});
361366

362-
describe('#functionCallWithValue(address,bytes,uint256,string)', () => {
367+
describe('#functionCallWithValue(address,bytes,uint256,bytes4)', () => {
363368
it('returns the bytes representation of the return value of the target function', async () => {
364369
const mock = await deployMockContract(deployer, [
365370
'function fn () external payable returns (bool)',
@@ -376,8 +381,8 @@ describe('AddressUtils', async () => {
376381
await instance
377382
.connect(deployer)
378383
[
379-
'$functionCallWithValue(address,bytes,uint256,string)'
380-
].staticCall(target, data, 0, ''),
384+
'$functionCallWithValue(address,bytes,uint256,bytes4)'
385+
].staticCall(target, data, 0, ethers.randomBytes(4)),
381386
).to.equal(ethers.zeroPadValue('0x01', 32));
382387
});
383388

@@ -401,42 +406,45 @@ describe('AddressUtils', async () => {
401406
instance
402407
.connect(deployer)
403408
[
404-
'$functionCallWithValue(address,bytes,uint256,string)'
405-
](target, data, value, ''),
409+
'$functionCallWithValue(address,bytes,uint256,bytes4)'
410+
](target, data, value, ethers.randomBytes(4)),
406411
).to.changeEtherBalances([instance, mock], [-value, value]);
407412
});
408413

409414
describe('reverts if', () => {
410415
it('target is not a contract', async () => {
411416
await expect(
412-
instance['$functionCallWithValue(address,bytes,uint256,string)'](
417+
instance['$functionCallWithValue(address,bytes,uint256,bytes4)'](
413418
ethers.ZeroAddress,
414419
'0x',
415420
0,
416-
'',
421+
ethers.randomBytes(4),
417422
),
418423
).to.be.revertedWithCustomError(
419424
instance,
420425
'AddressUtils__NotContract',
421426
);
422427
});
423428

424-
it('contract balance is insufficient for the call', async () => {
429+
it('contract balance is insufficient', async () => {
425430
await expect(
426431
instance
427432
.connect(deployer)
428433
[
429-
'$functionCallWithValue(address,bytes,uint256,string)'
430-
](await instance.getAddress(), '0x', 1, ''),
434+
'$functionCallWithValue(address,bytes,uint256,bytes4)'
435+
](await instance.getAddress(), '0x', 1, ethers.randomBytes(4)),
431436
).to.be.revertedWithCustomError(
432437
instance,
433438
'AddressUtils__InsufficientBalance',
434439
);
435440
});
436441

437-
it('target function is not payable and value is included', async () => {
442+
it('target function is not payable and value is included, with provided custom error', async () => {
438443
const value = 2n;
439-
const revertReason = 'REVERT_REASON';
444+
// unrelated custom error, but it must exist on the contract due to limitiations with revertedWithCustomError matcher
445+
const customError = 'AddressUtils__InsufficientBalance';
446+
const revertReason =
447+
instance.interface.getError(customError)?.selector!;
440448

441449
await setBalance(await instance.getAddress(), value);
442450

@@ -455,9 +463,9 @@ describe('AddressUtils', async () => {
455463
instance
456464
.connect(deployer)
457465
[
458-
'$functionCallWithValue(address,bytes,uint256,string)'
466+
'$functionCallWithValue(address,bytes,uint256,bytes4)'
459467
](await targetContract.getAddress(), data, value, revertReason),
460-
).to.be.revertedWith(revertReason);
468+
).to.be.revertedWithCustomError(instance, customError);
461469
});
462470

463471
it('target contract reverts, with target contract error message', async () => {
@@ -478,13 +486,16 @@ describe('AddressUtils', async () => {
478486
instance
479487
.connect(deployer)
480488
[
481-
'$functionCallWithValue(address,bytes,uint256,string)'
482-
](target, data, 0, ''),
489+
'$functionCallWithValue(address,bytes,uint256,bytes4)'
490+
](target, data, 0, ethers.randomBytes(4)),
483491
).to.be.revertedWith(revertReason);
484492
});
485493

486-
it('target contract reverts, with provided error message', async () => {
487-
const revertReason = 'REVERT_REASON';
494+
it('target contract reverts, with provided custom error', async () => {
495+
// unrelated custom error, but it must exist on the contract due to limitiations with revertedWithCustomError matcher
496+
const customError = 'AddressUtils__InsufficientBalance';
497+
const revertReason =
498+
instance.interface.getError(customError)?.selector!;
488499

489500
const targetContract = await new AddressUtils__factory(
490501
deployer,
@@ -494,9 +505,9 @@ describe('AddressUtils', async () => {
494505
instance
495506
.connect(deployer)
496507
[
497-
'$functionCallWithValue(address,bytes,uint256,string)'
508+
'$functionCallWithValue(address,bytes,uint256,bytes4)'
498509
](await targetContract.getAddress(), '0x', 0, revertReason),
499-
).to.be.revertedWith(revertReason);
510+
).to.be.revertedWithCustomError(instance, customError);
500511
});
501512
});
502513
});

0 commit comments

Comments
 (0)