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.

Sign up for GitHub

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

Jump to bottom

Fix caller validation on nested sends #660

Merged
merged 3 commits into from
Aug 28, 2020
Merged

Fix caller validation on nested sends #660

merged 3 commits into from
Aug 28, 2020

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Aug 28, 2020 •
edited

Summary of changes
Changes introduced in this pull request:

  • So still can't duplicate the runtime to match the go implementation completely, but I verified that this is the only other variable that could cause an issue (there is also the allow_internal flag that would be reset, but I believe that to be a bug in their code that they would allow internal calls while inside a transaction if vm send is called, but their code could never do this so I suppose it's fine Update allowInternal flag on child runtime for nested calls filecoin-project/lotus#3377)

Reference issue to close (if applicable)

Closes

Other information and links

timvermeulen
timvermeulen reviewed Aug 28, 2020
View reviewed changes
vm/interpreter/src/default_runtime.rs Outdated
@@ -261,12 +261,16 @@ where
gas_premium: Default::default(),
};

// Since it is unsafe to share a mutable reference to the state tree by copying
// the runtime, all variables must be copied/and reset at the end of the transition.
Copy link
Contributor

Choose a reason for hiding this comment

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

copied/and

Typo?

@austinabell austinabell merged commit 7d2bd2f into main Aug 28, 2020
@austinabell austinabell deleted the austin/vm/calval branch August 28, 2020 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timvermeulen timvermeulen timvermeulen left review comments

@ec2 ec2 ec2 approved these changes

@dutterbutter dutterbutter dutterbutter approved these changes

@ansermino ansermino Awaiting requested review from ansermino

Assignees
No one assigned
Labels
Status: Needs Review VM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@austinabell @timvermeulen @ec2 @dutterbutter