[L] ConfigTimelockController One address should not be able to be a proposer and a executor at the same time
The purpose of the design which have two separate roles of proposers and executors is to prevent the centralization risk which one can propose a change and get it executed right away by themselves.
However, the current implementation allows the same address to be in proposers and executors at the same time, this makes such a design becomes a unenforced law.
In operation practise, the admin can and, we believe they have a natural tendency to add both roles to the same address as this is easier in operation.
Recommendation
Consider adding a check to make sure proposers and executors do not include any same address
[L] Tokens with fee on transfer are not supported
There are ERC20 tokens that charge fee for every transfer() or transferFrom().
In the current implementation, Controller.sol#_mintOptionsPosition(), Controller.sol#_mintSpread() and Controller.sol#_claimCollateral() assumes that the received amount is the same as the transfer amount, and uses it for collateralToken minting and redeeming.
Recommendation
Making sure no FOT tokens will be whitelisted in the protocol.
alcueca
commented
2 years ago
[L]
ConfigTimelockControllerOne address should not be able to be aproposerand aexecutorat the same timeThe purpose of the design which have two separate roles of
proposersandexecutorsis to prevent the centralization risk which one can propose a change and get it executed right away by themselves.However, the current implementation allows the same address to be in
proposersandexecutorsat the same time, this makes such a design becomes a unenforced law.In operation practise, the admin can and, we believe they have a natural tendency to add both roles to the same address as this is easier in operation.
Recommendation
Consider adding a check to make sure
proposersandexecutorsdo not include any same address[L] Tokens with fee on transfer are not supported
There are ERC20 tokens that charge fee for every
transfer()ortransferFrom().In the current implementation,
Controller.sol#_mintOptionsPosition(),Controller.sol#_mintSpread()andController.sol#_claimCollateral()assumes that the received amount is the same as the transfer amount, and uses it for collateralToken minting and redeeming.Recommendation
Making sure no FOT tokens will be whitelisted in the protocol.