2

I had an odd behavior in a contract. Google told me that in C++, that could be caused by booleans not reseting in loops, because of how the code was compiled.

Here is an example of the code that seemed to have failed. It seemed like isOwner was not reset between loops, so that multiple accounts in listOfAccounts had their balance increased.

  address owner;

  for(uint i = 0; i < listOfAccounts.length; i++) {

    bool isOwner;
    if(listOfAccounts[i] == owner) isOwner = true;

    if(isOwner) {
      balanceOf[listOfAccounts[i]] += share;
    }

  }

Not sure what else could have caused the balance to increase for an account where listOfAccounts[i] == owner was not true. But it could have been something else, just wanted to check if this is a possible error, seeing as it could happen in C++.

  • Just to be on the safe side, I would use bool isOwner = false; – The Officious BokkyPooBah Jan 3 '17 at 1:52
  • 1
    Solidity is not C++. The 'isOwner' variable is not local to the 'for' loop. It's function wide. If you look at it that way, it's obvious it's not being reset each time through the loop. – Thomas Jay Rush Jan 3 '17 at 11:04
2

You're right. Once set to true, it looks like it will stay that way. You're initializing isOwner with:

bool isOwner;

Without going into too much detail, the compiler sets up a reservation and address for the variable. By default, the value will be zero or falsy. It's safe to assume the initial value, but it's a mistake to assume that initialization by itself does anything on the next pass.

In other words, the initialization is done, so there's nothing else to do.

I expect this would work because it explicitly sets the value:

bool isOwner = false;  //  = false will execute each time

For clarity and readability, I would initialize it before the loop starts and then explicitly set it when changes are needed:

function test() {

  bool isOwner;

  for(uint i = 0; i < listOfAccounts.length; i++) {

    isOwner = false;

    if(listOfAccounts[i] == owner) isOwner = true;

    if(isOwner) {
      balanceOf[listOfAccounts[i]] += share;
    }
  }  
}

Obviously, it can be more to the point with something like:

if(listOfAccounts[i] == owner) balanceOf[listOfAccounts[i]] += share;

It's always good to avoid loopy processes if possible. If this is working toward something beyond proving details about the compiler, it looks like you can avoid the loop and sum it up with:

balanceOf[owner] += share;

I'm assuming owner is an address and your code initializes balanceOf with a mapping like:

mapping(address=>uint) balanceOf;

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.