bancorprotocol / contracts-solidity

Bancor Protocol Contracts
Other
840 stars 391 forks source link

Move to oz reentrancy guard #610

Closed barakman closed 3 years ago

barakman commented 3 years ago

A minor downside of this is slightly larger bytecode, which means slightly higher deployment gas cost (btw, there's also a minor upside in the form of slightly lower runtime gas cost for users). Of course, the issue here is not gas cost, but the possibility of bytecode size exceeding the maximum limit (24kb). And at least currently, it not the case in either one of the contracts involved in this PR.

Please see OZ code vs ours below.


OZ code:

    modifier nonReentrant() {
        require(_status != _ENTERED, "ReentrancyGuard: reentrant call");
        _status = _ENTERED;
        _;
        _status = _NOT_ENTERED;
    }

Our code:

    modifier protected() {
        _protected();
        state = LOCKED;
        _;
        state = UNLOCKED;
    }

    function _protected() internal view {
        require(state == UNLOCKED, "ERR_REENTRANCY");
    }
}