Skip to content

Commit 54f06e7

Browse files
ernestognwernestognw
committed
Improve ERC1363 tests
1 parent 96551b0 commit 54f06e7

File tree

3 files changed

+66
-37
lines changed

3 files changed

+66
-37
lines changed

‎contracts/token/ERC20/extensions/ERC1363.sol‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363 {
5959
* - The target has code (i.e. is a contract).
6060
* - The target `to` must implement the {IERC1363Receiver} interface.
6161
* - The target should return the {IERC1363Receiver} interface id.
62-
* - The internal {transfer} must have succeeded (returned `true`)
62+
* - The internal {transfer} must have succeeded (returned `true`).
6363
*/
6464
function transferAndCall(address to, uint256 value) public returns (bool) {
6565
return transferAndCall(to, value, "");
6666
}
6767

6868
/**
69-
* @dev Variant of {transferAndCall} that accepts an additional `data` parameter with
69+
* @dev Variant of {transferAndCall} that accepts an additional `data` parameter with
7070
* no specified format.
7171
*/
7272
function transferAndCall(address to, uint256 value, bytes memory data) public virtual returns (bool) {
@@ -86,14 +86,14 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363 {
8686
* - The target has code (i.e. is a contract).
8787
* - The target `to` must implement the {IERC1363Receiver} interface.
8888
* - The target should return the {IERC1363Receiver} interface id.
89-
* - The internal {transferFrom} must have succeeded (returned `true`)
89+
* - The internal {transferFrom} must have succeeded (returned `true`).
9090
*/
9191
function transferFromAndCall(address from, address to, uint256 value) public returns (bool) {
9292
return transferFromAndCall(from, to, value, "");
9393
}
9494

9595
/**
96-
* @dev Variant of {transferFromAndCall} that accepts an additional `data` parameter with
96+
* @dev Variant of {transferFromAndCall} that accepts an additional `data` parameter with
9797
* no specified format.
9898
*/
9999
function transferFromAndCall(
@@ -118,14 +118,14 @@ abstract contract ERC1363 is ERC20, ERC165, IERC1363 {
118118
* - The target has code (i.e. is a contract).
119119
* - The target `to` must implement the {IERC1363Spender} interface.
120120
* - The target should return the {IERC1363Spender} interface id.
121-
* - The internal {approve} must have succeeded (returned `true`)
121+
* - The internal {approve} must have succeeded (returned `true`).
122122
*/
123123
function approveAndCall(address spender, uint256 value) public returns (bool) {
124124
return approveAndCall(spender, value, "");
125125
}
126126

127127
/**
128-
* @dev Variant of {approveAndCall} that accepts an additional `data` parameter with
128+
* @dev Variant of {approveAndCall} that accepts an additional `data` parameter with
129129
* no specified format.
130130
*/
131131
function approveAndCall(address spender, uint256 value, bytes memory data) public virtual returns (bool) {

‎contracts/token/ERC20/utils/SafeERC20.sol‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ library SafeERC20 {
8787
* code. This can be used to implement an {ERC721}-like safe transfer that rely on {ERC1363} checks when
8888
* targeting contracts.
8989
*
90-
* Revert if the returned value is other than `true`.
90+
* Reverts if the returned value is other than `true`.
9191
*/
9292
function transferAndCallRelaxed(IERC1363 token, address to, uint256 value, bytes memory data) internal {
9393
if (to.code.length == 0) {
@@ -102,7 +102,7 @@ library SafeERC20 {
102102
* has no code. This can be used to implement an {ERC721}-like safe transfer that rely on {ERC1363} checks when
103103
* targeting contracts.
104104
*
105-
* Revert if the returned value is other than `true`.
105+
* Reverts if the returned value is other than `true`.
106106
*/
107107
function transferFromAndCallRelaxed(
108108
IERC1363 token,
@@ -127,7 +127,7 @@ library SafeERC20 {
127127
* Opposedly, when the recipient address (`to`) has code, this function only attempts to call {ERC1363-approveAndCall}
128128
* once without retrying, and relies on the returned value to be true.
129129
*
130-
* Revert if the returned value is other than `true`.
130+
* Reverts if the returned value is other than `true`.
131131
*/
132132
function approveAndCallRelaxed(IERC1363 token, address to, uint256 value, bytes memory data) internal {
133133
if (to.code.length == 0) {

‎test/token/ERC20/extensions/ERC1363.test.js‎

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ const { ethers } = require('hardhat');
22
const { expect } = require('chai');
33
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
44

5-
const { shouldBehaveLikeERC20 } = require('../ERC20.behavior.js');
5+
const {
6+
shouldBehaveLikeERC20,
7+
shouldBehaveLikeERC20Transfer,
8+
shouldBehaveLikeERC20Approve,
9+
} = require('../ERC20.behavior.js');
610
const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior');
711
const { RevertType } = require('../../../helpers/enums.js');
812

@@ -43,19 +47,25 @@ describe('ERC1363', function () {
4347

4448
shouldSupportInterfaces(['ERC165', 'ERC1363']);
4549
shouldBehaveLikeERC20(value);
46-
// Note: transferAndCall(address,uint256) fails "shouldBehaveLikeERC20Transfer" because it revert on EOAs
47-
// Note: transferAndCall(address,uint256,bytes) fails "shouldBehaveLikeERC20Transfer" because it revert on EOAs
48-
// Note: approveAndCall(address,uint256) fails "shouldBehaveLikeERC20Approve" because it revert on EOAs
49-
// Note: approveAndCall(address,uint256,bytes) fails "shouldBehaveLikeERC20Approve" because it revert on EOAs
5050

5151
describe('transferAndCall', function () {
52-
it('to an EOA', async function () {
52+
describe('as a transfer', function () {
53+
beforeEach(async function () {
54+
this.recipient = this.receiver;
55+
this.transfer = (holder, ...rest) =>
56+
this.token.connect(holder).getFunction('transferAndCall(address,uint256)')(...rest);
57+
});
58+
59+
shouldBehaveLikeERC20Transfer(value);
60+
});
61+
62+
it('reverts transfering to an EOA', async function () {
5363
await expect(this.token.connect(this.holder).getFunction('transferAndCall(address,uint256)')(this.other, value))
5464
.to.be.revertedWithCustomError(this.token, 'ERC1363InvalidReceiver')
5565
.withArgs(this.other.address);
5666
});
5767

58-
it('without data', async function () {
68+
it('succeeds without data', async function () {
5969
await expect(
6070
this.token.connect(this.holder).getFunction('transferAndCall(address,uint256)')(this.receiver, value),
6171
)
@@ -65,7 +75,7 @@ describe('ERC1363', function () {
6575
.withArgs(this.holder.address, this.holder.address, value, '0x');
6676
});
6777

68-
it('with data', async function () {
78+
it('succeeds with data', async function () {
6979
await expect(
7080
this.token.connect(this.holder).getFunction('transferAndCall(address,uint256,bytes)')(
7181
this.receiver,
@@ -79,7 +89,7 @@ describe('ERC1363', function () {
7989
.withArgs(this.holder.address, this.holder.address, value, data);
8090
});
8191

82-
it('with reverting hook (without reason)', async function () {
92+
it('reverts with reverting hook (without reason)', async function () {
8393
await this.receiver.setUp(this.selectors.onTransferReceived, RevertType.RevertWithoutMessage);
8494

8595
await expect(
@@ -93,7 +103,7 @@ describe('ERC1363', function () {
93103
.withArgs(this.receiver.target);
94104
});
95105

96-
it('with reverting hook (with reason)', async function () {
106+
it('reverts with reverting hook (with reason)', async function () {
97107
await this.receiver.setUp(this.selectors.onTransferReceived, RevertType.RevertWithMessage);
98108

99109
await expect(
@@ -105,7 +115,7 @@ describe('ERC1363', function () {
105115
).to.be.revertedWith('ERC1363ReceiverMock: reverting');
106116
});
107117

108-
it('with reverting hook (with custom error)', async function () {
118+
it('reverts with reverting hook (with custom error)', async function () {
109119
const reason = '0x12345678';
110120
await this.receiver.setUp(reason, RevertType.RevertWithCustomError);
111121

@@ -120,7 +130,7 @@ describe('ERC1363', function () {
120130
.withArgs(reason);
121131
});
122132

123-
it('with reverting hook (with panic)', async function () {
133+
it('panics with reverting hook (with panic)', async function () {
124134
await this.receiver.setUp(this.selectors.onTransferReceived, RevertType.Panic);
125135

126136
await expect(
@@ -132,7 +142,7 @@ describe('ERC1363', function () {
132142
).to.be.revertedWithPanic();
133143
});
134144

135-
it('with bad return value', async function () {
145+
it('reverts with bad return value', async function () {
136146
await this.receiver.setUp('0x12345678', RevertType.None);
137147

138148
await expect(
@@ -152,7 +162,16 @@ describe('ERC1363', function () {
152162
await this.token.connect(this.holder).approve(this.other, ethers.MaxUint256);
153163
});
154164

155-
it('to an EOA', async function () {
165+
describe('as a transfer', function () {
166+
beforeEach(async function () {
167+
this.recipient = this.receiver;
168+
this.transfer = this.token.connect(this.other).getFunction('transferFromAndCall(address,address,uint256)');
169+
});
170+
171+
shouldBehaveLikeERC20Transfer(value);
172+
});
173+
174+
it('reverts transfering to an EOA', async function () {
156175
await expect(
157176
this.token.connect(this.other).getFunction('transferFromAndCall(address,address,uint256)')(
158177
this.holder,
@@ -164,7 +183,7 @@ describe('ERC1363', function () {
164183
.withArgs(this.other.address);
165184
});
166185

167-
it('without data', async function () {
186+
it('succeeds without data', async function () {
168187
await expect(
169188
this.token.connect(this.other).getFunction('transferFromAndCall(address,address,uint256)')(
170189
this.holder,
@@ -178,7 +197,7 @@ describe('ERC1363', function () {
178197
.withArgs(this.other.address, this.holder.address, value, '0x');
179198
});
180199

181-
it('with data', async function () {
200+
it('succeeds with data', async function () {
182201
await expect(
183202
this.token.connect(this.other).getFunction('transferFromAndCall(address,address,uint256,bytes)')(
184203
this.holder,
@@ -193,7 +212,7 @@ describe('ERC1363', function () {
193212
.withArgs(this.other.address, this.holder.address, value, data);
194213
});
195214

196-
it('with reverting hook (without reason)', async function () {
215+
it('reverts with reverting hook (without reason)', async function () {
197216
await this.receiver.setUp(this.selectors.onTransferReceived, RevertType.RevertWithoutMessage);
198217

199218
await expect(
@@ -208,7 +227,7 @@ describe('ERC1363', function () {
208227
.withArgs(this.receiver.target);
209228
});
210229

211-
it('with reverting hook (with reason)', async function () {
230+
it('reverts with reverting hook (with reason)', async function () {
212231
await this.receiver.setUp(this.selectors.onTransferReceived, RevertType.RevertWithMessage);
213232

214233
await expect(
@@ -221,7 +240,7 @@ describe('ERC1363', function () {
221240
).to.be.revertedWith('ERC1363ReceiverMock: reverting');
222241
});
223242

224-
it('with reverting hook (with custom error)', async function () {
243+
it('reverts with reverting hook (with custom error)', async function () {
225244
const reason = '0x12345678';
226245
await this.receiver.setUp(reason, RevertType.RevertWithCustomError);
227246

@@ -237,7 +256,7 @@ describe('ERC1363', function () {
237256
.withArgs(reason);
238257
});
239258

240-
it('with reverting hook (with panic)', async function () {
259+
it('panics with reverting hook (with panic)', async function () {
241260
await this.receiver.setUp(this.selectors.onTransferReceived, RevertType.Panic);
242261

243262
await expect(
@@ -250,7 +269,7 @@ describe('ERC1363', function () {
250269
).to.be.revertedWithPanic();
251270
});
252271

253-
it('with bad return value', async function () {
272+
it('reverts with bad return value', async function () {
254273
await this.receiver.setUp('0x12345678', RevertType.None);
255274

256275
await expect(
@@ -267,21 +286,31 @@ describe('ERC1363', function () {
267286
});
268287

269288
describe('approveAndCall', function () {
270-
it('an EOA', async function () {
289+
describe('as an approval', function () {
290+
beforeEach(async function () {
291+
this.recipient = this.spender;
292+
this.approve = (holder, ...rest) =>
293+
this.token.connect(holder).getFunction('approveAndCall(address,uint256)')(...rest);
294+
});
295+
296+
shouldBehaveLikeERC20Approve(value);
297+
});
298+
299+
it('reverts approving an EOA', async function () {
271300
await expect(this.token.connect(this.holder).getFunction('approveAndCall(address,uint256)')(this.other, value))
272301
.to.be.revertedWithCustomError(this.token, 'ERC1363InvalidSpender')
273302
.withArgs(this.other.address);
274303
});
275304

276-
it('without data', async function () {
305+
it('succeeds without data', async function () {
277306
await expect(this.token.connect(this.holder).getFunction('approveAndCall(address,uint256)')(this.spender, value))
278307
.to.emit(this.token, 'Approval')
279308
.withArgs(this.holder.address, this.spender.target, value)
280309
.to.emit(this.spender, 'Approved')
281310
.withArgs(this.holder.address, value, '0x');
282311
});
283312

284-
it('with data', async function () {
313+
it('succeeds with data', async function () {
285314
await expect(
286315
this.token.connect(this.holder).getFunction('approveAndCall(address,uint256,bytes)')(this.spender, value, data),
287316
)
@@ -301,15 +330,15 @@ describe('ERC1363', function () {
301330
.withArgs(this.spender.target);
302331
});
303332

304-
it('with reverting hook (with reason)', async function () {
333+
it('reverts with reverting hook (with reason)', async function () {
305334
await this.spender.setUp(this.selectors.onApprovalReceived, RevertType.RevertWithMessage);
306335

307336
await expect(
308337
this.token.connect(this.holder).getFunction('approveAndCall(address,uint256,bytes)')(this.spender, value, data),
309338
).to.be.revertedWith('ERC1363SpenderMock: reverting');
310339
});
311340

312-
it('with reverting hook (with custom error)', async function () {
341+
it('reverts with reverting hook (with custom error)', async function () {
313342
const reason = '0x12345678';
314343
await this.spender.setUp(reason, RevertType.RevertWithCustomError);
315344

@@ -320,15 +349,15 @@ describe('ERC1363', function () {
320349
.withArgs(reason);
321350
});
322351

323-
it('with reverting hook (with panic)', async function () {
352+
it('panics with reverting hook (with panic)', async function () {
324353
await this.spender.setUp(this.selectors.onApprovalReceived, RevertType.Panic);
325354

326355
await expect(
327356
this.token.connect(this.holder).getFunction('approveAndCall(address,uint256,bytes)')(this.spender, value, data),
328357
).to.be.revertedWithPanic();
329358
});
330359

331-
it('with bad return value', async function () {
360+
it('reverts with bad return value', async function () {
332361
await this.spender.setUp('0x12345678', RevertType.None);
333362

334363
await expect(

0 commit comments

Comments
 (0)