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.