4

I have a modifier in my code:

modifier isOkay(address addr) {
    if (balances[addr].balance == 0)
        throw;
    _;
}

Which I used thus:

function whatever() isOkay(msg.sender) {
    // whatever
}

I want to use the modifier as a boolean test insider of other functions, thus:

function other_function() {
    // some other function
    if (isOkay(msg.sender)) {
       // do whatever
    }
}

Is that possible? I see an obvious way to do it (have the modifier call the function), but I wonder if there's a more elegant pattern to use.

4

No, but you can make some consistent and readable code

pragma solidity 0.4.19; 

contract Okay {

    modifier onlyIfOkay() {
        require(isOkay(msg.sender));
        _;
    }

    function isOkay(address checkOkay) public pure returns(bool isIndeed) {
        // check something
        return checkOkay==checkOkay;
    }

    function restricted() public view onlyIfOkay {
        // carry on
    }

}

That gives you the option of using isOkay(addr)/!isOkay(addr) when needed inside functions and a simple modifier for guarding functions (it assumes the most common case, we need to check the sender).

There are lots of ways to work out case-by-case details. I would urge caution/avoidance of a somewhat hazardous pattern:

modifier isOkay() { _; if(!ok) revert(); }

The hazard is in putting _; in front of the modifier steps because that might help camouflage a bug.

For the benefit of others:

If a modified function returns (looks normal enough to a reviewer), the modifier code will not execute because it won't get a chance to. It would be equivalent to:

return;
if(!ok) revert(); // <= unreachable code doesn't run

It will only seem like the modifier is in play in every case. Consequently, I'm leery about using that style.

  • I didn't know about that. You're right. Looks a bit hazardous. Can a modifier return a value? Also, will the calling code get inserted twice (once for each '_')? – Thomas Jay Rush Sep 26 '17 at 6:16
  • My understanding is it will put the top code block, then the modifier, then the bottom code block, so looks nice and does what you want, but I don't use it owing to the hazard. – Rob Hitchens - B9lab Sep 26 '17 at 13:25
  • Modifiers aren't intended to be used for this kind of thing. They should be used to validate conditions prior to execution of a function. If you wish to do something like this it is much cleaner and a better practice to have this kind of conditional logic inside a function – hextet Jan 8 '18 at 2:16
  • Are u sure about this hazardous pattern? It seems it is not true or maybe was fixed in some version. – k06a May 10 '18 at 10:22
4

I think the safer way to do this is to decompose the modifier into a function that does the check and then invoke that function in the modifier.

For example:

function hasNonZeroBalance(address addr) returns (bool) {
    return balances[addr].balance != 0;
}

modifier isOkay(address addr) {
    if (!hasNonZeroBalance(addr))
        throw;
    _;
}

function whatever() isOkay(msg.sender) {
    // whatever
}

function other_function() {
    // some other function
    if (hasNonZeroBalance(msg.sender)) {
        // do whatever
    }
}

where the function is called hasNonZeroBalance() (rather than hasZeroBalance()) to keep the semantics aligned with those of the isOkay() modifier. Thus instead of calling isOkay() you just call hasNonZeroBalance().

A bit redundant - but if only a few of the conditionals in your modifier need this it won't add much heft to the code and keeps the logic clear and thus the contract safer.

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.