UnsignedMessage cbor encoding #174
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
austinabell
requested review from
ansermino,
dutterbutter and
ec2
as code owners
ec2
approved these changes
Jan 21, 2020
vm/message/src/unsigned_message.rs
Outdated
| impl UnsignedMessage { | ||
| pub fn builder() -> MessageBuilder { | ||
| MessageBuilder::default() | ||
| } | ||
| } | ||
|
|
||
| /// Structure defines how the fields are cbor encoded as an unsigned message | ||
| #[derive(Serialize, Deserialize)] | ||
| struct CborUnsignedMessage( |
There was a problem hiding this comment.
that was refactored 4 hours ago lol how did you even review that? And no, didn't need to be since it was just used in that file for consistent ser/de
| @@ -7,20 +7,20 @@ use std::ops::Deref; | |||
|
|
|||
| /// Method number indicator for calling actor methods | |||
| #[derive(Default, Clone, Copy, PartialEq, Debug, Serialize, Deserialize)] | |||
| pub struct MethodNum(i32); // TODO: add constraints to this | |||
| pub struct MethodNum(u64); // TODO: add constraints to this | |||
There was a problem hiding this comment.
Should this not be an enum? Then you can impl the From traits to convert from a number type
There was a problem hiding this comment.
nah, needs to be an integer because methods vary based on actor and needs to be future proofed for when you can initialize own actor with custom code
| assert_eq!(enc_bz.as_slice(), expected); | ||
| // Assert decoding from those bytes goes back to unsigned message | ||
| assert_eq!( | ||
| &from_slice::<UnsignedMessage>(expected).expect("Should be able to deserialize cbor bytes"), |
There was a problem hiding this comment.
Nit: all .expect other than this one begins with a lowercase.
There was a problem hiding this comment.
ah ya, is just used for easier test debugging instead of unwrapping. I made all consistent.
dutterbutter
approved these changes
Jan 21, 2020
ec2
approved these changes
Jan 21, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Match format with one defined in Lotus.
Once these are done I will open PR