Solidity – SafeMath Safe Add Function Assertions Against Overflows

Architectureoverflowsolidity

Looking at the following safeAdd function, which is common in many smart contracts out there, it seems that only a and c are compared. But can't it be that b will be the uint that will cause an overflow?

  function safeAdd(uint a, uint b) internal returns (uint) {
    uint c = a + b;
    assert(c >= a);
    return c;
  }

Why is compering only a and c is sufficient?

assert(c >= b && c >= a);  <- Why not like this?

Best Answer

Firstly, kudos to the OpenZeppelin safeMath library parts of which I've used in my code.

Given that addition is commutative, it doesn't really matter which you use. The answer will either be equal or greater than both and so valid, or less than both, invalid.

Say we have an uint3 (for simplicity but not a valid Solidity type) where overflow is mod 8:

1+7 = 0 0 < 2 && 0 < 7

7+1 = 0 0 < 2 && 0 < 7

Overflowed

0+7 = 7 7 > 0 && 7 >= 7

7+0 = 7 7 >= 7 && 7 > 0

-edit-

Aside from the question, I might add that I use this pattern in solidity 0.4.10 (or above) to separate side effects and validation rather than a more expensive function call.

uint _check = c; // where c is a variable being updated
    c = a + b;
assert(c >= _check);

Related Solutions

[Ethereum] Function with returns compiles without return statement

The statement "return x" is equivalent to "r = x; return;" where r is the return variable. As all variables are automatically assigned a "zero value" (including the return variables), not using "return" is not unsafe but it might indicate a programming error.

Especially if you name the return variables, code that does not use an explicit "return" is sometimes easier to read and check.

Solidity Mapping – How to Return a Mapping Type

You need to break the interface down into smaller chunks. In practice, you'll want to extend the internal storage pattern a little, I think.

Iterable mapping comes to mind. You keep lists of keys to mappings in arrays. Something like this:

  struct VoterStruct {
    address voterAddress;
    uint256 tokensBought;
    mapping (bytes32 => uint256) tokensUsed;
    bytes32[] tokenIndex; // a list of mapping keys that exist for the voter
  }

  mapping (address => VoterStruct) public voterStructs;

Then you can expose functions that allow iterating over lists while the contract returns one set of values at a time. Very sketchy to inspire some ideas:

// client can discover how may tokens are in the list for one voter
function getVoterTokenCount(address voter) returns(uint tokenCount) {
    return voterStructs[voter].tokenIndex.length;}

// client can discover the tokenId for a voter in a given row
function getVoterTokenAtIndex(address voter, uint index) returns(bytes32 tokenId) {
    return voterStructs[voter].tokenIndex[index];}

// client can discover token used by voter and tokenId
function getVoterTokenUsed(address voter, bytes32 tokenId) returns(uint tokensUsed) {
    return voterStructs[voter].tokensUsed[tokenId];}

// append keys to the list as you go
function insertVoterToken(address voter, bytes32 tokenId) returns(bool success) {
    voterStructs[voter].tokenIndex.push(tokenId);
    return true;}

Hope I didn't flub the syntax. It might be possible to sketch out the contract a little more. Would need a general description of what it's going to do. When I use this pattern, I always add some rules to ensure there are no duplicate keys, and so on.

Anyway, possibly the hints will inspire some ideas.

Hope it helps.

Best Answer

Related Solutions

[Ethereum] Function with returns compiles without return statement

Solidity Mapping – How to Return a Mapping Type

Related Topic