vyper icon indicating copy to clipboard operation
vyper copied to clipboard

fix[lang]: disallow `blockhash` in `pure` functions

Open tserg opened this issue 2 years ago • 8 comments

What I did

Fix #3141.

How I did it

Check for Name nodes with blockhash in local validation of pure functions.

How to verify it

See test.

Commit message

fix: disallow blockhash in pure functions

Description for the changelog

Disallow blockhash in pure functions

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

tserg avatar Nov 24 '22 08:11 tserg

An alternative and perhaps more scalable approach is to add a _reads_environment property to BuiltinFunction, which is set to True for BlockHash(). We can then define the set of builtin functions that reads from the chain by checking for this property (e.g. ENVIRONMENT_BUILTIN_FUNCTIONS). However, I encountered issues with circular imports if I define ENVIRONMENT_BUILTIN_FUNCTIONS in vyper/builtin_functions/functions.py and import it in vyper/semantics/validation/local.py.

tserg avatar Nov 24 '22 09:11 tserg

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.38%. Comparing base (4595938) to head (e2e9864).

:exclamation: Current head e2e9864 differs from pull request most recent head b1517b7. Consider uploading reports for the commit b1517b7 to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3157      +/-   ##
==========================================
+ Coverage   86.34%   86.38%   +0.03%     
==========================================
  Files          92       92              
  Lines       14028    14026       -2     
  Branches     3083     3081       -2     
==========================================
+ Hits        12113    12116       +3     
+ Misses       1487     1483       -4     
+ Partials      428      427       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Nov 27 '22 06:11 codecov-commenter

I have changed the approach to one that also fixes #3425.

tserg avatar May 18 '23 07:05 tserg

this is looking better. i'm still hesitant though, because i tried this myself and realized that the approach doesn't work for raw_call -- raw_call requires a call site in order to figure out whether it's a staticcall or not. but maybe we can just fenagle in some extra logic into raw_call (maybe in its fetch_call_return implementation)

charles-cooper avatar Feb 21 '24 12:02 charles-cooper

this is looking better. i'm still hesitant though, because i tried this myself and realized that the approach doesn't work for raw_call -- raw_call requires a call site in order to figure out whether it's a staticcall or not. but maybe we can just fenagle in some extra logic into raw_call (maybe in its fetch_call_return implementation)

do you mean we should move the current check from build_IR() into fetch_call_return() for raw_call?

https://github.com/vyperlang/vyper/blob/015cf81408cbff22f8bc60fdc506f8e53bfdcca8/vyper/builtins/functions.py#L1117-L1121

tserg avatar Feb 21 '24 14:02 tserg

let's revert the changes to raw_call here because they seem a bit out of scope; we can revisit in another PR.

charles-cooper avatar Mar 25 '24 13:03 charles-cooper