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

VIP: use reentrancy lock by default #1756

Open
fubuloubu opened this issue Dec 5, 2019 · 2 comments
Open

VIP: use reentrancy lock by default #1756

fubuloubu opened this issue Dec 5, 2019 · 2 comments
Labels
VIP: Deferred VIP is not scheduled to move forward at this time

Comments

@fubuloubu
Copy link
Member

Simple Summary

Change how reentrancy protection is applied by having external calls use the nonreentrant guard decorator (with a default key) instead of the 2300 gas stipend.

Motivation

The 2300 gas stipend as a protection against reentrant behavior by malicious callers is widely regarded as a bad idea leading to misuse by developers that enable difficult bugs. Similarly, the actual 2300 gas value was chosen to align with a particular set of costs that will potentially change during future upgrades. It is a best practice not to assume gas costs with stay the same with a particular set of operations, so it is important that our codebase also does not make assumptions on gas cost.

The nonreentrant decorator guard was added in #1204. It is currently an "opt-in" feature, requiring that users specify guards on their code with a key that groups together different uses of the guard within their code. This VIP proposes that in general, certain types of external calls (such as send, maybe others) be guarded with a default key without configuration, or otherwise force the use of the decorator somehow to ensure that adequate protection is in place. This would allow us to deprecate the 2300 gas guard in lieu of this superior protection mechanism.

Specification

TBD

Backwards Compatibility

This VIP would change current behavior of Vyper contracts, which would need to be well documented and understood by others. It is important that the default key chosen not interfere with other uses of the guard, as well as have a predictable value so that guards can be determined via static analysis without prior knowledge of the version of Vyper used.

Dependencies

Dependent on #1204

Copyright

Copyright and related rights waived via CC0

@fubuloubu fubuloubu added the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Dec 5, 2019
@fubuloubu
Copy link
Member Author

This VIP is deferred because the accepted practice is to assume the 2300 gas stipend on Ether transfers from contracts (to which we add 0 additional gas to the default EVM stipend). If this accepted practice changes, this VIP would be the way to accomplish the goal of protecting against reentrancy attacks when using the send(address) function

@fubuloubu fubuloubu added VIP: Deferred VIP is not scheduled to move forward at this time and removed VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting labels Dec 9, 2019
@charles-cooper
Copy link
Member

cf. #3380

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Deferred VIP is not scheduled to move forward at this time
Projects
None yet
Development

No branches or pull requests

2 participants