Implementation of an ERC20 token with snapshots #991
Conversation
frangio
force-pushed
the
feature-snapshot-token
branch
from
b779fc8 to
61d16d6
Compare
| function balanceOfAt(address _account, uint256 _snapshotId) public view returns (uint256) { | ||
| require(_snapshotId > 0 && _snapshotId <= currSnapshotId); | ||
|
|
||
| uint256 idx = snapshotIds[_account].findUpperBound(_snapshotId); |
There was a problem hiding this comment.
This is doing a binary search. We should calculate if gas becomes too much for long arrays. I doubt it.
| } | ||
|
|
||
| function snapshot() public returns (uint256) { | ||
| currSnapshotId += 1; |
There was a problem hiding this comment.
could use AutoIncrementing here if it happens to be available when this is merged
| }); | ||
|
|
||
| it('can snapshot!', async function () { | ||
| assert.equal(await this.token.balanceOf(account1), 10000); |
There was a problem hiding this comment.
could we update this to use should assertions?
There was a problem hiding this comment.
although that may be annoying if should.eventually.be.bignumber.eq() doesn't work correctly
|
@frangio This is super cool! Someone asked us if it'd be possible for our contracts to decouple themselves from the MiniMe token and maybe this could be standardized as a EIP token standard that extends ERC20 with snapshot capabilities. At least for voting, the manual |
|
@frangio @sohkai Although this is an elegant solution, and definitely better than MiniMe, I question if this is really the concern of a token. If this is used for example for a voting mechanism then I think staking is far more beneficial. My worry here is that it over bloats and promotes over bloating of tokens with features which do not need to be inside the token contract taking away from the general simplicity of a token. |
@decanus I think you're right in many cases, and this comment left me thinking quite a bit. After discussing it with a number of people, I think that this is still a mechanism worth having in OpenZeppelin. We're completely aligned with not bloating tokens, but I think this is an example where it makes total sense to have extra code in one. We need code to run in every transfer in order to update the copy-on-write balances. The snapshot data could live in another contract but the token itself would still need at least some code to trigger this update. I'm closing this PR for now and opening #1209 instead so that someone with more time can take it to completion by adding tests and documentation. @decanus Please add a comment over there if you want to continue the discussion! |
Got this idea from talking with the @aragon team (cc @sohkai @izqui).
This is an ERC20 token with the ability to query balances in the past, similar to what the MiniMeToken does. We need to do some measurements but I'm sure everything here is much cheaper than MiniMe. In this case though, we cannot retrieve the balances at any arbitrary block in the past, but only at previously requested snapshots. I'm not sure if this is enough for voting applications, so I'm throwing this out there to get some feedback!
(Another feature which is in MiniMe and not here is the ability to fork. I don't intend to do it, but it could easily be implemented by taking a snapshot and then creating a token which lazily imports its balances from said snapshot.)
Snapshot->SnapshotCreatedor some other verb?)