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

VM and Runtime updates #569

Merged
merged 26 commits into from
Jul 29, 2020
Merged

VM and Runtime updates #569

merged 26 commits into from
Jul 29, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Went very in detail to make sure details are matching 1:1

The scope of this is in between the actors (didn't change anything within actors that wasn't directly connected) and within the apply tipset messages (function was correct before but can be verified outside of these changes

Mainly there was just inconsistencies around how nil values were handled and the actor error handling within actors and the runtime. Huge pain to match the go implementation panicing in certain spots and catching the actor error, but I think I should have all those errors and exit codes matched (#345 but will wait to close after actor updates are done)

Reference issue to close (if applicable)

Closes #554

Other information and links

Copy link
Contributor

@dutterbutter dutterbutter left a comment

Choose a reason for hiding this comment

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

Must have been painful ensuring 1:1 matching, great attention to detail! 🔥🔥🔥

Copy link
Member

@ec2 ec2 left a comment

Choose a reason for hiding this comment

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

LGTM. Just had one question but not a biggie

"actor {} must be an account ({}), was {}",
raw, &*ACCOUNT_ACTOR_CODE_ID, code_cid
),
Err(format!(
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why you moved the error creation out of here. Can a call to this method return a diff error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because in specs-actors there is no actor errors being returned here and no aborts in sub calls. The annoying part is that fatal errors are not handled because it panics internally in the runtime, but that's why I added to TODO to update it to match completely when I make a pass through that actor (wanted to keep scope as contained as possible)

@austinabell austinabell merged commit 2863f64 into main Jul 29, 2020
@austinabell austinabell deleted the austin/vmupdate branch July 29, 2020 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VM/Runtime interop updates
3 participants