Skip to content

Commit 9a99628

Browse files
ItsNickBarryItsNickBarry
authored
Merge pull request #138 from solidstate-network/erc20-allowance
ERC20 Allowance Improvements
2 parents 100b480 + 27b1fe5 commit 9a99628

File tree

6 files changed

+139
-42
lines changed

6 files changed

+139
-42
lines changed

‎contracts/token/ERC20/base/ERC20BaseInternal.sol‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,26 @@ abstract contract ERC20BaseInternal is IERC20BaseInternal {
6868
return true;
6969
}
7070

71+
/**
72+
* @notice decrease spend amount granted by holder to spender
73+
* @param holder address on whose behalf tokens may be spent
74+
* @param spender address whose allowance to decrease
75+
* @param amount quantity by which to decrease allowance
76+
*/
77+
function _decreaseAllowance(
78+
address holder,
79+
address spender,
80+
uint256 amount
81+
) internal {
82+
uint256 allowance = _allowance(holder, spender);
83+
84+
require(allowance >= amount, 'ERC20: insufficient allowance');
85+
86+
unchecked {
87+
_approve(holder, spender, allowance - amount);
88+
}
89+
}
90+
7191
/**
7292
* @notice mint tokens for given account
7393
* @param account recipient of minted tokens
@@ -151,16 +171,7 @@ abstract contract ERC20BaseInternal is IERC20BaseInternal {
151171
address recipient,
152172
uint256 amount
153173
) internal virtual returns (bool) {
154-
uint256 currentAllowance = _allowance(holder, msg.sender);
155-
156-
require(
157-
currentAllowance >= amount,
158-
'ERC20: transfer amount exceeds allowance'
159-
);
160-
161-
unchecked {
162-
_approve(holder, msg.sender, currentAllowance - amount);
163-
}
174+
_decreaseAllowance(holder, msg.sender, amount);
164175

165176
_transfer(holder, recipient, amount);
166177

‎contracts/token/ERC20/base/ERC20BaseMock.sol‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,12 @@ contract ERC20BaseMock is ERC20Base {
2828
) external {
2929
_approve(holder, spender, amount);
3030
}
31+
32+
function __decreaseAllowance(
33+
address holder,
34+
address spender,
35+
uint256 amount
36+
) external {
37+
_decreaseAllowance(holder, spender, amount);
38+
}
3139
}

‎contracts/token/ERC20/extended/ERC20ExtendedInternal.sol‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,15 @@ abstract contract ERC20ExtendedInternal is
2424
virtual
2525
returns (bool)
2626
{
27-
unchecked {
28-
mapping(address => uint256) storage allowances = ERC20BaseStorage
29-
.layout()
30-
.allowances[msg.sender];
27+
uint256 allowance = _allowance(msg.sender, spender);
3128

32-
uint256 allowance = allowances[spender];
29+
unchecked {
3330
require(
3431
allowance + amount >= allowance,
3532
'ERC20Extended: excessive allowance'
3633
);
3734

38-
_approve(
39-
msg.sender,
40-
spender,
41-
allowances[spender] = allowance + amount
42-
);
43-
44-
return true;
35+
return _approve(msg.sender, spender, allowance + amount);
4536
}
4637
}
4738

@@ -56,24 +47,8 @@ abstract contract ERC20ExtendedInternal is
5647
virtual
5748
returns (bool)
5849
{
59-
unchecked {
60-
mapping(address => uint256) storage allowances = ERC20BaseStorage
61-
.layout()
62-
.allowances[msg.sender];
50+
_decreaseAllowance(msg.sender, spender, amount);
6351

64-
uint256 allowance = allowances[spender];
65-
require(
66-
amount <= allowance,
67-
'ERC20Extended: insufficient allowance'
68-
);
69-
70-
_approve(
71-
msg.sender,
72-
spender,
73-
allowances[spender] = allowance - amount
74-
);
75-
76-
return true;
77-
}
52+
return true;
7853
}
7954
}

‎spec/token/ERC20/ERC20Base.behavior.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export function describeBehaviorOfERC20Base(
238238
receiver.address,
239239
amount,
240240
),
241-
).to.be.revertedWith('ERC20: transfer amount exceeds allowance');
241+
).to.be.revertedWith('ERC20: insufficient allowance');
242242
});
243243
});
244244
});

‎spec/token/ERC20/ERC20Extended.behavior.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ export function describeBehaviorOfERC20Extended(
3434
});
3535

3636
describe('#increaseAllowance(address,uint256)', function () {
37+
it('returns true', async () => {
38+
expect(
39+
await instance
40+
.connect(holder)
41+
.callStatic['increaseAllowance(address,uint256)'](
42+
instance.address,
43+
ethers.constants.Zero,
44+
),
45+
).to.be.true;
46+
});
47+
3748
it('increases approval of spender with respect to holder by given amount', async function () {
3849
let amount = ethers.constants.Two;
3950

@@ -90,6 +101,17 @@ export function describeBehaviorOfERC20Extended(
90101
});
91102

92103
describe('#decreaseAllowance(address,uint256)', function () {
104+
it('returns true', async () => {
105+
expect(
106+
await instance
107+
.connect(holder)
108+
.callStatic['decreaseAllowance(address,uint256)'](
109+
instance.address,
110+
ethers.constants.Zero,
111+
),
112+
).to.be.true;
113+
});
114+
93115
it('decreases approval of spender with respect to holder by given amount', async function () {
94116
let amount = ethers.constants.Two;
95117
await instance
@@ -142,7 +164,7 @@ export function describeBehaviorOfERC20Extended(
142164
spender.address,
143165
ethers.constants.One,
144166
),
145-
).to.be.revertedWith('ERC20Extended: insufficient allowance');
167+
).to.be.revertedWith('ERC20: insufficient allowance');
146168
});
147169
});
148170
});

‎test/token/ERC20/ERC20Base.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,87 @@ describe('ERC20Base', function () {
249249
});
250250
});
251251

252+
describe('#_decreaseAllowance(address,address,uint256)', function () {
253+
it('reduces approval of spender with respect to holder by given amount', async function () {
254+
await instance.__approve(
255+
holder.address,
256+
spender.address,
257+
ethers.constants.Two,
258+
);
259+
260+
await instance
261+
.connect(holder)
262+
.__decreaseAllowance(
263+
holder.address,
264+
spender.address,
265+
ethers.constants.One,
266+
);
267+
268+
await expect(
269+
await instance.callStatic['allowance(address,address)'](
270+
holder.address,
271+
spender.address,
272+
),
273+
).to.equal(ethers.constants.One);
274+
275+
// decreases are cumulative
276+
await instance
277+
.connect(holder)
278+
.__decreaseAllowance(
279+
holder.address,
280+
spender.address,
281+
ethers.constants.One,
282+
);
283+
284+
await expect(
285+
await instance.callStatic['allowance(address,address)'](
286+
holder.address,
287+
spender.address,
288+
),
289+
).to.equal(ethers.constants.Zero);
290+
});
291+
292+
it('emits Approval event', async function () {
293+
await instance.__approve(
294+
holder.address,
295+
spender.address,
296+
ethers.constants.Two,
297+
);
298+
299+
await expect(
300+
instance.__decreaseAllowance(
301+
holder.address,
302+
spender.address,
303+
ethers.constants.One,
304+
),
305+
)
306+
.to.emit(instance, 'Approval')
307+
.withArgs(holder.address, spender.address, ethers.constants.One);
308+
});
309+
310+
describe('reverts if', function () {
311+
it('holder is the zero address', async function () {
312+
await expect(
313+
instance.__decreaseAllowance(
314+
ethers.constants.AddressZero,
315+
spender.address,
316+
ethers.constants.Zero,
317+
),
318+
).to.be.revertedWith('ERC20: approve from the zero address');
319+
});
320+
321+
it('spender is the zero address', async function () {
322+
await expect(
323+
instance.__decreaseAllowance(
324+
holder.address,
325+
ethers.constants.AddressZero,
326+
ethers.constants.Zero,
327+
),
328+
).to.be.revertedWith('ERC20: approve to the zero address');
329+
});
330+
});
331+
});
332+
252333
describe('#_beforeTokenTransfer(address,address,uint256)', function () {
253334
it('todo');
254335
});

0 commit comments

Comments
 (0)