0

I am attempting to call an external contract function, but modifier on the function is throwing an error. And I can't figure out why. Any help is appreciated! Info is below:

I have two contracts:

  1. Handles user balances (deposit, withdraw, get balance of addresses)
  2. Interacts with balances contract to add or subtract balance amounts

Error occurs when: I call the "addToAddressBalance" function in the second contract below. When I remove the "onlyOwner" and "onlyAdmin" modifiers from the function, it works as intended.

Error message in remix.ethereum.org when I call the "addToAddressBalance" function: "Gas estimation errored with the following message. The transaction execution will likely fail. Do you want to force sending?"

My code is below:

External Balances Contract:

pragma solidity 0.4.23;

contract Balances {
  using SafeMath for uint256;

  address owner;

  mapping (address => uint) public balances;
  mapping (address => bool) public admins;

  event AdminAdded(address adminAddress);
  event AdminRemoved(address adminAddress);
  event SetOwner(address previousOwner, address newOwner);
  event Deposit(address user, uint256 amount, uint256 userBalance);
  event Withdraw(address user, uint256 amount, uint256 userBalance);
  event AddToAddressBalance(address user, uint256 amount, uint256 userBalance);

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

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

  modifier onlyAdmin {
    require(admins[msg.sender] == true);
    _;
  }

  function setOwner(address _newOwner) public onlyOwner {
    emit SetOwner(owner, _newOwner);
    owner = _newOwner;
  }

  function getOwner() public constant returns (address) {
    return owner;
  }

  function isAdmin(address _address) public constant returns (bool) {
    return admins[_address];
  }

  function isOwner(address _address) public constant returns (bool) {
      return _address == owner;
  }

  function isAdminOrOwner(address _address) public constant returns (bool) {
      return admins[_address] == true || _address == owner;
  }

  function setAdmin(address _adminAddress, bool _isAdmin) public onlyOwner {
    admins[_adminAddress] = _isAdmin;

    if (_isAdmin == true) {
      emit AdminAdded(_adminAddress);
    } else {
      emit AdminRemoved(_adminAddress);
    }
  }

  function deposit() public payable {
    require(msg.value > 0, "Msg.value was not provided or was not greater than zero.");

    balances[msg.sender] = balances[msg.sender].add(msg.value);

    emit Deposit(msg.sender, msg.value, balances[msg.sender]);
  }

  function withdraw(uint _amount) public {
    require(_amount > 0, "Amount was not provided or was not greater than zero.");
    require(_amount <= balances[msg.sender], "Amount provided is greater than balances[msg.sender].");

    balances[msg.sender] = balances[msg.sender].sub(_amount);

    msg.sender.transfer(_amount);

    emit Withdraw(msg.sender, _amount, balances[msg.sender]);
  }

  function addToAddressBalance(address _address, uint _amount) external onlyOwner onlyAdmin {
    require(_amount > 0, "Amount was not provided or was not greater than zero.");

    balances[_address] = balances[_address].add(_amount);

    emit AddToAddressBalance(_address, _amount, balances[_address]);
  }

  function balanceOf(address _address) public constant returns(uint) {
    return balances[_address];
  }
}

library SafeMath {
  function mul(uint256 a, uint256 b) internal pure returns (uint256) {
    uint256 c = a * b;
    assert(a == 0 || c / a == b);
    return c;
  }

  function div(uint256 a, uint256 b) internal pure returns (uint256) {
    uint256 c = a / b;
    return c;
  }

  function sub(uint256 a, uint256 b) internal pure returns (uint256) {
    assert(b <= a);
    return a - b;
  }

  function add(uint256 a, uint256 b) internal pure returns (uint256) {
    uint256 c = a + b;
    assert(c >= a);
    return c;
  }
}

Contract that interacts with Balances contract:

pragma solidity ^0.4.23;
contract Balances {
    function isOwner(address) public pure returns (bool) {}
    function isAdmin(address) public pure returns (bool) {}
    function getOwner() public pure returns (address) {}
    function balanceOf(address _address) public pure returns (uint) {}

    function addToAddressBalance(address _address, uint _amount) {}   
}

contract MainContract {
    Balances balances = Balances(<BALANCES_CONTRACT_ADDRESS>);

    function balanceOf(address _userAddress) public constant returns (uint) {
        return balances.balanceOf(_userAddress);
    }

    function addToAddressBalance(address _address, uint _amount) {
        require(_amount > 0, "Amount was not provided or was not greater than zero.");
        require(isAdminOrOwner(msg.sender) == true, "Msg.sender is not admin or owner. Access denied.");

        balances.addToAddressBalance(_address, _amount);
    }

    function getOwner() public constant returns (address) {
        return balances.getOwner();
    }

    function isAdminOrOwner(address _address) public constant returns (bool) {
        return balances.isOwner(_address) || balances.isAdmin(_address);
    }

    function isAdmin(address _address) public constant returns (bool) {
        return balances.isAdmin(_address);
    }

    function isOwner(address _address) public constant returns (bool) {
        return balances.isOwner(_address);
    }
}

1 Answer 1

1

Sounds like you might have a slight misunderstanding to how calls to smart contracts, and msg.sender works. When you're calling the external Balances contract from MainContract, msg.sender will be the address of the MainContract, not the address of the ethereum account you are using to pay the gas costs, and submit the transaction to the blockchain.

So what's happening here is that when you deploy the balances contract, the owner becomes the address of the account used to deploy the contract. For the addToAddressBalance function you are using the onlyOwner modifier so this will fail. My suggestion would be to use a modifier similar to

modifier onlyAmins() {
     require(msg.sender == owner || admins[msg.sender] == true);
     _;
}

Then add that modifier to all your protected function calls. So long as you appoint the contract address for MainContract to be an admins

Hope this helps!

2
  • Yes this ended up being the issue. Thanks for the help and taking the time!
    – Nick Major
    May 16, 2018 at 3:34
  • No problem! I highly recommend becoming extremely familiar with how call data, and msg.sender behave, it can lead to surprising bugs (such as this) but also some cool tricks.
    – hextet
    May 16, 2018 at 17:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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