-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Be more careful about endianness with Identity and Address #1926
Conversation
This does not presently fix the incompatibility between the |
1bd28aa
to
212e049
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Just what the doctor ordered I think. I left a few small comments. Keen to hear about the motivation for changing the control db key serialization.
Can you be more specific about what incompatibility you mean here? |
Yes, This is slightly silly because we already have the |
On reflection I don't think this needs anything else, besides possibly additional testing, but I'm not sure what that testing would entail. |
Okay, I think I've got everything and this is ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM but internal tests are failing.
Description of Changes
This fixes some issues caused by #1616.
We are now more careful about endianness when converting
Identity
andAddress
to byte arrays / strings.API and ABI breaking changes
ABI break; this will require a wipe of modules, since the controldb has been changed to look up
identities
andaddresses
by their usual printed form, which corresponds to a big-endian byte array.Expected complexity level and risk
5/5
Testing
See the added tests, also running smoketests.
I am not sure what else would be a good idea to do.