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

Disallow self.* calls to public functions #1561

Conversation

iamdefinitelyahuman
Copy link
Contributor

@iamdefinitelyahuman iamdefinitelyahuman commented Aug 4, 2019

What I did

Prevent calls to public functions via self - fixes #1199

How I did it

  • In vyper/parser/self_call.py, raise StructureException where previously there was a call to call_self_public
  • Update tests
  • Update examples
  • Update documentation

How to verify it

Description for the changelog

  • Disallow calls to public functions via self

Cute Animal Picture

image

@iamdefinitelyahuman
Copy link
Contributor Author

This PR relies on #1559 and #1560 - opening now to show where I'm at on implementing this, as they're all somewhat related.

@charles-cooper charles-cooper added the work in progress Work on this PR or issue is not yet complete but reviewers are free to add their input for guidance. label Aug 4, 2019
if sig.private:
return call_self_private(stmt_expr, context, sig)
else:
return call_self_public(stmt_expr, context, sig)
Copy link
Member

Choose a reason for hiding this comment

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

I have a maintenance concern here which is that this will leave a bunch of dead code in the codebase. @jacqueswww @davesque thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is significant overlap between the removed function and external_contract_call in vyper/parser/external_call.py

Copy link
Contributor

Choose a reason for hiding this comment

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

Well looking at call_self_public = it doesn't really have that much dead code or functions it calls out to? Most of those functions are used by call_self_private from what I checked.

@charles-cooper
Copy link
Member

#386

@iamdefinitelyahuman
Copy link
Contributor Author

Merged in master, tests are now passing. Things to still resolve:

@fubuloubu fubuloubu removed the work in progress Work on this PR or issue is not yet complete but reviewers are free to add their input for guidance. label Aug 24, 2019
…ncy_check

Check constancy when calling to external function
Copy link
Contributor

@jacqueswww jacqueswww left a comment

Choose a reason for hiding this comment

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

LGTM 👟

@fubuloubu
Copy link
Member

@iamdefinitelyahuman rebase master and this can be merged

@fubuloubu
Copy link
Member

Hate to be this guy, but there's too much in the history to be merged. Can you part compress the history via rebase -i?

@iamdefinitelyahuman iamdefinitelyahuman deleted the disallow-self-public branch August 25, 2019 08:28
@iamdefinitelyahuman iamdefinitelyahuman restored the disallow-self-public branch August 25, 2019 08:29
@iamdefinitelyahuman
Copy link
Contributor Author

had to do some ugly stuff to fix the commits, opening a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VIP: Restrict usage of msg fields in public functions
4 participants