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: Reentrant Lock Decorator #1204

Closed
jacqueswww opened this issue Jan 18, 2019 · 5 comments

Comments

@jacqueswww
Copy link
Collaborator

@jacqueswww jacqueswww commented Jan 18, 2019

Simple Summary

Add a decorator called @nonreentrant to be used on all functions that handle funds and at the discretion of the developer.

Abstract

Using a lock around a function call one can prevent a function doing send / external call with value to do a re-entrant call.

Motivation

Re-entrancy is super difficult to detect as a programmer, Vyper should make it simple not to introduce or hard to get wrong.

As an extra security measure it is encouraged to force the usage of @nonreentrant on functions that are sending funds, either with the value= parameter or the send() function. This is just an extra layer of protection, however attacks are still possible if a function uses state-setting and fund moving in separate transactions.

Specification

Using a unique identifier per function (derivitive of method_id), we can set a lock per function. See https://github.com/protofire/zeppelin-solidity/blob/master/contracts/ReentrancyGuard.sol.

In pseudo vyper code this would be the equivelant of something like:

locks: map(bytes32, bool)

def withdraw():
    assert self.locks[method_id('withdraw()', bytes32)] == False
    self.locks[method_id('withdraw()', bytes32)] = True
     # Body of code.
    self.locks[method_id('withdraw()', bytes32)] = False

Backwards Compatibility

This will effect backwards compatibility if this becomes a forced decorator, if only an optional decorator it will not effect backwards compatibility.

Dependencies

Optional Implementation of @nonreentrant: None (5200 gas per application)
Forced implementation of @nonreentrant: Net-Gas Metering (400 gas per application, well worth it!)

Copyright

Copyright and related rights waived via CC0

@jacqueswww jacqueswww changed the title No-reentrant Decorator No-reentrant Lock Decorator Jan 18, 2019
@fubuloubu fubuloubu changed the title No-reentrant Lock Decorator VIP: No-reentrant Lock Decorator Jan 18, 2019
@jacqueswww jacqueswww changed the title VIP: No-reentrant Lock Decorator VIP: Reentrant Lock Decorator Jan 18, 2019
@charles-cooper

This comment has been minimized.

Copy link
Collaborator

@charles-cooper charles-cooper commented Jan 18, 2019

Should the first line of the withdraw example read like follows?

    assert self.locks[method_id('withdraw()', bytes32)] == False
@fubuloubu

This comment has been minimized.

Copy link
Member

@fubuloubu fubuloubu commented Jan 18, 2019

Added the edit!

@fubuloubu

This comment has been minimized.

Copy link
Member

@fubuloubu fubuloubu commented Jan 18, 2019

I don't think we should "enforce" this as a default, rather this is a good tool for developers to use in situtations where it is called for

We should instead raise warnings of when a function should have this decorator, to reinforce the pattern that not every function will need it

@jacqueswww

This comment has been minimized.

Copy link
Collaborator Author

@jacqueswww jacqueswww commented Jan 28, 2019

Marked as accepted, @nonreentrant(key)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.