test: bulker #765
test: bulker #765
Conversation
pakim249CAL
requested review from
Rubilmax,
MerlinEgalite and
makcandrov
and removed request for
Rubilmax and
MerlinEgalite
There was a problem hiding this comment.
Nice !! 🔥
Some suggestions to improve the testing suite as I think it's not 100% complete:
- Add some random (or not if too complex) sequences of tx because we are only testing one tx at a time
- Make sure that it's not possible to borrow, withdraw on behalf of someone else. Example: Alice deposits some WETH as collateral, bob should be able to borrow, withdrawCollateral in any way
| (actions[0], data[0]) = _getWrapStETHData(amount); | ||
|
|
||
| bulker.execute(actions, data); | ||
| assertEq(ERC20(sNative).balanceOf(address(bulker)), IWSTETH(sNative).getWstETHByStETH(amount), "bulker balance"); |
There was a problem hiding this comment.
in general you also test that the stETH balance is zero
| assertApproxEqAbs(morpho.borrowBalance(borrowedMarket.underlying, delegator), 0, 2, "onBehalf borrow balance"); | ||
| } | ||
|
|
||
| // asset amount receiver maxiterations |
There was a problem hiding this comment.
Whoops, yes. I put this as a note for myself so I wouldn't have to reference later haha.
I agree with your other points, but this particular one I'm not sure we can actually address. There is simply no entry point to ever do an action on behalf of anyone else in the bulker, as the parameters sent to Morpho are strictly based on msg.sender. So testing this would simply be testing msg.sender. |
I think we can consider invariant tests to be in a different scope and can be completely separated into another PR. |
Separate PR for testing the bulker to prevent clutter in the feat PR while I work on this.