1

I was trying to create a simple quote registry having a mapping defined as:

mapping (string => quoteOwner) quoteRegistry;

where quoteOwner structure is:

struct quoteOwner{
    address ownerAdd;
    bool hasOwner;
}

I did extend typical owned contract and I identified that contract owner's address went on changing as soon as I called functions like register_quote.

The code I was using initially was:

quoteOwner new_owner;
new_owner.ownerAdd=msg.sender;
new_owner.hasOwner=true;
quoteRegistry[_quote]=new_owner;

and I figured out,the line new_owner.ownerAdd=msg.sender; was causing the issue. As soon as I changed the above to simple assignment:

quoteRegistry[_quote]=quoteOwner(msg.sender,true);

Everything is working perfectly fine! Can anyone help with what is wrong with the former method?

3

This line should have caused a couple of compiler warnings:

quoteOwner new_owner;

The implicit location for new_owner is storage. This is better written explicitly:

quoteOwner storage new_owner;

The other warning is that this is an uninitialized storage reference. If you've used languages like C, this is roughly the equivalent of an uninitialized pointer. It points to slot 0 in storage. That's where the first state variable in your smart contract resides. So changing new_owner will overwrite that first state variable, whatever it is.

There are two potential fixes. The first fix is to use a memory variable:

quoteOwner memory new_owner;
...
quoteOwner[_quote] = new_owner;

Here, you've declared the variable is residing in memory. This gives you a newly-allocated struct to work with, and then the value is copied into storage in the last line.

The second fix is to initialize the storage reference:

quoteOwner storage new_owner = quoteOwner[_quote];
new_owner.ownerAdd = msg.sender;
new_owner.hasOwner = true;

This points new_owner directly at the slot in storage you're trying to edit. There's no need to do an assignment at the end, because you were already editing the right data in storage.

The latter approach is slightly less code and slightly more gas efficient, so it's what I would recommend.

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.