Exec upgrade (defer liquidity for multiple accounts, max amount support on pToken wrap/unwrap) #163
Conversation
There was a problem hiding this comment.
Excellent, great work on the testing, especially adding specific tests for the deferred liquidity callbacks!
In the future I suggest working on uncoupled features in separate branches and using separate PRs, so that comments on one of the features doesn't delay the merging of the other. Don't worryy about splitting this one apart though, they both look fine to me.
contracts/modules/Exec.sol
Outdated
| /// @notice Defer liquidity checking for an array of accounts, to perform rebalancing, flash loans, etc. msg.sender must implement IDeferredLiquidityCheck | ||
| /// @param accounts The array of accounts to defer liquidity for | ||
| /// @param data Passed through to the onDeferredLiquidityCheck() callback, so contracts don't need to store transient data in storage | ||
| function deferLiquidityCheckExtended(address[] memory accounts, bytes memory data) external reentrantOK { |
There was a problem hiding this comment.
I think better name might be deferLiquidityChecks or deferLiquidityCheckMulti or something, to make clear how it was extended.
|
|
||
| if (status == DEFERLIQUIDITY__DIRTY) checkLiquidity(account); | ||
| } | ||
| } |
There was a problem hiding this comment.
Looking at it here, I think it would make sense to re-use the code between the single and multi-versions by having the single call the multi. Technically it would cost a bit more gas, but I think it's worth it to keep the code clean.
No description provided.