multiformats / py-multiaddr

multiaddr implementation in Python
https://multiformats.io/
Other
33 stars 23 forks source link

Change hexlified bytes to raw bytes in Multiaddr? #28

Closed mhchia closed 5 years ago

mhchia commented 5 years ago

Currently, if we initialize a Multiaddr, the to_bytes() will output a hexlified value in bytes format. We need an additional binascii.unhexlify to get the raw bytes.

>>> m1 = Multiaddr("/ip4/127.0.0.1/udp/1234")
>>> m1.to_bytes()
b'047f0000011104d2'
>>> binascii.unhexlify(b'047f0000011104d2')
b'\x04\x7f\x00\x00\x01\x11\x04\xd2'

IMO we can just keep the raw bytes in Multiaddr, e.g. b'\x04\x7f\x00\x00\x01\x11\x04\xd2' in the above example. This removes the dependency on binascii, additional converting overhead(we handle raw bytes in [de]serializations). And also aligns with the Multiaddr.Bytes() in go-multiaddr.

What do you guys think? cc @raulk @zixuanzh @zaibon @robzajac @alexh

raulk commented 5 years ago

As a developer I would expect to_bytes() to return a byte literal. So yes, I tend to agree with this change. We can add a to_hex() for syntactic sugar – I'm not super worried about the binascii dep as it's built in.

mhchia commented 5 years ago

Thanks for the feedback. Good point for the dep. And this change also affects the return types, which might cause the code dependent on this package to fail.