`ReadableAttributeDict` is not well implemented!

[wiseaidev]

• Version: Any
• Python: Any
• OS: Any

What was wrong?

Not sure if this bug is relevant. But, the other day, I was playing around with the datastructures module and found a bug in ReadableAttributeDict class from which I understood it doesn't allow items' assignment. However, it turns out there is a hacky way to change the value of a given key.

Please include any of the following that are applicable:

• The code which produced the error

$ python -i web3/datastructures.py

>>> foo_dict =ReadableAttributeDict({'foo': 'foo', 'bar': 'bar'})
>>> foo_dict['foo'] = "let's foo this"
TypeError: 'ReadableAttributeDict' object does not support item assignment

Till this point, it behaves as expected, however you can do the following for key-val assignment:

>>> foo_dict.foo = "let's foo this"
>>> foo_dict
ReadableAttributeDict({'foo': "let's foo this", 'bar': 'bar'})

And voila! The ReadableAttributeDict instance is a mutable object. It turns out that each key becomes an attribute for every instance of that class. We can even confirm this by calling __dir__:

>>> foo_dict.__dir__()
['foo', 'bar', '__module__', '__doc__', '__init__', '__getitem__', '__iter__', '__len__', '__repr__', '_repr_pretty_', '_apply_if_mapping', 'recursive', '__orig_bases__', '__dict__', '__weakref__', '__parameters__', '__abstractmethods__', '_abc_impl', '__slots__', 'get', '__contains__', 'keys', 'items', 'values', '__eq__', '__reversed__', '__hash__', '__subclasshook__', '__str__', '__getattribute__', '__setattr__', '__delattr__', '__lt__', '__le__', '__ne__', '__gt__', '__ge__', '__new__', '__reduce_ex__', '__reduce__', '__init_subclass__', '__format__', '__sizeof__', '__dir__', '__class__', '__class_getitem__', '_is_protocol']

Notice the appearance of foo and bar at the beginning as attributes/fields.

How can it be fixed?

Reimplement this class as an immutable dict, I guess?

---

Note: We prefer to use issues to track our work. If you think you've encountered a bug in web3py or have a feature request, you're in the right place. If you have implementation or usage questions, please refer to our documentation and/or join the conversation on discord.

[fselmo]

I'm not quite sure this is a bug. ReadableAttributeDict doesn't look like it is used directly. It's used to build MutableAttributeDict and AttributeDict (which is indeed immutable). Thoughts on that?

[wiseaidev]

AttributeDict (which is indeed immutable).

Well, it turns out to be mutable:

$ python -i web3/datastructures.py

>>> foo_dict = AttributeDict({'foo': 'foo', 'bar': 'bar'})
>>> foo_dict.__dict__['bar'] = "let's foo this"
>>> foo_dict
AttributeDict({'foo': 'foo', 'bar': "let's foo this"})

I think this is what this docstring mean:

https://github.com/ethereum/web3.py/blob/ca662c879e95e9e184d9dfdb78855492e1d7db23/web3/datastructures.py#L97-L99

Besides, I believe that there is room for improvement in the implementations. I am currently doing some investigations on this module.

edit: same for del:

the following is not legal:

>>> del foo_dict.foo
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "web3/datastructures.py", line 108, in __delattr__
    raise TypeError('This data is immutable -- create a copy instead of modifying')
TypeError: This data is immutable -- create a copy instead of modifying

However, the following is allowed which shouldn't be:

>>> del foo_dict.__dict__['foo']
>>> foo_dict
AttributeDict({'bar': "let's foo this"})

[fselmo]

I think that's indeed what the docstring is trying to convey. Improvements are certainly welcome.

[wiseaidev]

It is quite funny that I took this issue so seriously to the point that I have created a brand new modern package, named frozndict for having a frozenset like data structure. The rabbit hole I went through was so profoundly deep. It was an enjoyable experience. But, holy smokes!

Back to this issue, I will apply what I think of reasonable fixes for it.

[ubordignon]

Well, it turns out frozndict is mutable too:

>>> d = frozndict.frozendict(dict(a=1))
>>> d
frozendict({'a': 1})
>>> dict.__delitem__(d, "a")
>>> d
frozendict({})

Maybe we can open an issue on cpython to ask them to support actually private attributes?

[wiseaidev]

Thanks for pointing this out, I wasn't aware that it is actually allowed in python! Because of this, I have just discovered that there is a bug in the dict cpython implementation. The interesting part is that some of the dunder methods of the dict class don't take into consideration the instance's type before doing data manipulation on its fields. Take the following as example:

>>> class Foo:
...   def __init__(self, foo: str):
...     self.foo: str = foo
... 
>>> foo = Foo('value')
>>> foo.foo
'value'
>>> type(foo) == dict
False
>>> isinstance(foo, dict)
False

Everything is looking normal till this point. However, It turns out that python allows the following, which shouldn't be, i guess?

>>> dict.__setattr__(foo , 'foo', 'new value')
>>> foo.foo
'new value'

How in the world is this even allowed? Shouldn't dict.__setattr__ check for foo's type, which is not dict in this case, before setting the value for its attribute foo?

Maybe we can open an issue on cpython to ask them to support actually private attributes?

Yeah. I think we need to file an issue for this and/or for fixing the bug described above. What do you think? It is a bug. Isn't it?

Besides that, when I was developing the frozndict package, I was only thinking about overriding the dunder methods to make instances of type frozendict to be immutable, as shown in the test cases.

[wiseaidev]

I am closing this issue because there is no workaround for the so-called true immutability in python. Edit: Even if you use third-party packages such as attrs.frozen.

[ubordignon]

As it turns out attrs.frozen seems to be affected by the same condition:

from attrs import frozen

@frozen
class FrozenAttrs:
    x: int

Testing:

>>> fa = FrozenAttrs(1)
>>> fa
FrozenAttrs(x=1)
>>> fa.x = 2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/local_path/lib/python3.9/site-packages/attr/_make.py", line 642, in _frozen_setattrs
    raise FrozenInstanceError()
attr.exceptions.FrozenInstanceError
>>> object.__setattr__(fa, "x", 2)
>>> fa
FrozenAttrs(x=2)

There's no escaping this.

Yeah. I think we need to file an issue for this and/or for fixing the bug described above. What do you think? It is a bug. Isn't it?

You should probably try!

[wiseaidev]

As it turns out attrs.frozen seems to be affected by the same condition:

I just figured that out.

Key takeaway:

• Python is a bad choice when it comes to immutability. Consider using statically typed languages such as C++, Java, and C#.