IPCTDelegator Code Style Findings

IPD-01C: Inefficient Event Storage Read

Description:

The linked event emits the values of variables in storage that were written to by the function itself with in-memory values.

Example:

Copy
39/**
40 * @notice Called by the admin to update the implementation of the delegator
41 * @param implementation_ The address of the new implementation for delegation
42 */
43function _setImplementation(address implementation_) public {
44    require(msg.sender == admin, "IPCTDelegator::_setImplementation: admin only");
45    require(
46        implementation_ != address(0),
47        "IPCTDelegator::_setImplementation: invalid implementation address"
48    );
49
50    address oldImplementation = implementation;
51    implementation = implementation_;
52
53    emit NewImplementation(oldImplementation, implementation);
54}

Recommendation:

We advise the in-memory values from the function arguments to be utilized directly significantly reducing the gas cost of those functions.

Alleviation:

The code is no longer part of the codebase rendering this exhibit irrelevant.

IPD-02C: Redundant Low Level Implementation

Description:

The fallback function of IPCTDelegator is a mixture of a low-level assembly block and high level delegatecall instruction which is ill-advised.

Example:

Copy
71/**
72 * @dev Delegates execution to an implementation contract.
73 * It returns to the external caller whatever the implementation returns
74 * or forwards reverts.
75 */
76fallback() external {
77    // delegate all other functions to current implementation
78    (bool success, ) = implementation.delegatecall(msg.data);
79
80    assembly {
81        let free_mem_ptr := mload(0x40)
82        returndatacopy(free_mem_ptr, 0, returndatasize())
83
84        switch success
85        case 0 {
86            revert(free_mem_ptr, returndatasize())
87        }
88        default {
89            return(free_mem_ptr, returndatasize())
90        }
91    }
92}

Recommendation:

We advise the full code block to either be in its low-level assembly format or its high-level format whereby the second returned variable of delegatecall is properly used and decoded as necessary. Examples for both implementations can be observed in the proxy patterns and opportunistical calling patterns of OpenZeppelin in its Proxy style and SafeERC20 implementations respectively. The code may misbehave in future compiler implementations as the unused returned argument from delegatecall may clear the returndata stack in the future.

Alleviation:

The code is no longer part of the codebase rendering this exhibit irrelevant.