Skip to content
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

Refactor Message type and vm packages #79

Merged
merged 12 commits into from
Dec 11, 2019
Merged

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Dec 7, 2019

Edit: Refactored the vm components into crates also in this PR to get those changes in ASAP. This will be important to do now to not have conflicts and an extremely complicated refactor later. If you look at the structure, the vm crate is primarily used for the vm types currently and the logic and functionality and types used commonly in other packages (message, address) into their own package.

Message refactor info:
After building out the System actors it became apparent that there needs to be some slight changes to the message types and there will probably be a need to iterate over ambiguously signed and unsigned messages.

This moves the Message struct to UnsignedMessage and adds a Message trait which both signed and unsigned messages will implement. Ideally, you would be able to iterate over signed messages and unsigned messages seperately, but if you need to iterate over both types at once (for readability and reduce redundancy) you can just iterate over a pointer to this Message trait implementation.

@austinabell austinabell added the VM label Dec 7, 2019
@austinabell austinabell changed the title [WIP] Refactor Message type Refactor Message type and vm packages Dec 10, 2019
@austinabell austinabell marked this pull request as ready for review December 10, 2019 18:34
vm/message/Cargo.toml Outdated Show resolved Hide resolved
}

impl Cbor for SignedMessage {
fn unmarshal_cbor(_bz: &[u8]) -> Result<Self, EncodingError> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I dont know if it suits this usecase exactly, but there is an unimplemented! macro.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I'm just keeping it as an error so it is recoverable and doesn't crash everything when something is attempted to be encoded/decoded

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, encoding/decoding shouldn't cause a panic, parent function should deal with it

@austinabell
Copy link
Contributor Author

@ec2 @dutterbutter moved the CodeID type to the actor crate

@austinabell
Copy link
Contributor Author

@ansermino @GregTheGreek pls review if you can, holding up Dustin and a few of my PRs

@austinabell austinabell merged commit de80b34 into master Dec 11, 2019
@austinabell austinabell deleted the austin/messagerework branch December 11, 2019 17:55
vmx pushed a commit to dignifiedquire/forest that referenced this pull request Mar 19, 2020
…hainSafe#79)

* feat(bellman): use bellman lib which uses env to disable/enable GPU

* feat(addpiece): update add_piece call site to conform to new API

* feat(addpiece): update add_piece call site to conform to new API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants