Omniscia Gravita Protocol Audit

ActivePool Code Style Findings

ActivePool Code Style Findings

APL-01C: Inefficient Renunciation of Ownership

TypeSeverityLocation
Gas OptimizationActivePool.sol:L105

Description:

The ActivePool::setAddresses function will invoke the OwnableUpgradeable::renounceOwnership function which in turn will apply the onlyOwner modifier redundantly.

Example:

contracts/ActivePool.sol
84function setAddresses(
85    address _borrowerOperationsAddress,
86    address _collSurplusPoolAddress,
87    address _defaultPoolAddress,
88    address _stabilityPoolAddress,
89    address _vesselManagerAddress,
90    address _vesselManagerOperationsAddress
91) external initializer {
92    require(!isInitialized, "Already initialized");
93    isInitialized = true;
94

95    __Ownable_init();
96    __ReentrancyGuard_init();
97

98    borrowerOperationsAddress = _borrowerOperationsAddress;
99    collSurplusPool = ICollSurplusPool(_collSurplusPoolAddress);
100    defaultPool = IDefaultPool(_defaultPoolAddress);
101    stabilityPoolAddress = _stabilityPoolAddress;
102    vesselManagerAddress = _vesselManagerAddress;
103    vesselManagerOperationsAddress = _vesselManagerOperationsAddress;
104

105    renounceOwnership();
106}

Recommendation:

We advise the OwnableUpgradeable::_transferOwnership function to be utilized directly, transferring ownership to the zero address.

Alleviation:

Ownership of the contract is no longer renounced in the latest iteration of the codebase rendering this exhibit inapplicable.

APL-02C: Inexplicable Ownable Pattern

TypeSeverityLocation
Gas OptimizationActivePool.sol:L95, L105

Description:

The ActivePool inherits the OwnableUpgradeable implementation redundantly as it initializes it within the ActivePool::setAddresses function and consequently renounces ownership in the same call.

Example:

contracts/ActivePool.sol
84function setAddresses(
85    address _borrowerOperationsAddress,
86    address _collSurplusPoolAddress,
87    address _defaultPoolAddress,
88    address _stabilityPoolAddress,
89    address _vesselManagerAddress,
90    address _vesselManagerOperationsAddress
91) external initializer {
92    require(!isInitialized, "Already initialized");
93    isInitialized = true;
94

95    __Ownable_init();
96    __ReentrancyGuard_init();
97

98    borrowerOperationsAddress = _borrowerOperationsAddress;
99    collSurplusPool = ICollSurplusPool(_collSurplusPoolAddress);
100    defaultPool = IDefaultPool(_defaultPoolAddress);
101    stabilityPoolAddress = _stabilityPoolAddress;
102    vesselManagerAddress = _vesselManagerAddress;
103    vesselManagerOperationsAddress = _vesselManagerOperationsAddress;
104

105    renounceOwnership();
106}

Recommendation:

We advise it to be removed, inheriting the Initializable implementation of OpenZeppelin instead which is properly put in use within the contract.

Alleviation:

While the renunciation has been removed, the OwnableUpgradeable contract is still inherited by the ActivePool. To properly alleviate this exhibit, we advise the OwnableUpgradeable contract to be omitted from the ActivePool entirely.

APL-03C: Redundant Initialization Paradigm

TypeSeverityLocation
Gas OptimizationActivePool.sol:L91-L93

Description:

The ActivePool contract inherits the OpenZeppelin OwnableUpgradeable implementation which contains the Initializable implementation, put in use within the ActivePool::setAddresses function. As such, the manual isInitialized flag is redundant.

Example:

contracts/ActivePool.sol
84function setAddresses(
85    address _borrowerOperationsAddress,
86    address _collSurplusPoolAddress,
87    address _defaultPoolAddress,
88    address _stabilityPoolAddress,
89    address _vesselManagerAddress,
90    address _vesselManagerOperationsAddress
91) external initializer {
92    require(!isInitialized, "Already initialized");
93    isInitialized = true;
94

95    __Ownable_init();
96    __ReentrancyGuard_init();
97

98    borrowerOperationsAddress = _borrowerOperationsAddress;
99    collSurplusPool = ICollSurplusPool(_collSurplusPoolAddress);
100    defaultPool = IDefaultPool(_defaultPoolAddress);
101    stabilityPoolAddress = _stabilityPoolAddress;
102    vesselManagerAddress = _vesselManagerAddress;
103    vesselManagerOperationsAddress = _vesselManagerOperationsAddress;
104

105    renounceOwnership();
106}

Recommendation:

We advise it and its validations to be omitted from the codebase as it is ineffectual and duplicates the purpose of the Initializable::initializer modifier.

Alleviation:

The manual initialization methodology has been removed from the contract as advised.

Prev

Timelock.sol (TKC-M)

Next

AdminContract.sol (ACT-C)

On this page