Omniscia Echidna Finance Audit

PtpDepositor Manual Review Findings

PtpDepositor Manual Review Findings

PDR-01M: Deprecated Approval Paradigm

Description:

The linked statements utilize the safeApprove function that has been officially deprecated.

Example:

contracts/core/PtpDepositor.sol
85//stake for msg.sender
86IERC20(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

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:

contracts/core/PtpDepositor.sol
108/// @notice Remove PTP from vePTP to the owner
109/// @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.