Make DataWord immutable

[Nashatyrev]

Mutable DataWord introduces wide possibilities for hardly debugable issues while performance gains are not evident in the current implementation

[arsalankhalid]

I'd like to take this over if no one is handling it, any advice for approaching this?

[Nashatyrev]

@arsalankhalid Just make the DataWord class immutable (enclosing byte[] data shouldn't change), fix all occurrences and make sure the whole live blockchain can be imported without errors

[arsalankhalid]

Sounds good, thank you!

[arsalankhalid]

What about all the setters in the class? Would they need to be abstracted into a mutable class?
I.e:

// TODO: improve with no BigInteger
    public void sub(DataWord word) {
        BigInteger result = value().subtract(word.value());
        this.data = ByteUtil.copyToArray(result.and(MAX_VALUE));
    }

// TODO: improve with no BigInteger
 public void exp(DataWord word) {
     BigInteger result = value().modPow(word.value(), _2_256);
     this.data = ByteUtil.copyToArray(result);

It seems like we're actually setting the values of this.data, so the class simply can't be made immutable by adding final and privatizing the object's instances.

Do you follow what I'm saying, or am I just totally wrong here?

Cheers!

[chimbs86]

Is this still in progress? I was looking for an easy low hanging fruit to jump into development for this project. And this definitely looks like a great task. If this is still available... since I see the last progress was > 2 months ago. If this is not, it would be a really great help if someone could point me to a similar issue that I can pick up. I will work hard... I promise :-)

[akibabu]

Is this still in progress?

The first person seemed to misunderstand immutability so I opened a PR on this 2 weeks ago you could merge and start from. Like you felt like doing some simple task to check the code base out. I debugged for a while but didn't find the reason a few integration tests started to fail with the change.
Feel free to take over my PR, you probably need to spend some time with the code to find it, reasonable to believe the mutable nature is assumed somewhere I didn't look.

Also didn't do anything showing the performance drop.

[chimbs86]

Picked it up. I see the GitHubStateTest are failing. Ill see what I can do. Thanks for the pointer

[danielsteward]

Hi chimbs86. I am a noobie looking to get involved but need a bit of hand-holding. Maybe I could help you out on this one please?

[chimbs86]

haha, same here. if you can, sure. I was able to check out and reproduce the failing tests that sebull mentioned.

[danielsteward]

Hey, I don't want to encroach on your territory! If there is something you haven't got time for just let me know - dansteward@gmail.com.

[tonycox]

@arsalankhalid Hi, are you going to continue on this?