Maia DAO - Ulysses - audityourcontracts's results

[01] setCoreRouter in Branchport.sol can be removed

setCoreRouter in Branchport.sol appears to be dead code. Although it requires the CoreRouter to call it using the modifier requiresCoreRouter there is no reference to this in any of the router contracts. The functionality seems to have been migrated to setCoreBranchRouter in Branchport.sol.

File: src/BranchPort.sol

331      function setCoreRouter(address _newCoreRouter) external override requiresCoreRouter {
332          require(coreBranchRouterAddress != address(0), "CoreRouter address is zero");
333          require(_newCoreRouter != address(0), "New CoreRouter address is zero");
334          coreBranchRouterAddress = _newCoreRouter;
335:     }

Recommendation

The setCoreRouter function can be removed from Branchport.sol and the function can also be removed from the interface IBranchPort.sol.

[02] Arrays are pushed to but are never referenced

strategyTokens and bridgeAgents in BranchPort are appended to arrays that are never used.

Recommendation

Remove these arrays as storage variables and remove the .push functions so they are not appended to.

[03] manageStrategyToken doesn't check value is within range

manageStrategyToken allows the _minimumReservesRatio to be set for new strategy tokens that are added to a Branch Port. This is passed to a branch and eventually executes addStrategyToken() where the _minimumReservesRatio is required to be <= the DIVISIONER (1e4) and >= MIN_RESERVE_RATIO (3e3) otherwise the function will revert.

Recommendation

This check could be performed at the source manageStrategyToken avoiding a cross chain call to set a value that may end up reverting, costing gas and then needing to be called again.