etoroxlabs / etoken

eToken is stablecoin implementation by eToro targeting the Ethereum platform

eToken - an eToro stablecoin

eToken is stablecoin implementation by eToro targeting the Ethereum platform.

USAGE

To test the library and setup the development environment, issue the following commands in a shell:

  yarn install
  yarn test # compile and test

Prerequisites

You need to have yarn and node installed. This repository has only been tested on UNIX-derived systems.

Design overview

See separate document.

FILES

Path	Description
contracts/	All the solidity files making up the implementation
contracts/token	Contains the eToken implementation
contracts/token/ERC20	ERC20 implementation using an external storage
contracts/roles	Defines the roles implementation, i.e. whitelisting, blacklisting, miners etc.
contracts/lifecycle	Implements lifecycle behaviors. Taken from OpenZeppelin
contracts/mocks	Contracts used specifically for testing purposes
test/	Contains testing code in JavaScript
scripts/	Specific scripts for testing, coverage & upgrading tokens

etoken's Issues

How to handle permissions?

In relation to my work in #9, I have been looking a bit more into how to handle access control using the facilities provided by OpenZeppelin 2.0 (OZ). I have a bit of a hard time understanding the motivations for the redesign of the OZ system and I doubt that it is suitable for our use case. This is not made easier by the fact that the current OZ documentation still refers to the removed RBAC module.

There are two main issues with the new OZ approach to access control which require us to reconsider if we need to, at least partially, roll our own access control system:

• The OZ 2.0 contracts enforce that permissions are stored inside the contract they govern. For example, the functions provided by ERC20Mintable contract is hard coded to allow access based on the MinterRole provided by OZ. Therefore, using this system, we would need to specify who is allowed to mint (and other related actions) on a per-token basis. As long as we maintain the whitelist in a single contract which is common to all tokens, this might be acceptable for us. However, it means that it is impossible for us to use parts of the OZ ERC20 implementation (such as ERC20Mintable) without also using the OZ access control systems.
• The predefined OZ roles does not support any kind permission hierarchy meaning that (using MinterRole as an example):
  • The creator of the MinterRole contract also becomes the first minter.
  • Only minters can add other minters
  • The only way permissions can be removed are by the permission holders renouncing their role.
    That means that there is no way for an owner or administrator to add or remove other minters.

Therefore, I think we need to revisit the issue of how to handle access control.

I suggest one of the following:

• We implement our own centralized access control contract which handles hierarchical permissions. If we do this, we need to rewrite small parts of the OZ ERC20 implementation (such as the mint function) to not depend on the built in OZ roles.
• We stick to the new OZ access control paradigm as closely as possible. In this scenario, we would only use a central access control contract for the whitelist and continue the OZ practice of managing access control on a per-contract basis for all other roles. As with the previous option, we need to rewrite parts of the OZ ERC20 implementation and change structure of the OZ roles such that they implement a notion of hierarchy by allowing a superior to both grant and revoke subordinate permissions.

What do you think?

Minor changes regarding ETokenExplicitSender.sol (ETokenExplicitSender.sol)

Review comments for public functions in EToken.sol

For example, the comment for allowance does not mention anything about the double spend attack. We should check if some of juicy details are hidden away in the comments in ERC20.sol

Avoid account permission enforcing modifiers in ExplicitSender functions

This is potentially problematic as we may wish to upgrade these later as pointed out by @peteremiljensen

Reporting on legal opinion and MiFIDII trade-offs.

Update the docs with the new minting restriction and it's idea

We should update the docs with the new minting restriction and its idea

Originally posted by @peteremiljensen in #51

Finish comments

Following is a list of commenting tasks that need to be done:

• Make sure every method is properly commented
• Add contract descriptions
• Remove "zeppelin/missing-natspec-comments": ["off"] from .soliumrc.json to enforce future comments

Refactor to remove redundant functions

Hopefully, this will reduce the amount of gas needed for contract deployment

Get upgrade address from ENS domain name

Approval events should be fired for transferFrom() and burnFrom() (ExternalERC20.sol)

Unlocked Pragma

Centralization of Power (ExternalERC20Storage.sol)

Proxies calls to an upgraded token do not chain multiple upgrades (EToken.sol)

Allowance Double-Spend Exploit (ExternalERC20.sol)

Branch naming

What about using the following naming convention for branches:

If you are the sole developer of a branch, then name it: my-name/branch-description, for example
peter/token-dynamic-creation.

If multiple works on the same functionality, then it is called features/token-dynamic-creation. Each individual is working on their own my-name/features/token-dynamic-creation (e.g. peter/features/token-dynamic-creation) and pull request into features/token-dynamic-creation for each independent work. And finally when the feature is done, then pull request into the master branch.

Does that make sense?

Redundant external storage (EToken.sol)

The function latchInitialImplementor() is callable separately from deployment (ExternalERC20Storage.sol)