-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extending ERC20 with token locking capability #1132
Comments
@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 https://github.com/nitika-goel/lockable-token/blob/master/contracts/LockableToken.sol#L67
It would also help if the reference implementation had some unit-tests that demonstrate various usage scenarios. |
Thanks for your views @davesag. Explaining line 67The purpose of line 67 in implementation sample is to ensure that tokens are only locked for utilities allowed by a dApp. The variable Implementing bytes32 for
|
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 |
Hi @nitika-goel, |
Great proposal, was in any more feedback on this @nitika-goel |
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 |
@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. |
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. |
Great initiative, and thank you all for the work! Suggestion:
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. |
I've started to implement the interface in our project. Observation: The interface is way too specific imo. Interfaces should be designed to reflect the smallest common denominator. |
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. 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! |
Unlock may not be possible due to loop attack. fee limit exceeded.
|
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.
|
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 |
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?
|
@nitika-goel that makes sense to me. |
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:
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()
andUnlock()
event, just like theTransfer()
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 wheneverbalanceOf()
queries are triggered on token holder’s account – the result does not include locked tokens. AtotalBalanceOf()
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
Fetching number of tokens locked under each utility
Fetching number of tokens locked under each utility at a future timestamp
Fetching number of tokens held by an address
Extending lock period
Increasing number of tokens locked
Fetching number of unlockable tokens under each utility
Fetching number of unlockable tokens
Unlocking tokens
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
The text was updated successfully, but these errors were encountered: