suberra/funnel-contracts

Non-decrementing allowance after recovery to MAX_INT

Opened this issue · 2 comments

After the fix for #30, renewable allowance that recovers to MAX_UINT256 will no longer decrement allowance going forward.

Potential solutions:

  1. Saturating add saturates at MAX_UINT256 - 1 (have to ensure that this applies only when it saturates)
  2. Only skip decrement when initialAmount is MAX_UINT256 (this potentially requires an additional read on state)

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);
  }