Skip to content
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

ERC20 token with snapshots #1209

Closed
frangio opened this issue Aug 15, 2018 · 7 comments · Fixed by #1617
Closed

ERC20 token with snapshots #1209

frangio opened this issue Aug 15, 2018 · 7 comments · Fixed by #1617
Assignees
Labels
contracts Smart contract code. feature New contracts, functions, or helpers.
Milestone

Comments

@frangio
Copy link
Contributor

frangio commented Aug 15, 2018

We should have a flavor of ERC20 that adds snapshotting of balances. This mechanism allows for implementing a number of things such as trustless dividends, weighted voting (think DAOs), and forking.

I wrote an implementation that is quite efficient, with O(log(snapshot count)) cost for retrieving past balances, and relatively small constant overhead for transfers. It can be found in my frangio:feature-snapshot-token branch.

I posted this initially in #991 but can't continue working on it for now due to lack of time. Someone needs to complete the work on it by adding comprehensive tests and documentation, and maybe review some naming. After #1097 is merged it should also account for the new internal _mint and _burn functions. We should measure the gas overhead too. Don't hesitate to ask any questions about it!

There was some previous discussion in #991, and in #1100 which was a proposal of an implementation that turned out to be too costly.

@frangio frangio added feature New contracts, functions, or helpers. contracts Smart contract code. labels Aug 15, 2018
@frangio frangio added this to the v2.1 milestone Aug 15, 2018
@dmdque
Copy link
Contributor

dmdque commented Sep 6, 2018

Came across this issue in passing. This contract may be helpful.

@jbogacz
Copy link
Contributor

jbogacz commented Sep 14, 2018

@frangio I'm trying map in my mind your branch code to current project state. Could you correct me if I'm wrong -> Old StandardToken is just now ERC20 right ? We'r gonna to produce something like SnapshotERC20 or better to stick naming convention ERC20Snapshot ?

@frangio
Copy link
Contributor Author

frangio commented Sep 14, 2018

Old StandardToken is just now ERC20 right ?

Yes! And we should stick tot he naming convention with something like ERC20Snapshot, or ERC20Snapshots. Other name ideas are welcome.

@jbogacz are you interested in working on this issue?

@jbogacz
Copy link
Contributor

jbogacz commented Sep 15, 2018

@frangio Yes, I'm! It's very good candidate to get more familiar with OZ engine.

@frangio
Copy link
Contributor Author

frangio commented Sep 16, 2018

@jbogacz Awesome. Let me know if you have any questions.

@jbogacz
Copy link
Contributor

jbogacz commented Sep 18, 2018

Hey @frangio! It took me a moment but I've created PR. Could you please take a look and let me know about your remarks? Do you suggest to prepare any other test scenario that I missed? I'm looking forward your comments.

@mswezey23
Copy link
Contributor

Validity Labs is also working on a Snapshot Token. Currently finishing it up now, will do an internal review later this week with the hopes of submitting a PR to OZ.

nventuro pushed a commit that referenced this issue Oct 8, 2018
*     Add Arrays library with unit tests (#1209)

    * prepared due to snapshot token requirements
    * add library with method to find upper bound
    * add unit test for basic and edge cases

* Imporove documentation for Arrays library

Simplify Arrays.test.js to use short arrays as test date

* Added comment for uint256 mid variable.
* Explaned why uint256 mid variable calculated as Math.average is safe to use as index of array.
nventuro pushed a commit to nventuro/openzeppelin-contracts that referenced this issue Oct 18, 2018
…1375)

*     Add Arrays library with unit tests (OpenZeppelin#1209)

    * prepared due to snapshot token requirements
    * add library with method to find upper bound
    * add unit test for basic and edge cases

* Imporove documentation for Arrays library

Simplify Arrays.test.js to use short arrays as test date

* Added comment for uint256 mid variable.
* Explaned why uint256 mid variable calculated as Math.average is safe to use as index of array.

(cherry picked from commit f7e53d9)
@frangio frangio modified the milestones: Test helpers, v2.1 Nov 20, 2018
@frangio frangio self-assigned this Dec 3, 2018
@frangio frangio modified the milestones: v2.1, v2.2 Dec 11, 2018
nventuro referenced this issue in mswezey23/openzeppelin-solidity Jan 22, 2019
nventuro referenced this issue in mswezey23/openzeppelin-solidity Jan 23, 2019
frangio referenced this issue in mswezey23/openzeppelin-solidity Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Smart contract code. feature New contracts, functions, or helpers.
Projects
None yet
4 participants