NavVault Code Style Findings

NVT-01C: Deprecated Revert Pattern

Type	Severity	Location
Code Style		NavVault.sol: • I-1: L170 • I-2: L625

Description:

The revert instances highlighted utilize the legacy revert format that is either empty or contains a text message.

Example:

Copy
170if (block.timestamp < pendingOracleSetAt + ORACLE_TIMELOCK) revert("Oracle update timelocked");

Recommendation:

We advise both instances to have a proper error message declared and utilized, optimizing their off-chain readability.

Alleviation:

The Altura Trade team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NVT-02C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		NavVault.sol: • I-1: L210 • I-2: L239 • I-3: L240 • I-4: L247

Description:

The linked mathematical operation is guaranteed to be performed safely by logical inference, such as surrounding conditionals evaluated in require checks or if-else constructs.

Example:

Copy
205require(amountAssets <= available, "fees:exceeds_accrued");
206
207IERC20 a = IERC20(address(asset()));
208if (a.balanceOf(address(this)) < amountAssets) revert InsufficientLiquidity();
209
210accruedExitFeesAssets = available - amountAssets;

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statement to be wrapped in an unchecked code block thereby optimizing its execution cost.

Alleviation:

The Altura Trade team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NVT-03C: Inexplicable Function Implementation

Type	Severity	Location
Gas Optimization		NavVault.sol:L610-L613

Description:

The referenced function implementation is inexplicable as it is possible to simply transmit EIP-20 assets directly to the NavVault contract, bypassing the re-entrancy and paused state checks the system imposes.

Example:

Copy
610function fundLiquidity(uint256 assets_) external whenNotPaused nonReentrant {
611    if (assets_ == 0) revert ZeroAmount();
612    IERC20(address(asset())).safeTransferFrom(msg.sender, address(this), assets_);
613}

Recommendation:

As a direct transfer is more efficient than a transfer-from operation, we advise the function to be removed and external contracts to manually validate the system's paused state before transferring any funds to it.

Alleviation:

The Altura Trade team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NVT-04C: Redundant Address Check

Type	Severity	Location
Gas Optimization		NavVault.sol:L552

Description:

The referenced address validation is redundant as the ensuing line ensures the r.owner is equal to the msg.sender.

Example:

Copy
552if (r.owner == address(0)) revert BadAddress();
553if (r.owner != msg.sender) revert NotOwner();

Recommendation:

We advise it to be omitted, optimizing the code's gas cost.

Alleviation:

The Altura Trade team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.

NVT-05C: Redundant Address Validation

Type	Severity	Location
Gas Optimization		NavVault.sol:L138

Description:

The referenced address validation is redundant as a function is invoked on the oracle_ which would infer that the address is non-zero via its success.

Example:

Copy
126constructor(
127    IERC20Metadata asset_,
128    string memory name_,
129    string memory symbol_,
130    INavOracle oracle_,
131    address admin_,
132    address operator_,
133    address guardian_,
134    uint256 initialMaxAllowedStaleness,
135    uint256 epochSeconds_
136) ERC20(name_, symbol_) ERC4626(asset_) {
137    if (
138        address(oracle_) == address(0) ||
139        admin_ == address(0) ||
140        operator_ == address(0) ||
141        guardian_ == address(0) ||
142        epochSeconds_ == 0
143    ) revert BadAddress();
144
145    uint256 cap = oracle_.maxOracleStaleness();

Recommendation:

We advise the validation to be omitted, optimizing the code's gas cost.

Alleviation:

The Altura Trade team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase.