Non-decrementing allowance after recovery to MAX_INT
Opened this issue · 2 comments
zlace0x commented
After the fix for #30, renewable allowance that recovers to MAX_UINT256 will no longer decrement allowance going forward.
zlace0x commented
Potential solutions:
- Saturating add saturates at MAX_UINT256 - 1 (have to ensure that this applies only when it saturates)
- Only skip decrement when initialAmount is MAX_UINT256 (this potentially requires an additional read on state)
zlace0x commented
On further investigation, this likely is a non-issue:
Since maxAmount limits the maxRecoverable too, the following scenario cannot happen.
The following test passes given the scenario described here #51
function testMaxRecovery() public {
// approves MAX_UINT-1, recovery 1
vm.prank(user1);
funnel.approveRenewable(address(spender), type(uint256).max - 1, 1);
// same block transfer MAX_UINT-1
vm.prank(address(spender));
vm.expectEmit(true, false, false, true);
emit Transfer(user1, user2, type(uint256).max - 1);
assertTrue(funnel.transferFrom(user1, user2, type(uint256).max - 1));
assertEq(funnel.allowance(user1, address(spender)), 0);
vm.warp(2);
// next block allowance
assertEq(funnel.allowance(user1, address(spender)), 1);
}