[Certora][M-07] Vaibhav/certora/fix steth swap #163
Conversation
|
It also looks like your changes have causes some liquifier unit tests to fail: |
src/Liquifier.sol
Outdated
| if (_amount > liquidityPool.eETH().balanceOf(msg.sender)) revert NotEnoughBalance(); | ||
|
|
||
| IERC20(address(liquidityPool.eETH())).safeTransferFrom(msg.sender, address(this), _amount); | ||
| bool isWhitelistedSwapper = roleRegistry.hasRole(EETH_STETH_SWAPPER, msg.sender); |
There was a problem hiding this comment.
Can you remove this line and include the hasRole check in the if statement like so:
if (!roleRegistry.hasRole(EETH_STETH_SWAPPER, msg.sender)) {
fees = _amount * feeSwappingEETHToSTETH / 10**18;
}
This removes an unnecessary line of code and is consistent with the pattern we have already established in the codebase
Yes met the requirement |
src/Liquifier.sol
Outdated
| @@ -242,7 +245,7 @@ contract Liquifier is Initializable, UUPSUpgradeable, OwnableUpgradeable, Pausab | |||
| } | |||
|
|
|||
| // Swap Liquifier's eETH for ETH from the liquidity pool and send it back to the liquidity pool | |||
| function withdrawEEth(uint256 amount) external { | |||
| function withdrawEEth(uint256 amount) public { | |||
There was a problem hiding this comment.
why public? i think you can leave it as external? :)
src/Liquifier.sol
Outdated
|
|
||
| IERC20(address(liquidityPool.eETH())).safeTransferFrom(msg.sender, address(this), _amount); | ||
| IERC20(address(lido)).safeTransfer(msg.sender, _amount); | ||
| liquidityPool.withdraw(address(liquidityPool), _amount-1); |
There was a problem hiding this comment.
what would be the clean way to handle -1?
We can do diff of stETH balance before/after its transfer and transfer that amount
| vm.startPrank(superAdmin); | ||
| roleRegistry.grantRole(liquifierInstance.EETH_STETH_SWAPPER(), bob); |
There was a problem hiding this comment.
need to test the case where it reverts without the role
vvalecha519
changed the title
seongyun-ko
changed the title
No description provided.