dforce-network / dToken

dForce yield token
MIT License
31 stars 10 forks source link

Modifications to AdminUpgradabilityProxy #31

Closed mgcolburn closed 4 years ago

mgcolburn commented 4 years ago

Severity: Code Quality

Description

The DTokenProxy contract is an implementation of the OpenZeppelin AdminUpgradeabilityProxy. The following lines have been commented out from that parent contract:

 modifier ifAdmin() { 
     if (msg.sender == _admin()) { 
         _; 
     } /* else { 
     _fallback(); 
     }*/ 
 } ```
  // require(msg.sender != _admin(), "Cannot call fallback function from the proxy admin"); 

By having the _fallback() call included in the modifier, it allows the implementations to define functions such as admin() and pendingAdmin(). Currently, these functions would be captured by the proxy instead of being delegated to the underlying implementation.

The reason for this modification is unclear. If it provides some benefit, this should be documented. Otherwise introducing modifications like this can have unintended side effects.