Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allowance issue with range payment #255

Closed
bb111189 opened this issue May 8, 2024 · 0 comments
Closed

allowance issue with range payment #255

bb111189 opened this issue May 8, 2024 · 0 comments

Comments

@bb111189
Copy link

bb111189 commented May 8, 2024

Describe the bug
There seems to be an issue with setting of allowance. The allowance can be overwritten by the subsequence range payment

    function payForRange(
        IPaymentCoordinator.RangePayment[] calldata rangePayments
    ) public virtual onlyOwner {
        for (uint256 i = 0; i < rangePayments.length; ++i) {
            // transfer token to ServiceManager and approve PaymentCoordinator to transfer again
            // in payForRange() call
            rangePayments[i].token.transferFrom(msg.sender, address(this), rangePayments[i].amount);
            rangePayments[i].token.approve(address(_paymentCoordinator), rangePayments[i].amount); <---- approve bug exist here
        }

        _paymentCoordinator.payForRange(rangePayments);
    }

For example, I am paying for two range

Range 0
token = TEST Token
amount = 10000
startTimestamp = 1000
duration = 100

Range 1
token = TEST Token
amount = 10
startTimestamp = 1100
uint32 duration = 100

Issue
rangePayments[i].token.approve(address(_paymentCoordinator), rangePayments[i].amount);

After the first loop, TEST token allowance for the contract is set to 10000
After the second loop, TEST token allowance for the contract is set to 10
The expected behavior should be the token allowance is set to 10010 instead of 10.

Note: This issue can happen in many different cases. Looping the rangePayments for different strategies, different reward window, etc. Basically, as long as you use the same ERC20 token and loop more than once, it will exists

When the following is called, it will have insufficient allowance to transfer TEST token
_paymentCoordinator.payForRange(rangePayments);

To fix this, very likely will need to

  • Get current allowance
  • set new allowance = current allowance + new token allowance

To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, could you add screenshots to help explain your problem?

Environment
Enter important environment info needed to reproduce the bug.

  • [e.g. chrome, safari]
  • [e.g. version]

Don't Forget To

  • Assign this to a project (our default is eigenlayer)
  • Add priority + size estimate
  • Set status to New
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants