0

So I have a ERC20 token smart contract and a timelock contract. Everytime I call the Deposit function on the timelock contract I get the error "The constructor should be payable if you send value".

So I just tried to make all the functions related to transferring funds payable so as to just make it work but still gives me the same error. Help is greatly appreciated:

TimeLock.sol

pragma solidity ^0.4.24;

import './IERC20.sol';

contract TimeLock {
    IERC20 token;

    struct LockBoxStruct {
        address beneficiary;
        uint balance;
        uint releaseTime;
    }

    LockBoxStruct[] public lockBoxStructs; // This could be a mapping by address, but these numbered lockBoxes support possibility of multiple tranches per address

    event LogLockBoxDeposit(address sender, uint amount, uint releaseTime);   
    event LogLockBoxWithdrawal(address receiver, uint amount);

    constructor(address tokenContract) public payable {
        token = IERC20(tokenContract);
    }

    function deposit(address beneficiary, uint amount, uint releaseTime) public payable returns(bool success) {
        require(token.transferFrom(msg.sender, address(this), amount));
        LockBoxStruct memory l;
        l.beneficiary = beneficiary;
        l.balance = amount;
        l.releaseTime = releaseTime;
        lockBoxStructs.push(l);
        emit LogLockBoxDeposit(msg.sender, amount, releaseTime);
        return true;
    }

    function withdraw(uint lockBoxNumber) public returns(bool success) {
        LockBoxStruct storage l = lockBoxStructs[lockBoxNumber];
        require(l.beneficiary == msg.sender);
        require(l.releaseTime <= now);
        uint amount = l.balance;
        l.balance = 0;
        emit LogLockBoxWithdrawal(msg.sender, amount);
        require(token.transfer(msg.sender, amount));
        return true;
    }    

}

FixedSupplyToken.sol

pragma solidity ^0.4.24;

// ----------------------------------------------------------------------------
// 'FIXED' 'Example Fixed Supply Token' token contract
//
// Symbol      : FIXED
// Name        : Example Fixed Supply Token
// Total supply: 1,000,000.000000000000000000
// Decimals    : 18
//
// Enjoy.
//
// (c) BokkyPooBah / Bok Consulting Pty Ltd 2018. The MIT Licence.
// ----------------------------------------------------------------------------


// ----------------------------------------------------------------------------
// Safe maths
// ----------------------------------------------------------------------------
library SafeMath {
    function add(uint a, uint b) internal pure returns (uint c) {
        c = a + b;
        require(c >= a);
    }
    function sub(uint a, uint b) internal pure returns (uint c) {
        require(b <= a);
        c = a - b;
    }
    function mul(uint a, uint b) internal pure returns (uint c) {
        c = a * b;
        require(a == 0 || c / a == b);
    }
    function div(uint a, uint b) internal pure returns (uint c) {
        require(b > 0);
        c = a / b;
    }
}


// ----------------------------------------------------------------------------
// ERC Token Standard #20 Interface
// https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md
// ----------------------------------------------------------------------------
contract ERC20Interface {
    function totalSupply() public constant returns (uint);
    function balanceOf(address tokenOwner) public constant returns (uint balance);
    function allowance(address tokenOwner, address spender) public constant returns (uint remaining);
    function transfer(address to, uint tokens) public payable returns (bool success);
    function approve(address spender, uint tokens) public returns (bool success);
    function transferFrom(address from, address to, uint tokens) public payable returns (bool success);

    event Transfer(address indexed from, address indexed to, uint tokens);
    event Approval(address indexed tokenOwner, address indexed spender, uint tokens);
}


// ----------------------------------------------------------------------------
// Contract function to receive approval and execute function in one call
//
// Borrowed from MiniMeToken
// ----------------------------------------------------------------------------
contract ApproveAndCallFallBack {
    function receiveApproval(address from, uint256 tokens, address token, bytes data) public;
}


// ----------------------------------------------------------------------------
// Owned contract
// ----------------------------------------------------------------------------
contract Owned {
    address public owner;
    address public newOwner;

    event OwnershipTransferred(address indexed _from, address indexed _to);

    constructor() public {
        owner = msg.sender;
    }

    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }

    function transferOwnership(address _newOwner) public onlyOwner {
        newOwner = _newOwner;
    }
    function acceptOwnership() public {
        require(msg.sender == newOwner);
        emit OwnershipTransferred(owner, newOwner);
        owner = newOwner;
        newOwner = address(0);
    }
}


// ----------------------------------------------------------------------------
// ERC20 Token, with the addition of symbol, name and decimals and a
// fixed supply
// ----------------------------------------------------------------------------
contract FixedSupplyToken is ERC20Interface, Owned {
    using SafeMath for uint;

    string public symbol;
    string public  name;
    uint8 public decimals;
    uint _totalSupply;

    mapping(address => uint) balances;
    mapping(address => mapping(address => uint)) allowed;


    // ------------------------------------------------------------------------
    // Constructor
    // ------------------------------------------------------------------------m
    constructor() public payable {
        symbol = "FIXED";
        name = "Example Fixed Supply Token";
        decimals = 18;
        _totalSupply = 1000000;  //* 10**uint(decimals);
        balances[owner] = _totalSupply;
        emit Transfer(address(0), owner, _totalSupply);
    }


    // ------------------------------------------------------------------------
    // Total supply
    // ------------------------------------------------------------------------
    function totalSupply() public view returns (uint) {
        return _totalSupply.sub(balances[address(0)]);
    }


    // ------------------------------------------------------------------------
    // Get the token balance for account `tokenOwner`
    // ------------------------------------------------------------------------
    function balanceOf(address tokenOwner) public view returns (uint balance) {
        return balances[tokenOwner];
    }


    // ------------------------------------------------------------------------
    // Transfer the balance from token owner's account to `to` account
    // - Owner's account must have sufficient balance to transfer
    // - 0 value transfers are allowed
    // ------------------------------------------------------------------------
    function transfer(address to, uint tokens) public payable returns (bool success) {
        balances[msg.sender] = balances[msg.sender].sub(tokens);
        balances[to] = balances[to].add(tokens);
        emit Transfer(msg.sender, to, tokens);
        return true;
    }


    // ------------------------------------------------------------------------
    // Token owner can approve for `spender` to transferFrom(...) `tokens`
    // from the token owner's account
    //
    // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md
    // recommends that there are no checks for the approval double-spend attack
    // as this should be implemented in user interfaces 
    // ------------------------------------------------------------------------
    function approve(address spender, uint tokens) public returns (bool success) {
        allowed[msg.sender][spender] = tokens;
        emit Approval(msg.sender, spender, tokens);
        return true;
    }


    // ------------------------------------------------------------------------
    // Transfer `tokens` from the `from` account to the `to` account
    // 
    // The calling account must already have sufficient tokens approve(...)-d
    // for spending from the `from` account and
    // - From account must have sufficient balance to transfer
    // - Spender must have sufficient allowance to transfer
    // - 0 value transfers are allowed
    // ------------------------------------------------------------------------
    function transferFrom(address from, address to, uint tokens) public payable returns (bool success) {
        balances[from] = balances[from].sub(tokens);
        allowed[from][msg.sender] = allowed[from][msg.sender].sub(tokens);
        balances[to] = balances[to].add(tokens);
        emit Transfer(from, to, tokens);
        return true;
    }


    // ------------------------------------------------------------------------
    // Returns the amount of tokens approved by the owner that can be
    // transferred to the spender's account
    // ------------------------------------------------------------------------
    function allowance(address tokenOwner, address spender) public view returns (uint remaining) {
        return allowed[tokenOwner][spender];
    }


    // ------------------------------------------------------------------------
    // Token owner can approve for `spender` to transferFrom(...) `tokens`
    // from the token owner's account. The `spender` contract function
    // `receiveApproval(...)` is then executed
    // ------------------------------------------------------------------------
    function approveAndCall(address spender, uint tokens, bytes data) public returns (bool success) {
        allowed[msg.sender][spender] = tokens;
        emit Approval(msg.sender, spender, tokens);
        ApproveAndCallFallBack(spender).receiveApproval(msg.sender, tokens, this, data);
        return true;
    }


    // ------------------------------------------------------------------------
    // Don't accept ETH
    // ------------------------------------------------------------------------
    function () public payable {
        revert();
    }


    // ------------------------------------------------------------------------
    // Owner can transfer out any accidentally sent ERC20 tokens
    // ------------------------------------------------------------------------
    function transferAnyERC20Token(address tokenAddress, uint tokens) public onlyOwner returns (bool success) {
        return ERC20Interface(tokenAddress).transfer(owner, tokens);
    }
}
  • The error message "The constructor should be payable if you send value" is a generic message that is returned whenever a transaction reverts in Remix. That is likely not the issue you are having... – Shawn Tabrizi Nov 16 '18 at 0:24
  • damn ok thank you. What's the issue then? the debug doesn't help at all – Cyzanfar Nov 16 '18 at 0:25
  • You haven't really explained at all what you were doing to cause the error. Did you make sure this was satisfied?: require(token.transferFrom(msg.sender, address(this), amount)); – Shawn Tabrizi Nov 16 '18 at 0:30
  • True... so the error happens when I call the Deposit function. I pass in 3 values: the address of the beneficiary (one of the address I have), the amount (100) and the releasetime (i.e 1542326766) – Cyzanfar Nov 16 '18 at 0:32
1

That message:

The constructor should be payable if you send value"

is quite misleading. It's a catch-all for something that wasn't allowed.

From the sound of it, the likely suspect is right here:

require(token.transferFrom(msg.sender, address(this), amount));

require() means throw an error if the inner expression doesn't evaluate to true.

So, we think token.transferFrom() is supposed to return a bool about success or failure. That would be right per the spec for ERC20.

Your question didn't say anything about approve() and that has to be the first step. The transferFrom() method is taking funds from elsewhere (not sending forward). Wait ... so Alice can instruct a contract to take funds from Bob? Only if it's okay with Bob. It goes like this:

  1. Alice tells the token contract she approves that this contract should be able to help itself to an amount of her funds. This sets an allowance in the token contract for this contract.
  2. Then she comes here an uses the deposit() function which will exercise that one-time privilege. When the tokens are moved from Alice to this contract (in the token contract), then the allowance will be used up.

This flow isn't the easiest thing to understand at first.

It might help to think about how it should not be possible to write a contract that spends other people's money. The approve() scheme addresses this so it can be done in a controlled way. This lets us set up deposit() functions like this and handle the result of the token deposit in an atomic fashion.

Hope it helps.

  • that's very clear thank you! so In my case who is the tokenOwner and the spender in the allowance function allowance(address tokenOwner, address spender)? since my goal is to be able to call the deposit and get funds locked for different beneficiaries in my timelock contract. (This is the continuity of the question you answered several days ago) :) – Cyzanfar Nov 16 '18 at 0:48
  • it goes approve(spender, amount) ... and it's from the same user who is going to use deposit() here. spender is your contract with the deposit() function that's going to take the funds. Don't forget the approver (msg.sender) needs a sufficient balance or the token contract won't allow the approval. – Rob Hitchens - B9lab Nov 16 '18 at 0:55
  • Terrific Rob! now last question (until my next question lol): the withdraw function on TimeLock takes a number which I believe is the index of the LockBoxStructs array. Is there a better way of releasing the funds for a particular beneficiary? here I need to rely on the index and that can be a problem. A nicer way would be to have a sort of dictionary keyed by the address of the beneficiary no? thanks again for your help!! – Cyzanfar Nov 16 '18 at 1:40
  • 1
    Thanks for accepting my answer. The rationale there is there could be multiple boxes for the same beneficiary with different amounts and deadlines. It's reasonable to ask them which box they intend to unlock because they can easily discover all boxes that belong to them, the contents and deadlines by observing event logs. Other designs are possible but you'd have to reconcile the logic in ways that will probably complicate matters. As a general rule, always simplify the contract, even at the expense of inconveniencing the client. – Rob Hitchens - B9lab Nov 16 '18 at 1:59
  • I see what you mean. I guess i'd need to keep track of the index somehow so that users know what index to pass when trying to withdraw the funds. Again this is very helpful thankful to have someone like you to help me out. Accepted!! – Cyzanfar Nov 16 '18 at 2:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.