bug: calls to state-modifying functions in range expressions are not caught

[tserg]

Version Information

• vyper Version (output of vyper --version): 9e3b9a2
• OS: linux
• Python Version (output of python --version): 3.10.4

What's your issue about?

The type checker does not catch the use of a state-modifying function call in a range expression, this leads the code generator to fail due to an assertion: assert use_staticcall, "typechecker missed this"

interface A:
    def foo()-> uint256:nonpayable

@external
def bar(x:address):
    a: A = A(x)
    for i in range(a.foo(), a.foo()+1):
        pass

h/t @trocher

How can it be fixed?

Fill this in if you know how to fix it.

[trocher]

Note that with commit d48438e

The codegen no longer prevent the bug in certain case, leading contracts with side effect in range to compiles successfully:

vyper/vyper/codegen/stmt.py

Line 268 in cfda16c

n = Expr.parse_value_expr(arg0, self.context)

interface A:
    def foo()-> uint256: nonpayable

@external
def bar(x:address):
    a: A = A(x)
    for i in range(a.foo(), bound = 12):
        pass

[fubuloubu]

Note that with commit d48438e

The codegen no longer prevent the bug in certain case, leading contracts with side effect in range to compiles successfully:

vyper/vyper/codegen/stmt.py

Line 268 in cfda16c

n = Expr.parse_value_expr(arg0, self.context)

interface A:
    def foo()-> uint256: nonpayable

@external
def bar(x:address):
    a: A = A(x)
    for i in range(a.foo(), bound = 12):
        pass

This may not be as big of a deal, since it is only called once

[charles-cooper]

@trocher thanks for finding, will fix before release

[trocher]

Also this one would currently compile even if it is really an edge case:

@external
def bar(x:address):
    for i in range(1 if raw_call(x, b'', revert_on_failure=False) else 2 , bound = 12):
        pass

Spent some time compiling a list of state modifying expressions:

• state modifying external call
• state modifying internal call
• raw_call
• pop() when used on a Dynamic Array stored in the storage
• create_minimal_proxy_to
• create_copy_of
• create_from_blueprint

For pop we have that #3172 btw.

[charles-cooper]

Note that with commit d48438e

The codegen no longer prevent the bug in certain case, leading contracts with side effect in range to compiles successfully:

vyper/vyper/codegen/stmt.py

Line 268 in cfda16c

n = Expr.parse_value_expr(arg0, self.context)

interface A:
    def foo()-> uint256: nonpayable

@external
def bar(x:address):
    a: A = A(x)
    for i in range(a.foo(), bound = 12):
        pass

@trocher i think this is a feature, not a bug? there isn't really any ambiguity about side effect ordering or anything here.

[trocher]

Yes I agree, in this case there is no ambiguity or anything, my concern was rather about the semantics of range as I understood that it must always be constant (from definition of is_constant)

vyper/vyper/codegen/context.py

Lines 96 to 97 in 728a276

	def is_constant(self):
	return self.constancy is Constancy.Constant or self.in_assertion or self.in_range_expr

[charles-cooper]

Yes I agree, in this case there is no ambiguity or anything, my concern was rather about the semantics of range as I understood that it must always be constant (from definition of is_constant)

vyper/vyper/codegen/context.py

Lines 96 to 97 in 728a276

	def is_constant(self):
	return self.constancy is Constancy.Constant or self.in_assertion or self.in_range_expr

ah interesting -- do you think this can lead to any bugs?

[trocher]

I don't see how it could lead to an issue in this specific case, it is really only about the semantics of range which changes and now becomes slightly more complex. Before we roughly had the following:

range's arguments must be constant

And now something along those lines:

range's arguments must be constant except for x when the range is on the form range(x,bound=N).

Note that I use here the term constant in the sense of Constancy.Constant and not Vyper's constants (a: constant(uint256)=1)

[charles-cooper]

Note that with commit d48438e

The codegen no longer prevent the bug in certain case, leading contracts with side effect in range to compiles successfully:

vyper/vyper/codegen/stmt.py

Line 268 in cfda16c

n = Expr.parse_value_expr(arg0, self.context)

interface A:
    def foo()-> uint256: nonpayable

@external
def bar(x:address):
    a: A = A(x)
    for i in range(a.foo(), bound = 12):
        pass

as of 194d60a, this triggers a compiler panic (slightly modified code for 0.4.x):

interface A:
    def foo()-> uint256: nonpayable

@external
def bar(x:address):
    a: A = A(x)
    for i: uint256 in range(extcall a.foo(), bound = 12):
        pass

the panic looks like

vyper.exceptions.CodegenPanic: unhandled exception typechecker missed this, parse_ExtCall

  contract "repros/3504.vy:7", function "bar", line 7:28 
       6     a: A = A(x)
  ---> 7     for i: uint256 in range(extcall a.foo(), bound = 12):
  -----------------------------------^
       8         pass

[charles-cooper]

(the panic looks like a duplicate of / related to #3867 actually)