Data Location Assignment

[zai]

User is declared as a state variable therefore it is automatically and permanently stored in the storage. With the memory keyword after User inside the updateBalance function, it created a new User in the memory and no longer referencing to the User declared as a state variable.

This is my solution:

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User storage user = users[id]; // changed memory to storage
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

[grondon]

pragma solidity 0.7.5;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

function addUser(uint id, uint balance) public {
    users[id] = User(id, balance);
}

function updateBalance(uint id, uint balance) public {
     User memory user = users[id];
     user.balance = balance;
     users[id] = user;
}

function getBalance(uint id) view public returns (uint) {
    return users[id].balance;
}

}

[ble4h]

function updateBalance(uint id, uint balance) public {
// User memory user = users[id];
users[id].balance = balance;
}

[jon_m]

That’s correct @deanb3, but don’t forget to actually add storage, after deleting memory

@deanb3

thinking structs can't be memory

Struct instances can be stored/located in memory or storage. It just depends how we want to use them. Storing them in memory isn’t wrong if we don’t need to store their values persistently e.g. if we just need them temporarily to perform some kind of operation within the function.

[jon_m]

Hi @gkassaras,

You’ve put a lot of effort and thought into this, and it does work

Just a few observations:

1. You’ve used exactly the same name for the local variable as the mapping: contractUsers

@gkassaras

User storage contractUsers = contractUsers[id];
contractUsers.userBalance +=newBalance;

The compiler should have given you a warning about this in Remix. While this doesn’t prevent your contract from working, it would be better practice to use different names e.g.

User storage userInstance = contractUsers[id];
userInstance.userBalance = newBalance;

However, the fact that you chose to use the same name raises an interesting point. Changing the local variable’s data location to storage makes it a pointer to the mapping i.e. a direct reference and not a copy. This is often useful for clarity when assigning multiple values to more complex data structures. In this more straightforward scenario, by using the same name, you have highlighted the fact that the local variable is an unnecessary additional step, and that we can achieve exactly the same thing by “cutting out the middle man”:

contractUsers[id].userBalance = newBalance;

---

2. The intended functionality of updateBalance isn’t to add an amount to the existing balance, but to update it by replacing it with a new balance. So you only need the assignment operator = and not the addition assignment operator += in the line:
   userInstance.userBalance = newBalance;
   // instead of
   userInstance.userBalance += newBalance;
   I must admit though, it would seem more logical and practical to do it your way, and therefore to also name the input parameter amount instead of newBalance , as this makes more sense.

---

3. The compiler should also have given you warnings about your getBalance functions (both the private and the public one), advising you to add the view keyword. This is appropriate because these functions only read the contract state, but don’t modify it. In Remix, doing this will also turn the call buttons blue for these functions, enabling you to see the output directly below the buttons, instead of having to open and scroll through the transaction data in the Remix terminal.

[jon_m]

Nice solution @zai

Just some comments about your explanation:

@zai

User is declared as a state variable therefore it is automatically and permanently stored in the storage.

@zai

struct User{
        uint id;
        uint balance;
    }

Here, User is not a state variable, it is a struct which defines a new data type (User). It acts like a template without any assigned value(s) until it is used to create a new instance. The new data type User can be used in a new variable declaration to create either a local variable or a state variable with a value of type User assigned to it.

In the updateBalance function, when we change the data location of the local variable user from memory to storage, we aren’t creating a state variable. The variable is still local, but it now acts as a pointer (a reference) to users[id] — the User instance with the key id in the users mapping.

In this assignment, the users mapping is acting as a state variable, its values stored persistently in storage.

---

So, in this statement…

@zai

With the memory keyword after User inside the updateBalance function , it created a new User in the memory and no longer referencing to the User declared as a state variable.

instead of:
"… no longer referencing to the User declared as a state variable.
this should read something like:
"… no longer referencing the User instance (with ID of "input parameter id") saved persistently in storage in the users mapping.

I hope this makes sense, but do let us know if anything is unclear or if you have any questions

[AaronRene]

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User memory user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

[gkassaras]

Hi @jon_m

You are correct the remix online compiler did give me warnings. The main reason I wrote the code this way is also for clarity! From my perspective:

User storage contractUsers = contractUsers[id];
contractUsers.userBalance +=newBalance;

More clear compared to:

User storage userInstance = contractUsers[id];
userInstance.userBalance = newBalance;

Gives more clarity in the purpose of the code, meaning we add extra lines to allow easier debugging and code auditing. What would be interesting is how the contract cost increases or decreases, depending on expressions.

But I am sure that after optimisation the lines will be replaced with the same byte codes.

[DUUKA_SMASH]

HELL YEA!!! I guess that was one of the solutions!!! EEEFFFFF YEAAAA
DDDDUUUUUUKKKKKKKAAAA!!!

Im going to be honest. I have absolutely no idea how to update the balance. So i just cheated and switched memory to storage in the line after my “updateBalance” function.
Im sorry man i really need to see what the solutions were cause all i did was basically make
my updateBalance function work like my addUser function. -__-. I looked over the videos again to relearn memory and storage and i couldnt figure it out. All i know is that memory doesnt work outside the function after its completed. Duuka… :*( I will do my best to learn from this failure, however its time to move on after 4hrs of trying to figure this out lol.

pragma solidity 0.7.5;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

function addUser(uint id, uint balance) public {
    users[id] = User(id, balance);
}

function updateBalance(uint id, uint balance) public {
     User storage user = users[id];
     user.balance = balance;
}

function getBalance(uint id) view public returns (uint) {
    return users[id].balance;
}

}

[lucas_pace]

    function updateBalance(uint id, uint balance) public {
         User memory user = users[id];
         user.balance = balance;
         users[id] = user;
    }

or

    function updateBalance(uint id, uint balance) public {
        users[id].balance = balance;
    }

I dont know if for some reason we need the object “User”, but i guess we can just change the array directly. If we need the object for some reason, we can just point the array content on index to the created object.

[Richard_Bond]

Ok, so it looks like the error is occuring because we are updating the variable “user” in memory but not the User instance inside the users mapping.

Here’s my solution:

function updateBalance(uint id, uint balance) public {
         users[id].balance = balance; 
         // Instead of copying the user to memory, we can update the user directly in storage
    }

[saf]

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance ) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance ) public {
         User storage user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

Correction: Replace ‘memory’ with ‘storage’ in updateBalance.

[Stequal]

function updateBalance(uint id, uint balance) public {
     User storage user = users[id];
     user.balance = balance;
}

The quickest method to fix the updateBalance function would be to replace User memory user = user[id]; to User storage user = user[id]; I don’t know if this is the most efficient solution as by replacing memory with storage would make it so that the information is permanently stored within the contract before it goes to update the users balance.

My only question would be that if we were to do this, wouldn’t every update to the balance be stored in the contract needlessly wasting space?

[MindFrac]

All I did was change memory to storage.

solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User (id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User storage user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

[Cristi_Preda-Carp]

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User storage user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

• Changed memory to storage in the updateBalance function, changing storage location from memory to storage will not create a new copy of data, rather it will create a pointer to storage. Hence any change in user object will be reflected in the storage itself.

[alve_lagander]

pragma solidity 0.7.5;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

function addUser(uint id, uint balance) public {
    users[id] = User(id, balance);
}

function updateBalance(uint id, uint balance) public {
     
     users[id].balance = balance;
}

function getBalance(uint id) view public returns (uint) {
    return users[id].balance;
}

}

[Rishi_Remesh]

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         users[id].balance = balance ; // - answer 
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

[zai]

Hi @jon_m. It took me a long time to make a reply on this comment of yours for honestly I didn’t understand it the first time. I’m very new to programming (only 3 months) so I needed to read a lot of documentations about solidity (the many words and its meaning). And it just made me realize I still really need to read more and practice what I read. Lol

Anyways, reading it again now, I felt like I need to say thank you for your explanation. It surely shed some light to my confuse mind. Thank you so much!

If there’s anything you can suggest that I read to fast track my learning that would be very much appreciated.

[jon_m]

Hi @gkassaras,

Valid points

I did actually look at the gas cost differences a while back, and changing memory to storage (and keeping 2 lines of code) was only very very slightly more expensive than the more concise one line  contractUsers[id].userBalance = newBalance; However, as you predict, the gas cost is exactly the same with optimization, which makes sense as both versions are essentially performing exactly the same operations. So, I think it’s more down to personal style, and also, as you say, which one allows easier debugging, code auditing and so, ultimately, more security. And having had a look at your profile, I can see you’ve had many years of experience in program security, security testing, security assurance and risk assessment. So thank you very much for your input, comments and well-reasoned opinion about the alternative solutions to this exercise from a security and assurance point of view — it makes for a more interesting and thought-provoking discussion.

@gkassaras

User storage contractUsers = contractUsers[id];
contractUsers.userBalance +=newBalance;

If we change the functionality of the addBalance function to add an amount to a user’s existing balance (instead of replacing it with a newBalance), do you not think that it would be clearer to rename the function deposit (or similar) and rename the parameter newBalance => amount (or similar)? We are adding an amount to an exisiting userBalance to create a newBalance, rather than adding the newBalance to the exisiting userBalance. Obviously, the assignment operator = or += makes it clear whether we are adding or reassigning, but don’t you think that this is also better reflected in the names we choose as well?

[jon_m]

Nice solution, and great question @Stequal

@Stequal

I don’t know if this is the most efficient solution as by replacing memory with storage would make it so that the information is permanently stored within the contract before it goes to update the users balance.

@Stequal

User storage user = users[id];

This line isn’t making a copy or saving any new data in persistent storage. Instead, it creates a pointer (a reference) to a specific User instance in the users mapping — the one which has the id parameter input into the function as its key). This means that when balance is assigned to user.balance in the second line it is effectively being assigned to users[id] in the mapping. This means the update is made to persistent storage, instead of just being saved temporarily in memory within the function until the function finishes executing.

@Stequal

My only question would be that if we were to do this, wouldn’t every update to the balance be stored in the contract needlessly wasting space?

So, no — this code doesn’t save every updated balance as separate values within persistent storage. Instead, each time the balance is updated, the exisiting balance is overwritten with the new one (the value is reassigned).

@Stequal

I don’t know if this is the most efficient solution

In terms of comparing the efficiency of different alternative solutions, have a look at the following 3-post discussion, which I think will get you thinking about the different factors that need to be considered:
https://forum.ivanontech.com/t/data-location-assignment/27990/17?u=jon_m
https://forum.ivanontech.com/t/data-location-assignment/27990/20?u=jon_m (reply to previous post)
https://forum.ivanontech.com/t/data-location-assignment/27990/31?u=jon_m (reply to previous post)

Let us know if this answers your question, and if it gives you a better understanding of what the code is actually doing