Omniscia Gravita Protocol Audit

VesselManager Code Style Findings

VesselManager Code Style Findings

VMR-01C: Inefficient mapping Lookups

TypeSeverityLocation
Gas OptimizationVesselManager.sol:L227-L228, L281-L283, L417-L418, L420, L422, L426-L427, L429, L504-L505, L522-L523, L533-L535, L541-L542, L547-L549, L554-L556, L558-L559, L594-L596, L598-L599, L610, L613, L619, L621-L622, L625, L692-L693, L701-L702, L711-L712, L721-L722, L731, L737

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

contracts/VesselManager.sol
500function _getCurrentVesselAmounts(address _asset, address _borrower) internal view returns (uint256, uint256) {
501    uint256 pendingCollReward = getPendingAssetReward(_asset, _borrower);
502    uint256 pendingDebtReward = getPendingDebtTokenReward(_asset, _borrower);
503

504    uint256 currentAsset = Vessels[_borrower][_asset].coll.add(pendingCollReward);
505    uint256 currentDebt = Vessels[_borrower][_asset].debt.add(pendingDebtReward);
506

507    return (currentAsset, currentDebt);
508}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

Alleviation:

All inefficient mapping lookups have been significantly optimized per our recommendation, rendering this exhibit fully alleviated.

VMR-02C: Redundant Data Point

TypeSeverityLocation
Gas OptimizationVesselManager.sol:L692

Description:

The address type asset data point present in each Vessel struct is redundant as the Vessel requires the _asset key to be accessed.

Example:

contracts/VesselManager.sol
687function setVesselStatus(
688    address _asset,
689    address _borrower,
690    uint256 _num
691) external override onlyBorrowerOperations {
692    Vessels[_borrower][_asset].asset = _asset;
693    Vessels[_borrower][_asset].status = Status(_num);
694}

Recommendation:

We advise the data point to be safely omitted as it is not utilized within the contract.

Alleviation:

The asset data point has been safely removed from the data entry.

VMR-03C: Redundant External Self-Call

TypeSeverityLocation
Gas OptimizationVesselManager.sol:L238

Description:

The referenced statement performs an external call to self via the this.getVesselStatus syntax redundantly.

Example:

contracts/VesselManager.sol
237function isVesselActive(address _asset, address _borrower) public view override returns (bool) {
238    return this.getVesselStatus(_asset, _borrower) == uint256(Status.active);
239}

Recommendation:

We advise the VesselManager::getVesselStatus function to be set as public and the call to be made "internally" by removing the this call prefix.

Alleviation:

The redundant self-call has been replaced by an "internal" call of its public function as advised.

VMR-04C: Redundant Initialization Paradigm

TypeSeverityLocation
Gas OptimizationVesselManager.sol:L134-L136

Description:

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

Example:

contracts/VesselManager.sol
124function setAddresses(
125    address _borrowerOperationsAddress,
126    address _stabilityPoolAddress,
127    address _gasPoolAddress,
128    address _collSurplusPoolAddress,
129    address _debtTokenAddress,
130    address _feeCollectorAddress,
131    address _sortedVesselsAddress,
132    address _vesselManagerOperationsAddress,
133    address _adminContractAddress
134) external override initializer {
135    require(!isInitialized, "Already initialized");
136    isInitialized = true;
137    __Ownable_init();
138    borrowerOperations = _borrowerOperationsAddress;
139    stabilityPool = IStabilityPool(_stabilityPoolAddress);
140    gasPoolAddress = _gasPoolAddress;
141    collSurplusPool = ICollSurplusPool(_collSurplusPoolAddress);
142    debtToken = IDebtToken(_debtTokenAddress);
143    feeCollector = IFeeCollector(_feeCollectorAddress);
144    sortedVessels = ISortedVessels(_sortedVesselsAddress);
145    vesselManagerOperations = IVesselManagerOperations(_vesselManagerOperationsAddress);
146    adminContract = IAdminContract(_adminContractAddress);
147}

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-C)

Next

VesselManagerOperations.sol (VMO-C)

On this page