fix[lang]: fix uses analysis for nonreentrant functions

[charles-cooper]

fixes #3922

What I did

How I did it

How to verify it

Commit message

`uses` analysis ignores nonreentrant functions, even though those that
(implicitly) use state.

this commit adds checks both for internally (called) and external
(exported) modules

misc/refactor:
- factor out `uses_state()` util
- rename `validate_functions` to more accurate `analyze_functions`
- improve locality of exceptions thrown in check_module_uses

Description for the changelog

Cute Animal Picture

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.81%. Comparing base (75fb059) to head (d899831).
Report is 1 commits behind head on master.

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3927      +/-   ##
==========================================
- Coverage   90.84%   90.81%   -0.04%     
==========================================
  Files          95       95              
  Lines       14413    14416       +3     
  Branches     3192     3192              
==========================================
- Hits        13094    13092       -2     
- Misses        913      917       +4     
- Partials      406      407       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pcaversaccio]

Right now the error message is very generic:

vyper.exceptions.ImmutableViolation: Cannot access `lib` state!

In most cases, this will work but it might be confusing if a nonreentrant decorator is used because it might not be obvious. I would recommend either issuing a separate error message if a nonreentrant decorator is used or amend the currently existing generic error message.

Furthermore, the compiler currently doesn't raise if block and transaction properties (e.g. block.number or msg.gas) or self / self.balance is used:

# lib.vy
@internal
def _foo() -> address:
    return self

# main.vy
import lib

@external
def foo() -> address:
    return lib._foo()

Q: Should we treat these properties as well as state accesses in an extended sense, since e.g. self is an immutable variable in the strict sense as it resolves at construction time and not at compile-time.

[cyberthirst]

lgtm

[cyberthirst]

Right now the error message is very generic:

vyper.exceptions.ImmutableViolation: Cannot access `lib` state!

In most cases, this will work but it might be confusing if a nonreentrant decorator is used because it might not be obvious.

agreed

Furthermore, the compiler currently doesn't raise if block and transaction properties (e.g. block.number or msg.gas) or self / self.balance is used:

what we're interested in is marking the access to modules, not to state per see

the lock is somewhat an exception as it implicitly accessible by all modules

[cyberthirst]

Should we treat these properties as well as state accesses in an extended sense, since e.g. self is an immutable variable in the strict sense as it resolves at construction time and not at compile-time

I'd say we already do that, eg the following contracts won't compile:

# ex1
@external
@pure
def foo() -> address:
    return self
    
 # ex2
 @external
@pure
def foo() -> uint256:
    return block.number

[charles-cooper]

Q: Should we treat these properties as well as state accesses in an extended sense, since e.g. self is an immutable variable in the strict sense as it resolves at construction time and not at compile-time.

no, these have more to do with environment access (you can think of them like "intrinsics" or "syscalls") and are not directly related to the language's module system

[charles-cooper]

Right now the error message is very generic:

vyper.exceptions.ImmutableViolation: Cannot access `lib` state!

In most cases, this will work but it might be confusing if a nonreentrant decorator is used because it might not be obvious. I would recommend either issuing a separate error message if a nonreentrant decorator is used or amend the currently existing generic error message.

opted for the latter: eaa5988