search

OpenZeppelin / openzeppelin-labs

A space for the community to interact and exchange ideas on the OpenZeppelin platform. Do not use in production!
https://openzeppelin.com/
MIT License
374 stars 115 forks source link

Design legacy token onboarding methodology #19

Closed maraoz closed 6 years ago

maraoz commented 6 years ago

Look for strategies for migrating "v1" tokens to upgradeable tokens, to onboard token users to zOS. We can also work with exchanges on this to ease the process for users.

adklempner commented 6 years ago

I thought of a simple opt-in strategy.

The new token contract starts with no initial supply and no balances. The only way to "mint" the new tokens is for users to "turn in" their old ones. This is done by first approving the amount they want to migrate via OldERC20.approve(newTokenAddress, amountToMigrate) and then calling a function in the new token called migrateTokens that would look something like this:

function migrateTokens(uint amount) public {
    require(ERC20(oldERC20Address).transferFrom(msg.sender, burnAddress, amount));
    _balances[msg.sender] = _balances[msg.sender].add(amount);
    _totalSupply = _totalSupply.add(amount);
  }

The old tokens are sent to a burn address, and the holder receives an equal amount in the new contract.

Pros of this strategy:

Cons:

AugustoL commented 6 years ago

@adklempner nice ! I think the token will need to implement this solution if there is no control from an owner over it, if the token can be paused in some way the tokens can be migrated with a contract like:

contract MigrateToken is Ownable {

  ERC20 public fromToken;
  Token_V0 public toToken;
  mapping(address => bool) public balanceMigrated;

  function MigrateToken(address _fromToken) {
    fromToken = ERC20(_fromToken);
  }

  function startMigration(address _toToken) onlyOwner {
    require(Token_V0(_toToken).owner() == address(this));
    toToken = Token_V0(_toToken);
  }

  function migrateBalances(address[] addrs) onlyOwner {
    require(toToken != address(0));

    for(uint8 i = 0; i < addrs.length; i++) {
      require(!balanceMigrated[addrs[i]]);
      require(fromToken.balanceOf(addrs[i]) > 0);

      toToken.mint(addrs[i], fromToken.balanceOf(addrs[i]));
      balanceMigrated[addrs[i]] = true;
    }
  }

  function finishMigration() onlyOwner {
    toToken.transferOwnership(owner());
  }

}

The solution that Arsenyi presents is better for a more "democratic" decision to upgrade or not, I guess it depends on each token, his features and needs.

@adklempner what do you think on adding an address parameter to the function too? instead of using msg.sender, this will allow a user from an old token to send tokens to a user that wants new tokens only, also it will remove the need of migrate the tokens myself, and maybe the token maintainers can do it. Also use address 0x as default burn address, with this you are sure that the tokens are burned. 

function migrateTokens(uint amount, address from) public {
    require(ERC20(oldERC20Address).transferFrom(from, address(0), amount));
    _balances[msg.sender] = _balances[from].add(amount);
    _totalSupply = _totalSupply.add(amount);
  }
maraoz commented 6 years ago

@adklempner love your proposal, please send a PR for review :) @AugustoL agree on using 0x to burn them :fire:

adklempner commented 6 years ago

If the token implements OZ's StandardToken then we can't use address(0) as a destination for transfers. Or is 0x different from address(0)?

facuspagnuolo commented 6 years ago

@adklempner really like the idea! @AugustoL agree too, but bear in mind many token contracts don't allow transfers to the zero address, we will need them to be burnable in that case.

Regarding the migrateTokens function with the from parameter, we can actually provide both ways :)

adklempner commented 6 years ago

Created a PR #24 that demonstrates this approach.

I did not add a migrateTokens with a from parameter yet. Since the caller of OldERC20.transferFrom() is the new token, token holders have to approve the address of the new token to transfer. In your example, @AugustoL , any caller could migrate tokens to themselves by passing any address to from that has approved tokens, whether the tokens were meant for them or not.

I think a to parameter might make more sense, but it's behavior is a little different than what you described:

function migrateTokens(uint amount, address to) public {
    require(ERC20(oldERC20Address).transferFrom(msg.sender, address(0), amount));
    _balances[to] = _balances[to].add(amount);
    _totalSupply = _totalSupply.add(amount);
  }

In this case, the caller can transfer new tokens to someone else, but only after calling OldERC20.approve(newToken, amount)

AugustoL commented 6 years ago

@adklempner yes, thats another way, we can add examples of all implementations, they are simple and therefore more secure and easy to test.

Regarding contracts that cant burn tokens we can create a Burn contract, this contract wont have any logic and it wont be able to use received tokens, so it will be the same as using address(0), where the totalSupply dont decrease and tokens are unusable .

adklempner commented 6 years ago

Yup, that's how I implemented it, using an empty contract whose address is set in the new token's initialize function. Also added a migrateTokensTo function, that migrates the callers tokens to a different address. Not sure if there's a clear, secure way for a user to migrate old tokens from another address they've been approved to use. What makes most sense to me is just using the old token's methods to transfer those tokens to themselves and then approving the new token to use them.

Other than that, the more I think of this migration method the more I like it. Are there any other cons you can think of?

maraoz commented 6 years ago

After reviewing all previous work and discussions, I see 2 groups of approaches: 1) Migration is fully controlled and paid for by the development team In this case, the dev team can deploy an upgradeable MintableToken instance, and migrate the token balances of all users in a centralized way. During the migration, the new token will not have any additional functionality the legacy token had, as it'd be a plain vanilla MintableToken. After the balance migration is over, the devs can then upgrade the token to a new implementation, which contains the (optionally modified) full behavior of the legacy token. At this point, the token is migrated to an upgradeable version with the same functionality as the legacy token.

This means more control from the devs is required, and may not be desirable for all projects (for legal, community concerns, or project management reasons). The upside of this approach is that users don't need to pay the migration gas costs.

See how augur migrated their old Serpent token to the solidity version for an analogy (they didn't migrate to an upgradeable version, but there's lots of lessons to be learnt from their experience centrally migrating a token to a new version). https://github.com/AugurProject/augur-core/blob/6b3f7e9e9106a6079ffd4bae0dd7d89edcce0964/source/contracts/legacy_reputation/LegacyRepToken.sol#L41

2) Migration is performed and paid for by the token holders Lazy, opt-in, migration, proposed by @adklempner above. In this case, the developers of the token deploy a new, upgradeable version, which provides a lazy migration mechanism for users to move their balances to the new upgradeable version. This means that balances are not moved from the legacy token to the new one at once, but each user chooses if/when to do it.

For more details, see Arseniy's comments above (https://github.com/zeppelinos/labs/issues/19#issuecomment-363531540) and the implementation in this repo (https://github.com/zeppelinos/labs/tree/master/migrating_legacy_token/contracts)

The drawback of this approach is token users pay for migration gas costs.

In any case, we should also take into account the following scenarios/questions:

Is the token pausable or freezable? If the token is not freezable or pausable by the dev team, the migration must be done with the lazy opt-in migration approach, or the dev team should choose a "freeze" block and then manually copy the balances of the legacy token to the new upgradeable version.

Is the token burnable? If the token is burnable, we can destroy the legacy token supply as the migration progresses. If it isn't, the legacy token will remain in the blockchain.

What happens with tokens held by contracts? If the legacy token is held by contracts (such as EtherDelta, or locking contracts), the opt-in approach may not be viable, because these contracts may not be able to call the migration function. This situation also complicates the operation of the centralized approach, but maybe the development team can refund contract owners with the supply reserved for the team.

maraoz commented 6 years ago

Next steps: