AToken.handleRepayment function does not accounts for repayments made on behalf of another user

[miguelmtzinf]

The AToken.handleRepayment function is intended to be invoked when a repayment of debt occurs, right after the funds are transferred from the user to the AToken. This function allows to add custom logic so the AToken can utilize the funds it holds by calling other parties (staking the funds on a vault, for instance).

The issue is that this function only expects the address that is executing the repayment, and not the address the repayment is being made on behalf of (onBehalfOf). Adding this new parameter helps to account for repayments on behalf of another user.

[eboadom]

In which case do you need the onBehalfOf() (or even msg.sender)? My line of thought is that when a capital injection of any type happens on the aToken, this capital immediately belongs 1) to all depositors or 2) to the collector via accruedToTreasury. So don't see the point of adding any type of "recipient", especially because from the perspective of the aToken, there is not really any "repayment" concept; the aToken doesn't understand about debt

[miguelmtzinf]

@eboadom Thanks for the comment.

I agree that handleRepayment function seems it belongs more to a debtToken rather than aToken. However, aTokens were designed to hold the underlying assets, and the simplest way to do "things" with those after a repayment is having such a function in the AToken.

Since it's possible to do a repayment on behalf of another user, it's interesting to know both the address of the sender and the onBehalfOf. In the case of a custom aToken that stakes the underlying at a vault, its natural to account for the right user when the repayment is done. This gives more flexibility (and room to innovate) and allows custom ATokens to stay aligned with the inner workings of the protocol (eg. onBehalfOf)

For instance, an aToken for BPTs that are staked at the Gauge:

1. Alice supplies 100 BPT to the pool.The AToken stakes the underlying at the corresponding Gauge.
2. Bob supplies 1000 USDC to the pool. Bob gives borrowing power to Cat.
3. Cat borrows 10 BPT on behalf of Bob. The AToken unstakes the underlying from the Gauge and redirects the assets.
   (Time flies)
4. Cat repays 15 BPT (10+interests) on behalf of Bob. These assets need to be staked back at the Gauge again.

At this point, theAToken.handleRepayment() is able to determine if the repay amount should be staked in favor of Cat or Bob. Without the onBehalfOf, it will be always in favor of the sender, which could be undesirable.

[eboadom]

@miguelmtzinf no opposition to having the function, my point is more that it is not a handle of repayment, it is just a hook to do an action post-transferFrom. Basically, the naming doesn't sound correct to me.

[miguelmtzinf]

@eboadom any suggestion? We are open to a name change (this function is internal and have never been used yet)

Although handleRepayment name is not ideal, it feels aligned when you are reading the code. The name indicates the function should be called whenever a repayment is done (funds going from user to AToken)

[eboadom]

Maybe it is fine to just keep it as is, to not increase the changes. The inconsistency for me is more high-level, as currently, transfer in on minting happens without a "hook" (the mint() itself is the hook), while this requires a hook itself.
But it is a bit different topic.