Best approach for permissions in calling methods of another contract

#1

Hi. I have a Token contract that was derived from StandaloneERC20. In the minting method (_mint), the same amount of minted tokens are transfered into a Vesting contract. Since this Vesting contract has a very complicated logic in terms of conditions to release token, I have to call a special function of this Vesting contract, that will register the newly minted token and do some other stuff.

I want to ensure, that only the minting process can execute this function. What would be the best approach?

In StandaloneERC20, a Minter role is defined. Should I pass the same role also to the Vesting contract and pass the onlyMinter-modifier to this particular method or are there better approaches?

This is my token contract:

contract XXXToken is StandaloneERC20, Ownable {

    TeamVesting private _teamVesting;

    event TeamVestingAssigned(address teamVesting);

    function initialize(
        address owner,
        address[] memory minters,
        address[] memory pausers
    ) public initializer {
        StandaloneERC20.initialize("XXXToken", "XXX", uint8(18), minters, pausers);
        Ownable.initialize(owner);
    }

    function assignTeamVesting(TeamVesting teamVesting) public onlyOwner {
        require(address(teamVesting) != address(0));

        _teamVesting = teamVesting;
        emit TeamVestingAssigned(address(teamVesting));
    }

    /**
     * @dev While minting token special team tokens are granted/vested
     * @param account The account of beneficiary who will get the minted token
     * @param value The amount of minted token
     */
    function _mint(address account, uint256 value) internal whenNotPaused onlyMinter {
        super._mint(account, value);

        // we give the same amount of token to the team vesting contract

        if(address(_teamVesting) != address(0)) {
            super._mint(address(_teamVesting), value);
            _teamVesting.registerTeamToken(value);
        }
    }

}

In the function _mint, a function registerTeamToken is executed on the TeamVesting-contract. It does some other stuff in order to register when and how many token can released due to detailed terms & conditions.

function registerTeamToken(uint256 value) public {
}

To keep things simple, can I make this function only executable by the Token contract in a different way than to pass a specific role like it was done with onlyMinter?

2 Likes
#2

If I understand correctly, every time XXXToken is minted, you also mint and register the same value again for team vesting.

I wonder if this minting logic needs to be in the token at all and couldn’t be separated out. It might be worth having a look at some of the Crowdsale functionality, especially MintedCrowdsale as an example.
The token is primarily just an ERC20, the crowdsale handles the minting, and any vesting is handled in vesting contracts.

In your case, you could look to have a token contract, a minter contract (to handle any minting logic) and vesting contracts, separating the concerns and potentially making each contract simpler.

The OpenZeppelin guidelines are good to keep in mind.

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/GUIDELINES.md

D1 - Simple and Modular

Simpler code means easier audits, and better understanding of what each component does. We look for small files, small contracts, and small functions. If you can separate a contract into two independent functionalities you should probably do it.

As for having permissioned functions. You could create your own roles https://docs.openzeppelin.org/v2.3.0/api/access or check that the caller is in a specific existing role e.g. isMinter, or have some form of simple registration (e.g. when creating the vesting contract, register who can call the register function).

1 Like
#3

Many thanks for your answer. The reason that I won’t use MintedCrowdsale Contract is because it derives from CrowdSale which has a payable function and fix rate. My customer wants to sell their token against current EURO-rate thats why we need a simple minting process, but not the original scheme of a typical ETH-Crowdsale.

But I will think about separation of token and crowdsale. I am also not a fan of mixing this up in the same contract.

2 Likes
#4

Given you are doing a crowdsale, I would look at the various crowdsale options to see what closest matches your needs. Ideally inheriting from OpenZeppelin implementations.

You could override the rate functionality to make the rate variable in Crowdsale.sol.

Separating functionality is great for users and auditors too.

1 Like
#5

Thanks @abcoathup. Actually it’s a pitty that there is no base contract for Crowdsale without any Rate inside or at least a base class that would allow a rate of zero. I understand that the basic concept of a crowdsale was to send Ether in and get token out and where a rate is required.

But in terms of Fiat based Crowdsales it is an issue also on legal perspective. We can’t update current ETH/EUR so often that it would reflect real market prices. For other legal reasons too we have to sell the token offchain in this project and mint them later after a 14 days refund policy.

Does it then still make sense to inherit from Crowdsale-Contract in your oppinion? If so, what would you recommend how to interact with CrowdSale contract derivation? Shall I override _getTokenAmount and revert? Or override buyTokens and just revert it?

Another approach would be to create an own CrowdSale contract without to inherit from Openzepplins CrowdSale.

2 Likes
#6

It depends what works for your specific case.

I would look at having a token inheriting from OpenZeppelin and ideally the vesting from
OpenZeppelin (assuming this makes sense for your vesting requirements) and then either inherit from an OpenZeppelin Crowdsale or reuse (complying with MIT licence) as much as possible and tailor for your requirements.

1 Like
#7

Hm, your requirements are somewhat uncommon. If you’re not really selling things on-chain, then I don’t see why you would use a Crowdsale: the whole model revolves around exchanging ETH for tokens.

Interesting! May I ask why you’re minting twice the amount, some for the recipient and some for the vesting contract?

Regarding only calling registerTeamToken on _mint, sadly there’s no way for a contract to react to ERC20s being received, so there’s no way to enforce your requirement.

An alternative would be e.g. keeping a balance of how many tokens _teamVesting is due, incrementing it on calls to _mint, and then having a public function on _teamVesting that causes it to call the token contract and retrieve its balance, then calling registerTeamToken.

    function _mint(address account, uint256 value) internal whenNotPaused onlyMinter {
        super._mint(account, value);

        // we give the same amount of token to the team vesting contract

        if(address(_teamVesting) != address(0)) {
            _teamTokensDue += value;
        }
    }

    function teamTokenWithdraw() public returns(uint256) {
        require(msg.sender == _teamVesting);

        uint256 amount = _teamTokensDue;
        _mint(_teamVesting, amount);
        _teamTokensDue = 0;
        return amount;
}

// in teamVesting
function withdrawTokens() public {
    uint256 amount = token.teamTokenWithdraw();
    require(amount > 0);
    registerTeamToken(amount);
    
}

Hope this helps!

3 Likes
#8

You’re welcome :slight_smile:

This is the business model of the client in his Token Sale: for every sold token another token is minted for the team in order to create value running their own business. But these tokens were vested on a monthly basis. Token that was sold in July 2019 can be released in July 2020, Tokens that was sold in August 2019, can be released by the team not before August 2020 and so on. This way they want to give the people trust that they won’t sell these tokens that quickly.

This monthly based vesting is the reason for my registerTeamToken-function, because this function is registering every sold token on a monthly indexed bases (according to a 31-days-fixed-month-size to make things easier).

But I find your approach with withdrawing total interesting and will give it a try now. It also feels more common in respect to other contracts. Thank you very much for this nice idea!

1 Like
#9

Glad you found it useful!

Ohh, interesting! Perhaps you could use TokenTimelock to take care of the vesting? Though with the way it’s set up, you’d deploying a new timelock contract on each call to _mint, which may not be ideal.

1 Like
#10

Yeah, I was already thinking about TokenTimelock as a basis. Since the locking occurs on a monthly basis, I could create 12 instances of TokenTimelock and mint token into it on the basis of the current month. But this also makes it not easier all in all. Maybe I should consider something like a MonthlyTokenTimelock-Contract to give it more separation of concerns and offer some reuseability for other projects with similar business rules?

1 Like
#11

I’m unsure: we’ve considered having a better vesting contract for some time now (https://github.com/OpenZeppelin/openzeppelin-solidity/issues/1214), but a concern I’ve always had with the current one is the need to deploy a new contract for each user. That approach has the nice property of limiting the attack surface (since each contract only funds funds for a single user, limiting damage), but is also cumbersome and expensive.

A scheme where a single contract can e.g. handle an unlimited number of timelocks for a single token sounds like a good idea. An issue with this however is the lack of a msg.value equivalent for ERC20 tokens, we’d have to go with an approve/transferFrom mechanism, which is annoying to use. ERC777 and its send function are a great fit for this.

2 Likes