Assessment reports>Singularity>Discussion>Some specifications are inaccurate

Some natspec comments and interfaces are inaccurate

During review of the codebase, we noticed some places where behavior of the code deviate from the documented behavior.

For example, the NatSpec documentation for the _convertToEthIfNecessary function in the UniswapCoreAssetManager contract documents it as returning address(0) in the WETH conversion case. However, the return value in that case is actually ETH_ADDRESS=0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE.

As another example, the IAssetPool interface contains the following signatures for the setAssetManager and getAssetManager functions:

function setAssetManager(address assetManager) external;

function getAssetManager() external view returns (address);

However, in the implementation in the BaseAssetPool contract, the signatures, and also the name of the function to retrieve asset-manager information is different:

function setAssetManager(
    address assetManager,
    bool registered
) external onlyOwner {
    _assetManagerRegistered[assetManager] = registered;
}

function getAssetManagerRegistration(
    address assetManager
) external view returns (bool) {
    return _assetManagerRegistered[assetManager];
}

This divergence likely arose after a change in the implementation to allow multiple assset managers at the same time.

We recommend ensuring that any specification and documentation stays up to date with the functionality implemented.

Singularity made the changes related to the two examples above in commits and , and informed us that they plan to go through all NatSpec comments before launch to ensure accuracy.

Zellic © 2024Back to top ↑