Extending ERC20 with token locking capability

[nitika-goel]

EIP: 1132
Title: Extending ERC20 with token locking capability
Author: nitika-goel <nitika@govblocks.io>
Type: Standards Track
Category: ERC
Status: Draft
Created: 2018-06-03

Simple Summary

An extension to the ERC20 standard with methods for time-locking of tokens within a contract.

Abstract

This proposal provides basic functionality to time-lock tokens within an ERC20 smart contract for multiple utilities without the need of transferring tokens to an external escrow smart contract. It also allows fetching balance of locked and transferable tokens.

Time-locking can also be achieved via staking (#900), but that requires transfer of tokens to an escrow contract / stake manager, resulting in the following six concerns:

1. additional trust on escrow contract / stake manager
2. additional approval process for token transfer
3. increased ops costs due to gas requirements in transfers
4. tough user experience as the user needs to claim the amount back from external escrows
5. inability for the user to track their true token balance / token activity
6. inability for the user to utilize their locked tokens within the token ecosystem.

Motivation

dApps often require tokens to be time-locked against transfers for letting members 1) adhere to vesting schedules and 2) show skin in the game to comply with the underlying business process. I realized this need while building Nexus Mutual and GovBlocks.

In Nexus Mutual, claim assessors are required to lock their tokens before passing a vote for claims assessment. This is important as it ensures assessors’ skin in the game. The need here was that once a claim assessor locks his tokens for ‘n’ days, he should be able to cast multiple votes during that period of ‘n’ days, which is not feasible with staking mechanism. There are other scenarios like skills/identity verification or participation in gamified token curated registries where time-locked tokens are required as well.

In GovBlocks, I wanted to allow dApps to lock member tokens for governance, while still allowing members to use those locked tokens for other activities within the dApp business. This is also the case with DGX governance model where they’ve proposed quarterly token locking for participation in governance activities of DGX.

In addition to locking functionality, I have proposed a Lock() and Unlock() event, just like the Transfer() event , to track token lock and unlock status. From token holder’s perspective, it gets tough to manage token holdings if certain tokens are transferred to another account for locking, because whenever balanceOf() queries are triggered on token holder’s account – the result does not include locked tokens. A totalBalanceOf() function intends to solve this problem.

The intention with this proposal is to enhance the ERC20 standard with token-locking capability so that dApps can time-lock tokens of the members without having to transfer tokens to an escrow / stake manager and at the same time allow members to use the locked tokens for multiple utilities.

Specification

I’ve extended the ERC20 interface with the following enhancements:

Locking of tokens

/**
  * @dev Locks a specified amount of tokens against an address,
  *      for a specified reason and time
  * @param _reason The reason to lock tokens
  * @param _amount Number of tokens to be locked
  * @param _time Lock time in seconds
  */
function lock(bytes32 _reason, uint256 _amount, uint256 _time) public returns (bool)

Fetching number of tokens locked under each utility

/**
  * @dev Returns tokens locked for a specified address for a
  *      specified reason
  *
  * @param _of The address whose tokens are locked
  * @param _reason The reason to query the lock tokens for
  */
   tokensLocked(address _of, bytes32 _reason) view returns (uint256 amount)

Fetching number of tokens locked under each utility at a future timestamp

/**
  * @dev Returns tokens locked for a specified address for a
  *      specified reason at a specific time
  *
  * @param _of The address whose tokens are locked
  * @param _reason The reason to query the lock tokens for
  * @param _time The timestamp to query the lock tokens for
  */
  function tokensLockedAtTime(address _of, bytes32 _reason, uint256 _time) public view returns (uint256 amount)

Fetching number of tokens held by an address

/**
  * @dev @dev Returns total tokens held by an address (locked + transferable)
  * @param _of The address to query the total balance of
  */
function totalBalanceOf(address _of)  view returns (uint256 amount)

Extending lock period

/**
  * @dev Extends lock for a specified reason and time
  * @param _reason The reason to lock tokens
  * @param _time Lock extension time in seconds
  */
  function extendLock(bytes32 _reason, uint256 _time) public returns (bool)

Increasing number of tokens locked

/**
  * @dev Increase number of tokens locked for a specified reason
  * @param _reason The reason to lock tokens
  * @param _amount Number of tokens to be increased
  */
  function increaseLockAmount(bytes32 _reason, uint256 _amount) public returns (bool)

Fetching number of unlockable tokens under each utility

/**
  * @dev Returns unlockable tokens for a specified address for a specified reason
  * @param _of The address to query the the unlockable token count of
  * @param _reason The reason to query the unlockable tokens for
  */
  function tokensUnlockable(address _of, bytes32 _reason) public view returns (uint256 amount)

Fetching number of unlockable tokens

/**
  * @dev Gets the unlockable tokens of a specified address
  * @param _of The address to query the the unlockable token count of
  */
  function getUnlockableTokens(address _of) public view returns (uint256 unlockableTokens)

Unlocking tokens

/**
  * @dev Unlocks the unlockable tokens of a specified address
  * @param _of Address of user, claiming back unlockable tokens
  */
  function unlock(address _of) public returns (uint256 unlockableTokens)

Lock event recorded in the token contract

event Lock(address indexed _of, uint256 indexed _reason, uint256 _amount, uint256 _validity)

Unlock event recorded in the token contract

event Unlock(address indexed _of, uint256 indexed _reason, uint256 _amount)

Implementation

• Complete implementation available at https://github.com/nitika-goel/lockable-token
• GovBlocks Project specific implementation available at https://github.com/somish/govblocks-protocol/blob/Locking/contracts/GBTStandardToken.sol

[davesag]

@nitika-goel I like this. I was just about to sit down and write an extension to ERC-884 to add time-locking in order to introduce Reg-D compatibility. Your proposal handles this quite elegantly.

In your reference implementation I don't understand line 67. I would have expected the 'purpose' to be a bytes32 or string to provide an human readable reason for locking the token. But you have defined _for as an int and then explicitly require that int to be less than a contact-wide locked_for property. I'm not clear on the rationale for that.

https://github.com/nitika-goel/lockable-token/blob/master/contracts/LockableToken.sol#L67

57 /**
58  * @dev Locks a specified amount of tokens against an address, for a specified purpose and time
59  * @param _for The purpose to lock tokens 
60  * @param _amount Number of tokens to be locked
61  * @param _time Lock time in seconds
62  */
63 function lock(uint256 _for,uint256 _amount,uint256 _time)
64 {
65  uint256 validUntil=block.timestamp.add(_time);
66  require(_amount <= transferableBalanceOf(msg.sender));
67  require(_for<locked_for);
68  locked[msg.sender][_for].push(lockToken(_amount,validUntil));
69  Lock(msg.sender,_for,_amount,validUntil);
70   
71 }

It would also help if the reference implementation had some unit-tests that demonstrate various usage scenarios.

[nitika-goel]

Thanks for your views @davesag.

Explaining line 67

The purpose of line 67 in implementation sample is to ensure that tokens are only locked for utilities allowed by a dApp. The variable locked_for is the number of utilities for which a token can be locked (apologies for the bad variable naming). So for example, if a dApp allows 2 utilities i.e claims assessment and governance, then locked_for = 2 and at all times _for should be less than 2, because if _for >= 2 then those tokens would be shown as locked in tokensLocked(). At the same time these tokens shall not be counted in transferableBalanceOf() and hence would be available for transfer, resulting in a chaos.

Implementing bytes32 for _for

Thanks for pointing that out. Originally, even I started writing the interface with _for as a bytes32, but the implementation wasn't elegant enough due to complexity around checking valid utility.

However, I came up with a better idea and have implemented _for as bytes32, branched at https://github.com/nitika-goel/lockable-token. The new contract is also available at 0xe4c32329de48334c8b635ccb6c4cf1fdbb7d1829 on Kovan testnet. Let me know what you think.

Test Cases

Will write test cases and submit soon on repo.

[davesag]

Hi @nitika-goel — I've submitted a PR for you for your reference implementation with some general code cleanups and some tests, hope you find it useful. I think the bytes32 idea works better.

[thekrol01]

Hi @nitika-goel,
Great proposal. Though I am a bit hesitant with altering the ERC20/ERC223 standard functions.
But If you don't want to escrow the tokens this could be the way to go.
I have not tested this but doesn't this make a standard transfer much more expensive, because every transfer is tested against the available balance for transfer. Therefore splitting the lock function from the not locked transfers could be cheaper for the regular transaction which don' t have a locked balance.
Why not ' transfer' the locked tokens to another balance (mapping) within the token contract where only the holder of the locked tokens has control of. In that way you can leave the current transfer as it is, because the locked tokens are extracted from the token balance and added to the locked balance. Because the user is in complete control the trust issue is not relevant anymore. When I look at your concerns only point 4 will still be there in some extend, but because of the complete control I wonder if the trust issue will stil be there.

[vongohren]

Great proposal, was in any more feedback on this @nitika-goel

[maxsam4]

I agree with @thekrol01 that we should refrain from editing the standard function. nitika-goel/lockable-token#5 is an updated implementation which kinda works as an interface without touching the standard functions.

An example of a token that implements this is https://github.com/somish/govblocks-protocol/blob/master/contracts/GBTStandardToken.sol

We can't expect the existing tokens to use this interface so I believe a proxy like token will be helpful which will transfer the "non-lockable" token from the user to itself when the user locks the token and transfer back the token to the user once they claim the token. A sample implementation can be found at
https://github.com/somish/govblocks-protocol/blob/master/contracts/TokenProxy.sol

[davesag]

@nitika-goel You've since addressed the issues cited above. Maybe update this Issue with the latest interface and raise a PR in regards to this issue.

[nitika-goel]

Thanks all of you. I've now updated the code with ideas from @davesag, @thekrol01 and @maxsam4! Changed the interface above to make sure ERC20 functions aren't changed. New functions are added to introduce manual unlocking of tokens - it kind of adds an extra step, but probably worth it. Submitting a PR now.

[m888m]

Great initiative, and thank you all for the work!

Suggestion:
To keep the interface maximum compatible, don't specify

• lockReason
• lockToken
• locked

in the interface, but keep this for the actual contracts that implement the interface.

Reason: We should not presume the inherent data structures of the implementing contracts. E.g., a contract that uses a flat file data storage architecture will not implement these variables in the same way.

[m888m]

I've started to implement the interface in our project.

Observation:

The interface is way too specific imo.
Many of the functions defined are actually aggregations of basic functions.
This should not be implemented in a library, but in the business logic contract that implements the interface.

Interfaces should be designed to reflect the smallest common denominator.
Hence my suggestion is to radically slim down the interface, and to really focus on the core (base) functionalities that exist in EVERY time locked token.

[nitika-goel]

Thanks for your feedback @m888m. The interface targets interoperability of tokens and reusability within the same application, for multiple utilities. Imagine a stable coin implements this interface, all users using this stable token across different applications can lock their tokens within the same contract with different reasons. A user can then, simply see in 1 contract, all the tokens locked up under different applications. Similarly, as we see more utility tokens coming up, imo, the same token shall be used for different purposes and it will be really useful to lock them in a single contract. I have seen that personally for 2 of my projects.
All data structures you mentioned are not intended to be a part of the interface, and are not mentioned in the EIP. I think ERC1132.sol in the in the impelementation repo is causing the confusion. Happy to make changes there.

The functions defined in the interface are those that shall be needed to facilitate interoperability and reusability. If an application has a single lock reason, it can always implement lock under 1 default reason. Unable to understand which functions are you referring to as aggregations of other functions. Feedback is always welcome!

[ray-hash]

    function unlock(address _of)
        public
        returns (uint256 unlockableTokens)
    {
        uint256 lockedTokens;

        for (uint256 i = 0; i < lockReason[_of].length; i++) {
            lockedTokens = tokensUnlockable(_of, lockReason[_of][i]);
            if (lockedTokens > 0) {
                unlockableTokens = unlockableTokens.add(lockedTokens);
                locked[_of][lockReason[_of][i]].claimed = true;
                emit Unlock(_of, lockReason[_of][i], lockedTokens);
            }
        }  

        if(unlockableTokens > 0)
        	this.transfer(_of, unlockableTokens);
    }

Unlock may not be possible due to loop attack. fee limit exceeded.
Need a defense.

    function unlockAll(address _of)
        public
        returns (uint256 unlockableTokens)
    {
        uint256 lockedTokens;

        for (uint256 i = 0; i < lockReason[_of].length; i++) {
            lockedTokens = tokensUnlockable(_of, lockReason[_of][i]);
            if (lockedTokens > 0) {
                unlockableTokens = unlockableTokens.add(lockedTokens);
                locked[_of][lockReason[_of][i]].claimed = true;
                emit Unlocked(_of, lockReason[_of][i], lockedTokens);
            }
        }

        if (unlockableTokens > 0)
            this.transfer(_of, unlockableTokens);
    }

    function unlock(address _of, bytes32 _reason)
        public
        returns (uint256 unlocked)
    {
        unlocked = tokensUnlockable(_of, _reason);
        if (unlocked > 0) {
            locked[_of][_reason].claimed = true;
            emit Unlocked(_of, _reason, unlocked);
            this.transfer(_of, unlocked);
        }
    }

[nitika-goel]

Great suggestion @RyoungHo . While I believe that the gas limit is good enough to support a lot of reasons and the code should not break in a practical scenario, we should still include this as a part of the interface.
However, there are tokens like Nexus Mutual https://github.com/somish/NexusMutual, which are using the interface in main-net and it would be great to keep them compatible with the interface as well.
How about the following:

1. unlock(address _of)
2. unlockReason(address _of, bytes32 _reason)

[davesag]

Great suggestion @RyoungHo . While I believe that the gas limit is good enough to support a lot of reasons and the code should not break in a practical scenario, we should still include this as a part of the interface.
However, there are tokens like Nexus Mutual https://github.com/somish/NexusMutual, which are using the interface in main-net and it would be great to keep them compatible with the interface as well.
How about the following:

1. unlock(address _of)
2. unlockReason(address _of, bytes32 _reason)

Hi @nitika-goel I think it's risky to use this interface in Main Net until it's been accepted. Right now this is still an open proposal.

That said, I agree it's ideal to keep the interface changes to a minimum.

I do like the terminology unlockAll and perhaps unlockOne however.

[nitika-goel]

Definitely @davesag. Nexus has used their own implementation of the EIP. They have just maintained the interface, which I think we should continue. Their contracts were audited by a third party too.

Do you suggest the following?

1. unlock(address _of)
2. unlockOne(address _of, bytes32 _reason)

[davesag]

@nitika-goel that makes sense to me.

[ray-hash]

Definitely @davesag. Nexus has used their own implementation of the EIP. They have just maintained the interface, which I think we should continue. Their contracts were audited by a third party too.

Do you suggest the following?

1. unlock(address _of)
2. unlockOne(address _of, bytes32 _reason)

Nexus will have to change the contract.
very dangerous.

If the attacks the locked whale, the address will be locked.

---

Oh, Nexus doesn't have the transferWithLock function. :)

[nitika-goel]

That's right @RyoungHo , transferWithLock is not a part of the interface. Also, similar to unlockOne(), they have the function releaseLockedTokens, which unlocks tokens for individual reasons.

Hence, I don't see the issue in their implementation.

[github-actions]

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

[github-actions]

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.