-
Notifications
You must be signed in to change notification settings - Fork 27
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
Consider adding byte packing utils #435
Comments
Existing implementation for more efficient unpacking by @ARR4N is limited to at most one dynamic field. We should verify if a similar solution is possible with multiple dynamic length fields and close if not |
Confirmed that this is a fundamental issue therefore closing. |
Adding the buffer length immediately before the packed bytes provides an elegant way to decode it because that mirrors the way Solidity encodes arrays in memory. It means that you can just point some output variable at the word in memory that encodes the length. My (jet-lagged) first thought is that this should still work even if the length word isn't on a regular memory-word boundary, but there's a small chance you'll either have to copy memory for decoding or have some empty bytes in the encoding. |
Changing the encoding in general to either account for @ARR4N do you think that doing some sort of annotation or special handling of blsPubKey length fields to assume that it's 48 bytes is possible? It looks like the intention of your PR is to call |
Sorry, I'm not sure why I wasn't notified about your reply.
Yup. There will be some nuance around the ability to unpack in place vs having to use |
Context and scope
The Staking Manager contracts pack and upack Warp messages to/from byte strings. This involves bit manipulation which can likely be extracted into libraries.
@ARR4N 's POC here #482 is the desired approach but it needs to be extended to support our encoding schema.
It i's impossible to decode a message with more than one dynamic length field without knowing the lengths ahead of time. We encode lengths for all dynamic length fields right now in the field prior to the dynamic length field except for the blsPublicKeys which are packed straight to 48 bytes but are still dynamic
bytes
field on the Solidity side.To use the approach above we need to account for both cases. One way of handling it might be to add an annotation to the blsPublicKey field that explicitly sets it's length to 48 and assume that all other dynamic length fields are directly preceeded by their encoded length.
The text was updated successfully, but these errors were encountered: