livepeer / merkle-mine

Token distribution based on providing Merkle proofs of inclusion in genesis state to generate allocation
29 stars 9 forks source link

MultiMerkleMine Contract #14

Closed dob closed 6 years ago

dob commented 6 years ago

Currently, the MerkleMine smart contract allows the caller to generate a single proof for a single recipient in the generate() transaction. This issue is about creating a new convenience smart contract called the MultiMerkleMine which allows a caller to submit many proofs for many accounts in a single transaction. This allows a MerkleMiner to more efficiently generate token on unclaimed accounts while saving on gas.

Description

The MultiMerkleMine contract is purely a convenience wrapper around an existing MerkleMine contract deployed on the blockchain.

MultiMerkleMine Spec

Constructor

function MultiMerkleMine() public

takes no arguments, as the contract stores no state

Public functions

function multiGenerate(address _merkleMineContract, address[] _recipients, bytes[] _merkleProofs) external

/**
 * 1. Perform validations:
 *   - length of _recipients == length of _merkleProofs
 *   
 * 2. The input _merkleMineContract address is the MerkleMine contract that this function will be interacting with. Record the MerkleMine token balance for this contract at the start of this function. During this function's execution, this contract's token balance will increase as it generates token, so we'll want to observe how much we generated later, so it can be transferred to the caller.
 * 
 * 3. For each input _recipient and _merkleProof pair (by given index i):
 * .    - check if MerkleMine contract generated[_recipients[i] == false. If so, call MerkleMine.generate(_recipients[i], _merkleProofs[i])
 * 
 * 4. At the completion of all the calls to generate, check the new balance of the MerkleMine token for this contract. This new balance minus the original balance is equal to the total newly generated token that the caller deserves. Transfer this balance to msg.sender.
 * 
 */

Tests

Please write tests to cover the following cases:

Additional implementation details

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 0.5 ETH (233.56 USD @ $467.11/ETH) attached to it.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 11 months, 3 weeks from now. Please review their questions below:

  1. vyomshm has started work.
    • Q: I've started working on this and will let you know if I have any more questions.

Q) So, the callerTokenAmount worth of newly generated tokens will then first be transferred to directly called MultiMinerMine contract, which will then forward it to the original msg.sender ?

dob commented 6 years ago

Q) So, the callerTokenAmount worth of newly generated tokens will then first be transferred to directly called MultiMinerMine contract, which will then forward it to the original msg.sender ?

I believe so. If the user calls the MerkleMine generate contract directly, they are msg.sender in the context, and the callerTokenAmount gets transfered to them. But in this case, since the MultiMerkleMine contract is going to invoke the MerkleMine.generate() function, I believe msg.sender will appear to MerkleMine to be the MultiMerkleMine contract itself. Which then needs to forward the token to the original caller. There may be other patterns to consider such as delegatecall, but I think the recommended path is the most straightforward.

Open to other suggestions to get the token to the original caller though, @vyomshm, if you have a recommendation, as that is the goal.

yondonfu commented 6 years ago

It would be nice to use the latest Solidity version (currently v0.4.24). The more recent versions of Solidity introduce the constructor keyword which could be used instead of a named constructor function like function MultiMerkleMine() public in addition to the ability to attach a reason string when reverting either with the require or revert statements.

One thing to look out for is that as of Solidity v0.4.24, dynamic arrays of dynamic arrays parameters i.e. string[] and bytes[] are not supported by default. Some options to possibly consider here include:

IMO option 3 seems appealing and doesn't require using an experimental compiler feature, but open to other thoughts here too!

gitcoinbot commented 6 years ago

@vyomshm Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

dob commented 6 years ago

@vyomshm are you still working on this? If not then I'd like to release it to someone else. Thanks, and let me know if you have any questions.

vyomshm commented 6 years ago

@dob @yondonfu

Yes, I'm still working on this.

Here's my current implementation of MultiMerkleMine.sol -

pragma solidity ^0.4.24;

import "./MerkleMine.sol";
import "zeppelin-solidity/contracts/token/ERC20/ERC20.sol";
import "zeppelin-solidity/contracts/math/SafeMath.sol";

contract MultiMerkleMine {

    using SafeMath for uint256;

    bytes[] private _proofs;

    event AlreadyGenerated(address indexed recipient, address indexed caller);

    function extractProofs(bytes _merkleProofs) internal returns(bytes[]) {
        require(_merkleProofs.length != 0, 'No proofs supplied!');
        for(uint256 i=0; i<_merkleProofs.length; i++){
            uint256 proofSize = uint256(_merkleProofs[i]);
            require(proofSize % 32 == 0, 'Invalid proof detected!');
            bytes memory _proof = new bytes(proofSize);
            uint256 j = 0;
            while(j<proofSize){
                _proof[j]=_merkleProofs[i+1];
                i+=1;
                j+=1;
            }
            _proofs.push(_proof);
        }

        return _proofs;
    }

    function multiGenerate(address _merkleMineContract, address[] _recipients, bytes _merkleProofs) public {
        MerkleMine mine = MerkleMine(_merkleMineContract);
        ERC20 token = ERC20(mine.token());

        require (block.number >= mine.callerAllocationStartBlock());

        uint256 initialBalance = token.balanceOf(this);
        bytes[] memory proofs = extractProofs(_merkleProofs);

        require(proofs.length == _recipients.length, 'Number of recipients and proofs is not equal!');

        for(uint256 i=0; i < _recipients.length; i++){
            if(!mine.generated(_recipients[i])){
                mine.generate(_recipients[i], proofs[i]);
            }else{
                emit AlreadyGenerated(_recipients[i], msg.sender);
            }
        }

        uint256 newBalanceSinceAllocation = token.balanceOf(this);
        uint256 callerTokensGenerated = newBalanceSinceAllocation.sub(initialBalance);

        if(callerTokensGenerated > 0){
            token.transfer(msg.sender, callerTokensGenerated);
        }

        delete _proofs;
    }

}

I didn't want to end up using the experimental compiler for implementing this, so I went with @yondonfu 's suggestion. This implementation passes all the required tests but does include a storage variable! i'm still working on an alternative way to do this.

vyomshm commented 6 years ago

15

vyomshm commented 6 years ago

15 So I've updated the MultiMerkleMine contract so that it uses assembly to extract proofs using a BytesUtil library. Also it no longer has that bytes[] storage _proofs array and everything happens in memory.

dob commented 6 years ago

Closed via #15

dob commented 6 years ago

@vyomshm I'm trying to pay out the Gitcoin bounty, but it looks like it's still in "work started" phase. If you can mark it as work submitted, then I think I can pay it out.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work for 0.5 ETH (233.25 USD @ $466.5/ETH) has been submitted by:

  1. @vyomshm
  2. @vyomshm

@dob please take a look at the submitted work:


vyomshm commented 6 years ago

Yeah, sorry about that. I just did so, should be working now.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


The funding of 0.5 ETH (233.25 USD @ $466.5/ETH) attached to this issue has been approved & issued to @vyomshm.