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

Implements apply_message and apply_implicit_message #353

Merged
merged 17 commits into from
Apr 16, 2020

Conversation

dutterbutter
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Added missing logic for apply_message
  • Added apply_implicit_message
  • Added/updated transfer gas functions
  • Modified send to return a tuple
  • Added syscalls to VM struct

I noticed in the spec they are applying a network_tx_fee to total_cost which is checked against the from_actors balance to see if there are sufficient funds. I added a TODO for this, however, I am not sure what that network_tx_fee is comprised of.

Additionally, I added apply_implicit_message as it exists in the lotus implementation, but I do not see apply_tipset_messages in the implementation anymore. I did not remove it from our implementation as it seems that it will be required?

Reference issue to close (if applicable)

Closes #340

Other information and links

vm/default_runtime/src/lib.rs Outdated Show resolved Hide resolved
vm/default_runtime/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Show resolved Hide resolved
vm/interpreter/src/lib.rs Show resolved Hide resolved
vm/src/actor_state.rs Outdated Show resolved Hide resolved
vm/default_runtime/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
vm/interpreter/src/lib.rs Outdated Show resolved Hide resolved
) -> (
Serialized,
DefaultRuntime<'_, 'm, '_, ST, DB, SYS>,
Option<ActorError>,
Copy link
Member

Choose a reason for hiding this comment

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

Don't understand why this is an option? If internal_send is successful, we can return ExitCode::Ok, otherwise return actor_err, no?

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 seems weird to construct an error upon a successful call. More preference I suppose.

@dutterbutter dutterbutter merged commit dbee0e6 into master Apr 16, 2020
@dutterbutter dutterbutter deleted the dustin/apply-msgs branch April 16, 2020 19:56
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.

Complete apply messages in VM
3 participants