giveth / minime

Minimi Token. ERC20 compatible clonable token

The MiniMeToken contract is a standard ERC20 token with extra functionality:

The token is easy to clone!

Anybody can create a new clone token from any token using this contract with an initial distribution identical to the original token at a specified block. The address calling the createCloneToken function will become the token controller and the token's default settings can be specified in the function call.

function createCloneToken(
    string _cloneTokenName,
    uint8 _cloneDecimalUnits,
    string _cloneTokenSymbol,
    uint _snapshotBlock,
    bool _isConstant
    ) returns(address) {

Once the clone token is created, it acts as a completely independent token, with it's own unique functionalities.

Balance history is registered and available to be queried

All MiniMe Tokens maintain a history of the balance changes that occur during each block. Two calls are introduced to read the totalSupply and the balance of any address at any block in the past.

function totalSupplyAt(uint _blockNumber) constant returns(uint)

function balanceOfAt(address _holder, uint _blockNumber) constant returns (uint)

Optional token controller

The controller of the contract can generate/destroy/transfer tokens at its own discretion. The controller can be a regular account, but the intention is for the controller to be another contract that imposes transparent rules on the token's issuance and functionality. The Token Controller is not required for the MiniMe token to function, if there is no reason to generate/destroy/transfer tokens, the token controller can be set to 0x0 and this functionality will be disabled.

For example, a Token Creation contract can be set as the controller of the MiniMe Token and at the end of the token creation period, the controller can be transferred to the 0x0 address, to guarantee that no new tokens will be created.

To create and destroy tokens, these two functions are introduced:

function generateTokens(address _holder, uint _value) onlyController

function destroyTokens(address _holder, uint _value) onlyController

The Token's Controller can freeze transfers.

If transfersEnabled == false, tokens cannot be transferred by the users, however they can still be created, destroyed, and transferred by the controller. The controller can also toggle this flag.

// Allows tokens to be transferred if true or frozen if false
function enableTransfers(bool _transfersEnabled) onlyController

Applications

If this token contract is used as the base token, then clones of itself can be easily generated at any given block number, this allows for incredibly powerful functionality, effectively the ability for anyone to give extra features to the token holders without having to migrate to a new contract. Some of the applications that the MiniMe token contract can be used for are:

1. Generating a voting token that is burned when you vote.
2. Generating a discount "coupon" that is redeemed when you use it.
3. Generating a token for a "spinoff" DAO.
4. Generating a token that can be used to give explicit support to an action or a campaign, like polling.
5. Generating a token to enable the token holders to collect daily, monthly or yearly payments.
6. Generating a token to limit participation in a token sale or similar event to holders of a specific token.
7. Generating token that allows a central party complete control to transfer/generate/destroy tokens at will.
8. Lots of other applications including all the applications the standard ERC 20 token can be used for.

All these applications and more are enabled by the MiniMe Token Contract. The most amazing part being that anyone that wants to add these features can, in a permissionless yet safe manner without affecting the parent token's intended functionality.

How to deploy a campaign

1. Deploy the MinimeTokenFactory
2. Deploy the MinimeToken
3. Deploy the campaign
4. Assign the controller of the MinimeToken to the campaign.

minime's Issues

MiniMe does not fire Transfer on Zero Value

Checking new ERC20 spec - it states that transfers MUST log if value is zero.

    function doTransfer(address _from, address _to, uint _amount
    ) internal returns(bool) {

           if (_amount == 0) {
               return true;
           }

Gas-snapshot contains merge conflict artifacts

Just fix this...

remove tokens held by the 0x0 address from the total supply

Add a special function in the total supply function that subtracts the amount in the 0x0

totalSupply is declared in ERC20Basic and MiniMe, which therefore won't compile

Hi Jordi,

Been working with the MiniMe standard for some projects I have going in Africa. Have updated everything to use require() and revert() which seems to all work as expected. However, when trying to compile, I could not get passed this error (after using the Aragon repos as my basis):

../MiniMeMeetup/contracts/MiniMe .sol:239:5: DeclarationError: Identifier already declared.
    function totalSupply() constant returns (uint) {
    ^
Spanning multiple lines.
The previous declaration is here: ../MiniMeMeetup/zeppelin-solidity/token/ERC20Basic .sol:10:3:
  uint256 public totalSupply;
  ^------------------------^
Compiliation failed. See above.

except by commenting out the totalySupply() declaration in the ERC20Basic contract. However, I am not sure how this will effect the whole system? How did you work it for ANT, and why is it now failing? Any leads would be much appreciated...

New code needs comments

Consider adding unlimited allowance to save gas

see ethereum/EIPs#717

Add an event in the token factory to find the clone token address

    // An event to make the token easy to find on the blockchain
    NewCloneToken(address(cloneToken), _snapshotBlock);

Add that to the factory was well

Consider adding `decreaseAllowance()` and `increaseAllowance()`

Also taken from the Token Card Token Contract:

https://github.com/MonolithDAO/token/blob/master/src/Token.sol#L98

I'm not sure on the style choices but I do have a strong opinion on the name ;-) so i changed that. but i expect the other details might change as well. (like I hope to implement Dani's version of ds-math so we will use add() and subtract() instead of safeAdd() and safeSub())

The point of this addition to the contract is to avoid the race condition further by allow the UI to avoid using approve a second time... in fact, with these two functions, we can avoid using approve all together, but yet it can stay there for ol' times sake ;-)

    function increaseAllowance (address _spender, uint _addedValue) 
    onlyPayloadSize(2)
    returns (bool success) {
        uint oldValue = allowance[msg.sender][_spender];
        allowance[msg.sender][_spender] = safeAdd(oldValue, _addedValue);
        return true;
    }

    function decreaseAllowance (address _spender, uint _subtractedValue) 
    onlyPayloadSize(2)
    returns (bool success) {
        uint oldValue = allowance[msg.sender][_spender];
        if (_subtractedValue > oldValue) {
            allowance[msg.sender][_spender] = 0;
        } else {
            allowance[msg.sender][_spender] = safeSub(oldValue, _subtractedValue);
        }
        return true;
    }

[Alert] Overflow in generateTokens(), doTransfer() and Reentrancy of Controller.

I know that this contract seems to lost maintenance and there is no way to exploit the contract without permission.
But since it has gained a lot of reputation about the special feature of "Checkpoint", some projects still seems to use this repository and keeps downloading.
So I thought that it is better noticed about overflow exploits and Reentrancy Attacks in this minimetoken.

Overflow

generateTokens()

minime/contracts/MiniMeToken.sol

Lines 376 to 386 in ea04d95

	function generateTokens(address _owner, uint _amount
	) public onlyController returns (bool) {
	uint curTotalSupply = totalSupply();
	require(curTotalSupply + _amount >= curTotalSupply); // Check for overflow
	uint previousBalanceTo = balanceOf(_owner);
	require(previousBalanceTo + _amount >= previousBalanceTo); // Check for overflow
	updateValueAtNow(totalSupplyHistory, curTotalSupply + _amount);
	updateValueAtNow(balances[_owner], previousBalanceTo + _amount);
	Transfer(0, _owner, _amount);
	return true;
	}

in generateTokens(), there is a require statement in line 379 to prevent overflow.
unfortunately, curTotalSupply is declared as uint(which is uint256 as default), but ALL checkpoint related values are stored as uint128.

For exploit scenario, let assume totalSupply() is big enough (let's assume it's 2^128 - 1 for convenience) and wants to generate 10 more tokens.
On require statement, lefthand side will be 2^128 + 9 since curTotalsupply and _amount is both capable of 256bit data and on righthand side will be 2^128 - 1.

But after updateValueAt(), totalSupply() will be just 9. Since stored value (curTotalSupply + _amount) is bigger than what Checkpoint can handle.

Same thing happens for doTransfer()

I wrote an simple testcase for generateTokens() overflow(not making a PR because i don't want to mess up your environment).
https://github.com/leekt216/minime/blob/master/test/minimetoken_normal.js#L221-L236

And also, it should be noticed that since doTransfer() function calls onTransfer() of controller after loading sender balance and before loading receiver balance, it can be easily used for Reentrancy Attacks by Controller.

approve should not enforce setting to 0 value

LOW IMPORTANCE
MiniMeToken.approve(...) has a check requiring non-zero approval limits to be set to 0 before being set to a different non-zero value. The recently finalised ERC20 Token Standard suggest that the contract should not enforce this requirement

From @bokky audit
https://github.com/bokkypoobah/AigangCrowdsaleContractAudit/tree/master/audit#testing

Should allow and transfer from address 2 to address allowed to 3 not working

When just testing this out of the box, "Should allow and transfer tokens from address 2 to address 1 allowed to 3" & "should move tokens in the clone token from 2 to 3" is not working. Perhaps I may be missing something obvious here, but I'm having some trouble.

ClaimTokens is awesome

I just made a video about your claimTokens
https://www.youtube.com/watch?v=RL_e8fot29A
Every contract should have it

Switch testrpc dependency

Currently, the repo is using a modified forked version of testrpc in order to get websocket support. The new beta version of ganache comes with webscocket support BAKED IN gasp. Let's switch back.

Cloned token state is not locked when cloned with current or future block number

When token is cloned at block number that is current block (the default option) in createCloneToken

if (_snapshotBlock == 0) _snapshotBlock = block.number;

or with future block number, it should not be possible to change state (that is balances and total supply) of the cloned token. Allowing that will easily lead to inconsistencies when state changes in cloned token partially "shadow" changes in parent. Example

we are at block 100, parent token state is {owner1 balance: 100, owner2 balance :10, totalSupply:110}, token is cloned at block 110.

at block 110 two operations happen
clone.destroyTokens(owner1, 1)
parent.transfer(owner2, 10, {from: owner1}

which will result in following state of cloned token (at block 110 and future blocks)
owner1 balance: 99 (shadows parent token)
owner2 balance: 20 (taken from parent token as there is no value stored for owner2 in the clone)
totalSupply: 109 (shadows parent token)

at this point balances sum to 119 but totalSupply is 109

solution would be to revert on any calls to destroyTokens and generateTokens that happen before and at the block at which clone is created as doTransfer function already does.

changing the default clone _snapshotBlock to

if (_snapshotBlock == 0) _snapshotBlock = block.number - 1;

would also be advisable as parent state at block.number - 1 is already immutable. cloning at current block is basically cloning at unknown parent state (which may be changed before but also after the clone transaction)

[Improvement Proposal] Save gas in getValueAt function

This is original function

function getValueAt(Checkpoint[] storage checkpoints, uint _block
    ) constant internal returns (uint) {
        if (checkpoints.length == 0) return 0;

        // Shortcut for the actual value
        if (_block >= checkpoints[checkpoints.length-1].fromBlock)
            return checkpoints[checkpoints.length-1].value;
        if (_block < checkpoints[0].fromBlock) return 0;

        // Binary search of the value in the array
        uint min = 0;
        uint max = checkpoints.length-1;
        while (max > min) {
            uint mid = (max + min + 1)/ 2;
            if (checkpoints[mid].fromBlock<=_block) {
                min = mid;
            } else {
                max = mid-1;
            }
        }
        return checkpoints[min].value;
    }

As you can see in the while block, it tries to recreate mid variable every loop and costs 20,000 gas. I propose move mid out of the while block to save gas.
This is new function should be:

function getValueAt(Checkpoint[] storage checkpoints, uint _block
    ) constant internal returns (uint) {
        if (checkpoints.length == 0) return 0;

        // Shortcut for the actual value
        if (_block >= checkpoints[checkpoints.length-1].fromBlock)
            return checkpoints[checkpoints.length-1].value;
        if (_block < checkpoints[0].fromBlock) return 0;

        // Binary search of the value in the array
        uint min = 0;
        uint max = checkpoints.length-1;
        uint mid;
        while (max > min) {
            mid = (max + min + 1)/ 2;
            if (checkpoints[mid].fromBlock<=_block) {
                min = mid;
            } else {
                max = mid-1;
            }
        }
        return checkpoints[min].value;
    }

It is possible to break totalSupply in a clone and clones that never generate tokens cannot destroy tokens

This all stems from Checkpoint[] totalSupplyHistory; is never set when a clone is created.

When generateTokens(...) is called it retrieves the totalSupply value using the getValueAt(...) function.

We can see that in the getValueAt(...) function it will return 0 in the first line if the checkpoint length is 0 (for a new clone it is)

function getValueAt(Checkpoint[] storage checkpoints, uint _block
) constant internal returns (uint) {
    if (checkpoints.length == 0) return 0;
    ...
}

Now looking back to the generateToken(...) function we will see that it then incorrectly sets the totalSupply to 0 + amount

if (curTotalSupply + _amount < curTotalSupply) throw(); // Check for overflow
updateValueAtNow(totalSupplyHistory, curTotalSupply + _amount);

The simplest fix would be to use totalSupply() instead of getValueAt(...).

Additionally, the destroyTokens(...) function also uses getValueAt(...) and thus will always throw if a clone attempts to destroy tokens before it has generated tokens (to set the totalSupply checkpoint).

uint curTotalSupply = getValueAt(totalSupplyHistory, block.number);
if (curTotalSupply < _amount) revert();

This is because curTotalSupply is incorrectly set to 0 by getValueAt(...), 0 is less than the amount we are trying to destroy, and thus it throws.

The simplest fix would be to use totalSupply() instead of getValueAt(...).

How to take back ownership of MiniMeToken after a Campaign?

If I understand the README.md correctly, the steps to launch a Token + Campaign are:

1. Create MiniMeTokenFactory

2. Create the MineMeToken contract using address of contract created in step 1 - controller will default to address of contract owner/deployer

3. Create the Campaign contract

4. Using the MineMeToken changeController function, set the controller to be the the Campaign's address created in step 3.

This works great in testing, but we've essentially lost ownership of the MiniMeToken when the controller was changed in step 4. So when the Campaign timeline is finished the Controller of the MiniMeToken cannot be reverted back to the original owner since we've now lost ownership, which now belongs to the dead Campaign.

Retaining ownership of the Token contract would be essential in launching another Campaign and assigning the Controller to that Campaign... Am I missing a step here?

Great first step in this framework. It has great potential for contract use!!!

.length yields the fixed length of the byte array (read-only). so I think checkpoints.length++ is pointless, Can you explain its intention?I'm confused,thank you!

minime/contracts/MiniMeToken.sol

Line 454 in c23d03c

Checkpoint storage newCheckPoint = checkpoints[ checkpoints.length++ ];

Consider implementing a check on msg.data.length

There was an attack vector that an address that ends in 0 or 00 or 000 etc could use their address and omit the final 0's and then mess with the data length.

I love how the Token Card guys dealt with it.

https://github.com/MonolithDAO/token/blob/master/src/Token.sol#L23

    modifier onlyPayloadSize(uint numwords) {
        assert(msg.data.length == numwords * 32 + 4);
        _;

I might suggest we consider changing the verbiage, but the intent and application is great, in every function that a user has to input msg.data that includes and address, this modifier is used... this means transfer(), transferFrom(), approve() and also decreaseApproval() and increaseApproval() if we use it. (see my other issue ;-))

My suggested verbiage:

    modifier onlyValidDataLengths(uint numOfParams) {
        assert(msg.data.length == numOfParams * 32 + 4);
        _;

Unclear (and possible wrong) code at internal function updateValueAtNow(Checkpoint[] storage, uint)

    /// @dev `updateValueAtNow` used to update the `balances` map and the
    ///  `totalSupplyHistory`
    /// @param checkpoints The history of data being updated
    /// @param _value The new number of tokens
    function updateValueAtNow(Checkpoint[] storage checkpoints, uint _value
    ) internal  {
        if ((checkpoints.length == 0)
        || (checkpoints[checkpoints.length -1].fromBlock < block.number)) {
               Checkpoint newCheckPoint = checkpoints[ checkpoints.length++ ];
               newCheckPoint.fromBlock =  uint128(block.number);
               newCheckPoint.value = uint128(_value);
           } else {
               Checkpoint oldCheckPoint = checkpoints[checkpoints.length-1];
               oldCheckPoint.value = uint128(_value);
           }
    }

Checkpoint newCheckPoint = checkpoints[ checkpoints.length++ ];

If checkpoints.length is zero. checkpoints.length++ when evaluated increases the storage allocation for that array and returns the lenght, leading to checkpoints[1], not checkpoints[0], that would be the correct point.
Consider using checkpoints.push(Checkpoint({fromBlock: block.number, value: _value})):

Withdrawal

Implement ds-math

https://github.com/Giveth/ds-math/blob/master/src/SafeMath.sol

I like this library! Let's use it (Thx Dani ;-) )

Verification of Code Impossible?

How is the the actual token code (not the MiniMe factory) obtained to verify the contract on Etherscan?
This seems to be a weakness in this contract creating a contract method?