Key hashing compatibility #333
Conversation
austinabell
requested review from
ansermino,
dutterbutter and
ec2
as code owners
| Some(v) => v.0 + value, | ||
| None => value, | ||
| }; | ||
| Ok(self.0.set(key.hash_key(), BigUintDe(new_val))?) | ||
| Ok(self.0.set(key.to_bytes().into(), BigUintDe(new_val))?) |
There was a problem hiding this comment.
you could just implement impl<T: AsRef<str>> From<T> for BytesKey and avoid all these to_bytes calls
There was a problem hiding this comment.
hmm, I don't think this helps, I'm not sure if you're seeing that the key is of the address type, it is being converted into a Vec<u8> and then the into() is putting it as the wrapper type used as a key here (BytesKey) which is specifically needed to ensure the string serialization of those bytes.
Edit: Yeah this wouldn't work because Address should (and could) never be AsRef<str>. I also do not like how there is this chained call, but I wanted to keep the Address and Key type seperate
There was a problem hiding this comment.
Ah sorry, .to_bytes() made me think it was a string,
|
Gonna let this PR hang for a bit, was indicated that the key serialization is going to be fixed in go impls, so might as well put this on ice to not have to revisit |
|
|
Summary of changes
Changes introduced in this pull request:
BytesKeybut open to a better name) for the type of the key to match.-The constraints on this type is that it has to hash the same as go implementation and that it serializes the same. str and [u8] hash the same with our impl so it's fine to use bytes as the hash type and that matches, and to get the serialization to match the bytes need to be unsafely converting to a string before serialization.
Edit: go impl changed to serialize keys as bytes, so this update changes key type used within Actors
Reference issue to close (if applicable)
Closes
#324
Other information and links