Conversation
zancas
left a comment
There was a problem hiding this comment.
I'm concerned about these byte order bugs. This is not the first time they've come up.
I don't understand how removing this explicit method, and directly exposing the .0 internal field PROVES that the bytes are in the correct order for the possible use contexts.
I don't know the correct solution to this issue, but it's a subtle and persistent issue, we should approach it with care.
my understanding here (also based on responses from ECC engineers on this topic) is that we should only be reversing bytes during hex string encoding or decoding. I can not find anywhere else in the code base other than the places fixed in this PR that reverse bytes without immediately encoding/deciding to/from hex strings and this is the way it should be. IMO it is unecessary to add additional bloat to further complicate the reversal interface... we should just only use the provided exisitng trait methods for hex encoding/decoding with include the reversal and never reverse bytes directly. this may be secured by actually removing the byte reversal methods or making them private or less public. This PR removes the only case of a direct reversal AFAIK so when this lands we will have consistency here and of course fix a bug blocking chain index integration |
This bug was found during the chain index integration (#596 and #625) and this work allowed failing tests to then pass.
Motivation
Solution
Tests
Specifications & References
Follow-up Work
PR Checklist