New temporary struct causing state variables to be overwritten

Question

I'm running an experiment where I'm trying to create the following behavior:

1. Service owner deploys Experiment1 to the blockchain and is recorded as the owner.
2. Owner then adds a user address, a user-defined uint, and a user-defined bytes32.
3. The information is pushed to the arrA array, and the index of the record is mapped to the user's address and pushed to the arrAIndices array.

The problem is in the addToArrA function: it seems that state variables are being overwritten, and I can't understand how to prevent that from happening. In particular, owner is overwritten by _user, and arrA is set to be _AdataA in length. Completely at a loss on how to fix this.

pragma solidity ^0.4.17;

contract Experiment1 {
    address public owner;

    modifier onlyOwner {require(msg.sender == owner); _;}

    struct structA {
        address user;
        uint AdataA;
        bool AdataB;
        bytes32 AdataC;

        structB[] arrB;
    }

    struct structB {
        uint BdataA;
        uint BdataB;
    }

    structA[] public arrA;

    // Solidity cannot return structs, and can only return arrays of addresses, bools, or uints
    mapping (address => uint256[]) private arrAIndices;

    // CurrencyHedge: Contract creator.
    function Experiment1() {
        owner = msg.sender;
    }

    // addToStructAArr: add an element to structAArr
    function addToArrA(address _user, uint _AdataA, bytes32 _AdataC) public onlyOwner {

        structA tempStructA;

        // Populate the information in the new structA.  This method must be used,
        // since struct arrays cannot be passed as parameters.
        // Refer to https://ethereum.stackexchange.com/questions/3525/how-to-initialize-empty-struct-array-in-new-struct
        tempStructA.user = _user;
        tempStructA.AdataA = _AdataA;
        tempStructA.AdataB = false;
        tempStructA.AdataC = _AdataC;

        // Add the structA to the global list, and associate the index of the hedge with the beneficiary
        arrA.push(tempStructA);
        arrAIndices[_user].push(arrA.length - 1);
    }

    // getArrALength: Check on length of arrA
    function getArrALength() public onlyOwner returns (uint) {
        uint length = arrA.length;
        return length;
    }

    // getArrAIndices: Retrieve a list of indices of the allHedges array associated with a particular beneficiary's
    // Refer to https://ethereum.stackexchange.com/questions/3589/how-can-i-return-an-array-of-struct-from-a-function
    function getArrAIndices(address _user) public onlyOwner returns (uint256[]) {
        return arrAIndices[_user];
    }
}

Answer 1

The problem is:

structA tempStructA;

is declared (implicitly) as an uninitialized storage pointer. It's uninitialized because there is no right side expression, e.g. = structAs[index];. It's storage because memory wasn't explicitly declared with structA memory tempStructA;

So, we have a structA starting in storage slot 0 which means the variables are laid out roughly like:

        address user; // slot 0, overwrites owner
        uint AdataA;  // slot 1, overwrites arrA.length;
        bool AdataB;  
        bytes32 AdataC;
        structB[] arrB;

This is Remix warning of multiple issues with the unrepaired code.

You'll run into a follow-up problem trying to push the memory struct into storage. This will call the data structure into question. You can resolve that with the Solidity CRUD pattern. https://medium.com/robhitchens/solidity-crud-part-1-824ffa69509a

Or, go nuts and use the library implementations, here: https://github.com/rob-Hitchens/UnorderedKeySet

pragma solidity 0.5.1;

import "./HitchensUnorderedKeySet.sol";
import "./HitchensUnorderedAddressSet.sol";

contract Experiment1 {

    using HitchensUnorderedAddressSetLib for HitchensUnorderedAddressSetLib.Set;
    using HitchensUnorderedKeySetLib for HitchensUnorderedKeySetLib.Set;

    address public owner;

    modifier onlyOwner {require(msg.sender == owner); _;}
    modifier onlyValidUser(address user) {
        require(addresses.exists(user), "usernot found.");
        _;
    }

    struct StructA {
        uint aDataA;
        bool aDataB;
        bytes32 aDataC;
        HitchensUnorderedKeySetLib.Set bIds;
        mapping(bytes32 => StructB) structBs;
    }

    HitchensUnorderedAddressSetLib.Set addresses;
    mapping(address => StructA) structAs;

    struct StructB {
        uint bDataA;
        uint bDataB;
    }

    event LogNewA(address sender, address A, uint aDataA, bool aDataB, bytes32 aDataC);
    event LogNewB(address sender, address A, bytes32 bId, uint bDataA, uint bDataB);

    // CurrencyHedge: Contract creator.
    constructor() public {
        owner = msg.sender;
    }

    // addToStructAArr: add an element to structAArr
    function addToArrA(address user, uint aDataA, bool aDataB, bytes32 aDataC) public onlyOwner {
        addresses.insert(user);
        StructA storage a = structAs[user];
        a.aDataA = aDataA;
        a.aDataB = aDataB;
        a.aDataC = aDataC;
        emit LogNewA(msg.sender, user, aDataA, aDataB, aDataC);
    }
    function addToArrB(address user, uint bDataA, uint bDataB) public onlyOwner onlyValidUser(user) {
        bytes32 bKey = keccak256(abi.encodePacked(user, bDataA, bDataB, now)); // possibly inadequate attempt at unique ID
        StructA storage a = structAs[user];
        a.bIds.insert(bKey);
        StructB storage b = a.structBs[bKey];
        b.bDataA = bDataA;
        b.bDataB = bDataB;
        emit LogNewB(msg.sender, user, bKey, bDataA, bDataB);
    }
    function getArrAbyAddress(address user) public view onlyValidUser(user) returns(uint aDataA, bool aDataB, bytes32 aDataC, uint bElementCount) {
        StructA storage a = structAs[user];
        return(a.aDataA, a.aDataB, a.aDataC, bCountByAddress(user));
    }
    function bCountByAddress(address user) public view onlyValidUser(user) returns(uint count) {
        return structAs[user].bIds.count();
    }
    function getArrALength() public view returns (uint) {
        return addresses.count();
    }
    function getAddressAtIndex(uint index) public view returns(address) {
        return addresses.keyAtIndex(index);
    }
    function getBIdAtIndex(address user, uint index) public view onlyValidUser(user) returns(bytes32) {
        StructA storage a = structAs[user];
        return a.bIds.keyAtIndex(index);
    }
    function getB(address user, bytes32 BId) public view onlyValidUser(user) returns(uint bdataA, uint bDataB) {
        StructA storage a = structAs[user];
        require(a.bIds.exists(BId));
        StructB storage b = a.structBs[BId];
        return(b.bDataA, b.bDataB);
    }
}

There are opportunities to:

1. Extend with the possibility of removing A and B instances.
2. Enumerate all B structs with pointers to related A instances.
3. Optimize storage gas cost.
4. Reduce the size of the contract.

The focus here is on readability and solving the original problem for this contrived experimental contract.

Hope it helps.

Answer 2

As far as I can tell, it seems that Solidity does not like struct arrays nested inside of structs, and Solidity seems to default 'tempStructA' to storage instead of memory. The workaround is to use mappings instead, and use ID numbers (in this case, array index numbers) to link information together. Instead of posting the whole contract again, I will go through the changes piecemeal, and quote code where necessary.

Remember that in this instance, the contract owner and 'user' / '_user' are not the same.

1) 'structB[] arrB' is removed from structA, and instead the following mappings are made:

// Each address is associated with an array of indices corresponding to particular structA elements in arrA
mapping (address => uint256[]) private arrAIndices;

// A reverse mapping is made to make it easy to verify that a given index is associated with a given address
mapping (uint => address) private arrAToUser;

// Each index of arrA is mapped to a structB[] array
mapping (uint => structB[]) private arrB;

2) The 'addToArrA' function is then modified as such:

function addToArrA(address _user, uint _AdataA, bytes32 _AdataC) public onlyOwner {

    structA memory newA = structA(_user, _AdataA, false, _AdataC);

    // Add the structA to the global list, and associate the index of the hedge with the beneficiary
    arrA.push(newA);
    uint newIndex = arrA.length - 1;
    arrAIndices[_user].push(newIndex);
    arrAToUser[newIndex] = _user;
}

3) If we want to add some structB and associated them with particular structA's, we can add this function:

function addToArrB(address _user, uint256 _index, uint _BdataA, uint64 _BdataB) public onlyOwner {
    require(arrAToUser[_index] == _user);

    structB memory newB = structB(_BdataA, _BdataB);
    arrB[_index].push(newB);
}