Omniscia Echidna Finance Audit
PtpDepositor Manual Review Findings
PtpDepositor Manual Review Findings
PDR-01M: Deprecated Approval Paradigm
Type | Severity | Location |
---|---|---|
Standard Conformity | Minor | PtpDepositor.sol:L86-L87 |
Description:
The linked statements utilize the safeApprove
function that has been officially deprecated.
Example:
85//stake for msg.sender86IERC20(ecdptp).safeApprove(_stakeAddress, 0);87IERC20(ecdptp).safeApprove(_stakeAddress, _amount);88IEcdPtpStaking(_stakeAddress).depositFor(msg.sender, _amount);
Recommendation:
We advise a direct approve
instruction to be utilized instead as the statements are indeed meant to replace any previously set allowance to the new one.
Alleviation:
A direct approve
statement is now utilized in place of the deprecated paradigm.
PDR-02M: Improperly Decentralized Functionality
Type | Severity | Location |
---|---|---|
Logical Fault | Minor | PtpDepositor.sol:L99-L116 |
Description:
The linked functions are meant to allow the system to recover from a failure case by allowing a shut down of PTP deposits and a consequent withdrawal of the full amount to the owner's address. Although a time based delay is introduced between the two function calls, the funds are still transferred to the owner's wallet and there is no way to reverse this operation.
Example:
108/// @notice Remove PTP from vePTP to the owner109/// @dev Can only be called 7 days after calling `shutwownPtpDeposits`110function executeWithdraw() external onlyOwner {111 require(withdrawAfter < block.timestamp);112 withdrawAfter = type(uint256).max;113 IPlatypusProxy(platypusProxy).release();114 uint256 balance = IERC20(ptp).balanceOf(platypusProxy);115 IERC20(ptp).safeTransferFrom(platypusProxy, msg.sender, balance);116}
Recommendation:
We advise this trait of the system to be revised and to allow for a potential revocation mechanism via a governance vote. Alternatively, if the owner is meant to be the governance module of Echidna we advise this to be clearly depicted so.
Alleviation:
The shutdown operation now assigns a governance
member that is consequently transferred all the funds to by executeWithdraw
, indicating that the governance module is indeed meant to acquire the funds that are withdrawn in this scenario.