Openzeppelin-contracts: Refactor code to use composition and delegation over Inheritance

[earizon]

🧐 Motivation
I'm trying to use the RBAC libraries of openzeppelin-solidity and I found they are using (abusing?) from inheritance.

📝 Details
I'm trying to create a controller in charge of RBAC, error-handing or action routing. Using inheritance I'm forced to make the controller inherit from many different classes and programming looks un-natural.

By promoting composition-and-delegation over inheritance the implementation would look much more natural. For example the controller could just "pick" the needed RBAC classes at will and delegate to them for access control.

Some related refs:

• https://en.wikipedia.org/wiki/Composition_over_inheritance
• https://medium.com/rocket-fuel/kotlin-by-class-delegation-favor-composition-over-inheritance-a1b97fecd839

(IMHO, inheritance is just a design mistake that cause problems and future maintenance issues).

[nventuro]

We've discussed this multiple times, and made efforts to switch to a composition-based architecture. This isn't always easy to achieve however, and usually comes with added gas costs.

Do you have specific recommendations for the RBAC scenario you could share?

[biocrypto730]

i think what you need is a "lean" codebase designed for gas (inheritance, no refactoring strings, minimized comments), and a beginner codebase with comments / refactoring strings / most things in one file.... I don't know

[frangio]

Hi @earizon. Could you please share some code for what you're trying to do?

Roles are actually quite good for composition, because you can compose functionality by assigning the role to a contract instance. This allows you for example to create a minting scheme without modifying ERC20Mintable at all, with minimal use of inheritance. Check out this guide for an example of this kind of composition.

[earizon]

@frangio , @nventuro , @projectoblio :

Correcting myself. Digging further I found a code like next one will (so I think) work with current RBAC implementation. All that needed is a very simple, intermediate step to create new classes inheriting from the RBAC ones and then continue to use the composition/delegation pattern as I'm used to:

  contract WhiteListAdmins is WhileListAdminRole { } // <- ** Inherit from Existing role **
  contract Role1Signers is SignerRole { }
  contract Role2Signers is SignerRole { } 

  contract Controller {
      WhiteListAdmins whiteListAdmins;
      Role1Signers role1Signers;
      Role2Signers role2Signers;
      ...          
      constructor() public {
          whiteListAdmins = new whiteListAdmins();
          role1Signers    = new Role1Signers();
          role2Signers    = new Role2Signers();
          emit WhileListAdminsCreated(whiteListAdmins,...)
          emit role1SignersCreated(role1Signers,...)
          emit role1SignersCreated(role2Signers,...)
      }   
      function addAdminRole() public  {
          if (! whiteListAdmins.isWhitelistAdmin(msg.sender)) { // <-- **Delegate** 
          // ... handleError....
        }
        function actionRequiringRole1Signature() public {
           if (!role1Signers.isSigner(msg.sender)) {                    // <-- **Delegate**
              // ... handleError...
           }
      }   
 }

RBAC function modifiers are lost (but that's a minor issue for me).

Do you see any problem in the long term with this approach?

A (not so important) problem I observe is that now I have to control 4 contracts for event emission (controller, whiteListAdmins, Role1/2 Signers). Ideally I would like to put all event emissions in a single contract. Maybe adding a flag to indicate whether emit on RBAC or the controller instance is just enough or maybe it's better to leave it as it is for some reason Im not aware of.

(Regarding gas-problems, I'm optimistic to think that current gas-limits are temporal ones while scalability technology improves through pegged chains, sharding, PoS, ... )

[biocrypto730]

that looks great. but in regards to scalability, the protocol is named casper for a reason. As it becomes cheaper to transact on a blockchain, more people will use it. It's a never ending cycle (i.e. casper the friendly ghost). If blockchain becomes cheaper to use, more people will use it, only increasing fees back to where they are about now (or higher). if you want to have an edge over your competitors design for the cheapest possible blockchain protocol.