4

I have the following function in Solidity which takes as arguments 2 arrays, an array of shareholder addresses and an array of their stakes. I'm keeping an array of shareholders in storage, together with a map to their stakes.

If the updated array is the same size, it's simple, just overwrite each position with the new values. If they are different sizes however, I first go through the entire array and delete each element, and then insert the new ones. I feel this is not very efficient and it could be done better.

PS: I am a complete beginner to Solidity and this is my first contract, so please feel free to let me know if I'm doing anything stupid or inefficiently.

Thanks !

function setShareholders(address[] _shareholders, uint256[] _stakes) public onlyCEO {
    require(_shareholders.length > 0);
    require(_shareholders.length == _stakes.length);
    uint256 accummulator = 0;
    for(uint8 x = 0; x < _stakes.length; x++){
      require(_addressNotNull(_shareholders[x]));
      require(_stakes[x] > 0 && _stakes[x] <= 100);
      accummulator = SafeMath.add(accummulator, _stakes[x]);
    }
    if(accummulator == 100){ // stakes need to add up to 100%
      _setShareholders(_shareholders, _stakes);
    }
  }

function _setShareholders(address[] _shareholders, uint256[] _stakes) private {
    if(_shareholders.length == shareholders.length){
      for(uint8 x = 0; x < shareholders.length; x++) {
        shareholders[x] = _shareholders[x];
        shareholderToStake[_shareholders[x]] = _stakes[x];
      }
    }
    else {
      for(x = 0; x < shareholders.length; x++) {
        delete shareholders[x];
        shareholders.length--;
        delete shareholderToStake[shareholders[x]];
      }
      for(x = 0; x < _shareholders.length; x++) {
        shareholders.push(_shareholders[x]);
        shareholderToStake[_shareholders[x]] = _stakes[x];
      }
    }
  }

1 Answer 1

2

In theory, this would work...unfortunately in solidity, managing arrays is a costly nightmare. Doing any array manipulation, on not just one but 2 arrays, is not recommended at all.

You could keep your array of shareholders...but from there, I'd recommend creating a mapping of address->structure. This way you can loop through your mapped structure and contain all necessary data within that.

So something like this for your refactor:

address[] public shareholders;

struct ShareHolder {
  uint stake;
  // ...other awesome data here
}

mapping (address => ShareHolder) public shareholderData;

This way, you have your shareholders list. And you can directly access a shareholder with shareholderData[<SHAREHOLDER ADDRESS>].

Sign up to request clarification or add additional context in comments.

3 Comments

In your code I assume you meant to write mapping (address => ShareHolder) public shareholderData; right ? Also, there is still the issue of replacing shareholders. By following this approach I basically have to delete entries in just the shareholders array, and not from the shareholderToStake map if I am not mistaken, which would be better indeed. I still feel there's a better way of clearing out the shareholders array though.
@c0mpute correct! As I mentioned in the answer as well. Updated. Created the code on the fly. Good catch!
@c0mpute And the idea here is not to delete anything in the shareholders array, but to do all of your validation based on your ShareHolder structure in the shareholderData. So you can add whatever parameters in the struct itself like bool stillShareholder.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.