I think that having a private list of _admins in Controller is redundant. The only place where it is used is when checking if the address is not an admin already:
require(_admins[account] == address(0), "Controller: admin already existed");
this can be effectively replaced with a call to function isAdmin:
require(!isAdmin(account)), "Controller: admin already existed");
Given that isAdmin is public, I agree with the finding, am not convinced this would bring any gas savings (potentially increase gas cost as using a function means a jump), will change to non-critical
Handle
pauliax
Vulnerability details
Impact
I think that having a private list of _admins in Controller is redundant. The only place where it is used is when checking if the address is not an admin already: require(_admins[account] == address(0), "Controller: admin already existed"); this can be effectively replaced with a call to function isAdmin: require(!isAdmin(account)), "Controller: admin already existed");