Create a Whitelist and WhitelistedCrowdsale

[nventuro]

The old Whitelist and WhitelistedCrowdsale contracts were removed in #1291.

These contracts made a distinction between a whitelister (the owner) and whitelisted accounts. Using Roles, since an account with a role can add that role to any other account, a whitelisted account would essentially be a whitelister itself. We need to figure out whether or not that's an issue, and if yes, come up with a solution.

[levino]

I am very interested in this. Is there a timeline for further work on this? I need a solution ASAP, so I will start to write my own, but would also be happy to contribute some code to this repo.

[nventuro]

Hey @levino! OpenZeppelin v1.12 (the current release) includes a WhitelistedCrowdsale contract, feel free to use that one! This issue refers to versions past v2.0 (which hasn't yet been released).

[levino]

Thanks for clarifying. I saw the changes but did not know that a new major version was in the making. Will continue to watch progress on the matter.

[frangio]

I think we should distinguish between two problems in the discussion.

The exact problem we ran into is that Whitelist implemented its whitelist with a Role assigned to the whitelisted accounts. When we tried to migrate to the newer Roles library, we realized that it would imply that whitelisted accounts could add other accounts to the whitelist, which was not a feature of the previous Whitelist.

The other problem is more conceptual. For roles in general, we reasoned that it makes sense for role bearers to add the role to other accounts because it was already possible to do it either off-chain (a user doing a favor to another user, or even providing a paid service) or on-chain (by transferring ownership to a forwarder contract); you are already trusting these accounts to begin with. Does the same reasoning make sense for a whitelist? I'd say yes, but it feels weird intuitively. So this made us think that maybe the design of Whitelist was ignoring the reality that whitelisted accounts can effectively give access to whitelisted functionality to other accounts. We thought this could result in insecure contract design by users of OpenZeppelin.

As far as implementation goes, we can easily add a whitelister role, and a set of whitelisted accounts that the whitelisters can modify. What is unclear is if this feature ever makes sense when designing a contract.

The one thing that is clear is that whitelisted accounts should never be able to remove other accounts from the whitelist, whereas it is probably reasonable for whitelisters to have this ability.

[nventuro]

Quoting @mjdietzx from #1291:

I think Whitelist.sol and whitelisted crowd-sales in general are a very common use-case (kyc & aml keep getting stricter 😕) And a whitelisted address being able to add another address to the whitelist wouldn't be acceptable for this use-case. So I would think it's important to be able to implement this well in openzeppelin.

Where does this restriction come from? A paper contract? Also, would whitelisters being able to add other whitelisters be fine?

[mjdietzx]

Just to make sure we're on the same page, kyc / aml checks require an address to be whitelisted before it can own any tokens (ie in a regulated ICO or a regulated asset-backed token / security token). This process involves the investor (user who wants to purchase tokens) to verify their identity and source of funds, at which point their address will be added to a whitelist (by the contract owner / operator). This is required by most (probably all truly) regulated tokens and is getting stricter.

So in our case, the kyc / aml required is extremely strict (fully regulated in Switzerland). We verify our users identity with a kyc / aml check done by a 3rd party specializing in this. Once this process is complete, the final step is we add them to our on-chain whitelist.

Only we can add / remove an address to this whitelist. Otherwise bad things can happen (ie a person gets whitelisted and then whitelists all their friends, etc...) Open zeppelin's implementation of this before 2.0 rc was perfect for us.

When we tried to migrate to the newer Roles library, we realized that it would imply that whitelisted accounts could add other accounts to the whitelist, which was not a feature of the previous Whitelist.

^ Definitely a no-go for this use-case.

I do expect this will be a very common use-case and get more and more common. It's simple enough to do on our own, but seems appropriate for this library to me at least.

[nventuro]

Thanks @mjdietzx, your use case makes perfect sense, and I agree with you in that it'd be great if OZ included such a contract.

I have a couple further questions regarding the regulation process you're facing:

• pre-2.0 rc, a single address (the owner) was in charge of adding and removing addresses from the whitelist. Would it be fine for this address to also be able to designate new whitelister addresses, with these same powers?
• should a whitelister be able to remove an address from the whitelisted set?

[mjdietzx]

Great @nventuro

should a whitelister be able to remove an address from the whitelisted set?

yes, this functionality is required by regulators because in certain cases an address needs to be removed (ie a person is added to an anti-money-laundering (AML) 'blacklist', or a user's passport has expired and they haven't / won't update their kyc info)

pre-2.0 rc, a single address (the owner) was in charge of adding and removing addresses from the whitelist. Would it be fine for this address to also be able to designate new whitelister addresses, with these same powers?

it sounds like a really nice feature for there to be multiple whitelister addresses with power to add / remove addresses to whitelist. although a whitelister being able to add / remove other whitelisters sounds like something we probably wouldn't want for our use-case, and would override. I'd have to think about it a bit more, but seems a bit too much power to give a whitelister and we tend to stick with least-privileged permissions.

that said, I understand it works with what you have in Roles best. and really if we only have one address (our address) as the whitelister this isn't a problem anyways and we have exactly what we had pre v2.

just incase it's interesting, here is our Whitelist.sol pre v2

pragma solidity 0.4.24;

import "openzeppelin-solidity/contracts/ownership/Claimable.sol";
import "openzeppelin-solidity/contracts/access/rbac/RBAC.sol";

contract Whitelist is Claimable, RBAC {
  function grantPermission(address _operator, string _permission) public onlyOwner {
    addRole(_operator, _permission);
  }

  function revokePermission(address _operator, string _permission) public onlyOwner {
    removeRole(_operator, _permission);
  }

  function grantPermissionBatch(address[] _operators, string _permission) public onlyOwner {
    for (uint256 i = 0; i < _operators.length; i++) {
      addRole(_operators[i], _permission);
    }
  }

  function revokePermissionBatch(address[] _operators, string _permission) public onlyOwner {
    for (uint256 i = 0; i < _operators.length; i++) {
      removeRole(_operators[i], _permission);
    }
  }
}

[nventuro]

Removing roles is not something Roles provides by default - the functions are there, but they are internal: you need to make public functions with whatever validation you require for this feature to be available.

Anyway, you could easily disable the ability for the original whitelister (the contract creator) to add other whitelisters, if you wanted to:

contract WhitelisterRoleNoAdd is WhitelisterRole {
  function addWhitelister(address account) public {
    revert();
  }

I'm not 100% sure in which case you'd want to do such a thing though, since you're already trusting the whitelister completely.

[lakamsani]

Hi guys, looks like this didn't make it to the 2.0.0 release announced today. Are you still planning on releasing it in a subsequent update? Any time estimates? Thx.

[nventuro]

Hey @levino, @mjdietzx, @lakamsani, just a heads up: we'll be working on this for the 2.1 release, so it should be out in a couple weeks. Thanks for your interest!

[levino]

Cool, thanks. Could you push a WIP branch? I will happily review and try it out. Need this feature asap.

[nventuro]

@levino please take a look: #1525. @mjdietzx if you could provide feedback regarding the new implementation's compliance of legal requirements, that'd be great!