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.

Thomas Jay Rush
  • 9,943
  • 4
  • 31
  • 72

2 Answers2

5

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.

Rob Hitchens
  • 55,151
  • 11
  • 89
  • 145
  • 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 06: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 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 08 '18 at 02: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
  • Whats the point of the modifiers? I mean, I would be able to create a function that validates stuff and then call the "validation function" on my other function, right? – redigaffi Mar 29 '21 at 18:04
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.

sofend
  • 241
  • 1
  • 6