Improved error handling style: assert -> require

[tholop]

🚀 Description

Replaced assert convenience function by require following Solidity documentation's advice.

Indeed, such exceptions can be reached even if the contract is correct, and are more a valid condition to be met.

This is not really an issue but a style improvement.

The assert function should only be used to test for internal errors, and to check invariants. The require function should be used to ensure valid conditions, such as inputs, or contract state variables are met, or to validate return values from calls to external contracts. If used properly, analysis tools can evaluate your contract to identify the conditions and function calls which will reach a failing assert. Properly functioning code should never reach a failing assert statement; if this happens there is a bug in your contract which you should fix.

http://solidity.readthedocs.io/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions

• 📘 I've reviewed the OpenZeppelin Contributor Guidelines
• ✅ I've added tests where applicable to test my new functionality.
• 📖 I've made sure that my contracts are well-documented.
• 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).

[shrugs]

Those asserts are invariants, and follow the documentation correctly :)

#565

[mxmauro]

Correct me if I'm wrong.

Because SafeMath uses asserts then we should take care to verify input, i.e, using 'require'. This may lead in a double verification of inputs.

[leonardoalt]

@shrugs I disagree that those asserts are invariants.
For example, take the sub code:

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

b <= a is not an invariant, since you can have pretty much any values reaching this point.
This code is pretty much filtering inputs, which is exactly Solidity's recommendation for require.
asserts should be used for conditions that have to be true at the point in the code, meaning before the line is executed, which is totally not the case here.

[shrugs]

@leonardoalt

I guess the correct thing I should have said is is they "should be invariants". But obviously they aren't because nobody is doing requires before calling safemath anyway.

I've come around to your argument in #1120, so let's continue it there?