Vyper: Potentially insufficient gas stipend for precompiled contract calls

Created on 10 Dec 2019  路  8Comments  路  Source: vyperlang/vyper

Version Information

  • vyper Version (output of vyper --version): 0.1.0b13+commit.375fb27

(confirmed the same behavior at 0.1.0b14+commit.49b201c as well.)

What's your issue about?

Vyper utilizes the ID precompiled contract call for doing memcpy-like operations. That is, the compiled bytecode contains the opcode pattern CALL GASCAP 4 0 SRC SIZE DEST SIZE to copy the local memory region from SRC to DEST.

The problem is that the allocated gas stipend GASCAP for the call in the bytecode is constant-bounded, (e.g., 21 when SIZE == 32), which may not be sufficient in a future hardfork that modifies the gas schedule. Thus this may result in that any such future hardfork may be significantly delayed or long-lasting contracts (such as the deposit contract) may need to be redeployed.

Is there a specific reason to have this tight gas cap bound?

BTW, this behavior doesn't seem to be consistent. For example, when it calls to the SHA256 precompiled contract, it simply allocates all available gas for the call stipend.

How can it be fixed?

Allocate all available gas (i.e., using the GAS opcode) when calling to precompiled contracts.

Easy Pickings bug

All 8 comments

Question: In case that this is indeed a bug, is it possible to release a hotfix version that fixes this issue on top of the beta 13 (not the beta 14)? That will significantly help re-run the deposit bytecode verification against the fixed bytecode, without going through all the changes made since the beta 13 release.

This should just be changed to use gas opcode and not a pre-calculated value (like sha256 does it).
https://github.com/vyperlang/vyper/blob/master/vyper/parser/parser_utils.py#L120

Yes indeed; thanks.

I want second the request to hotfix this as a release on b13 in addition to incorporating it for further releases. Our formal verification efforts on the eth2 deposit contract are very near complete and bumping to b14+ to fix this bug would majorly set us back.

Thank you!

It should be possible to have a fix for this be easily based on b13. We'd like to avoid maintaining a fork of b13, but obviously want to help.

@jacqueswww I made a PR that fixes this. Could you please take a look?
@fubuloubu I don't think the custom hotfix release needs to be further maintained in the future.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

haydenadams picture haydenadams  路  3Comments

nrryuya picture nrryuya  路  3Comments

fubuloubu picture fubuloubu  路  3Comments

fubuloubu picture fubuloubu  路  3Comments

lsaether picture lsaether  路  4Comments