VIP: Separating code reuse concerns from Viper core

[dani-corie]

Preamble

VIP: <to be assigned>
Title: Separating code reuse concerns from Viper core
Author: @daniel-jozsef
Type: Meta(?)
Status: Draft
Created: 2017-12-12

Simple Summary

A templating tool separate and distinct from the Viper compiler should be created, that allows assembling Viper contracts from a number of reusable contract aspects, potentially published as packages or libraries. (Eg. "ownable", "StandardERC20", etc.)

This would allow Viper to strictly defy calls for code composition features. We could have our cake and eat it too.

Abstract

A tool for composing fragment files could add code reusability, while at the same time freeing the Viper compiler of all user pressure on the topic.

In essence, this would mean that large-scale Viper projects, if, and only if the developers so decide, could follow a two-level build model.

A "programmer view" of the code could be composed of "fragment files", that define reusable contract aspects (state variables and functions that use them). Fragments could even have hierarchical dependency (eg. ERC20 depends on ERC20Basic, etc.)

The programmer view is converted into an "auditor view", ie. the Viper smart contracts, a single contract per file, no composition at all, by running a composer tool. This would be a simple copy-paste operation with text replace to ensure that name collisions don't happen.

Then, the "auditor view" is compiled into EVM bytecode via the Viper compiler.

Motivation

Viper should stick close to Vitalik's original vision, and strictly disallow all forms of composition, modifiers and similar syntactic sugar focused on code reuse. This is of the utmost importance if we want a secure and safe language for Ethereum smart contracts.

However, the abundance of VIPs (I personally violently disagree with) on adding code composition features is understandable, given the deep-seated desire for code reuse and dislike of duplication in developer circles.

My solution was motivated by observing the way lawyers work. Legal contracts, similar to Viper ones, have no concept of composition. Yet, a lawyer doesn't write contracts from scratch. They have a number of starting templates for various use cases, and also "aspects", ie. conditions that can be copy-pasted into a template at will, if it is required or appropriate.

I have no illusions, and am pretty sure we can't expect programmers to adopt the modus operandi of lawyers, given the deep hate for manual copy-pasting. So why not give them a tool to do the dirty work for them?

Note that an important part of my proposal is that the build process is two-step. The auditor view (the full, actual Viper contract) should be present in the source tree, and be the actual input of the Viper compiler, to ensure that anyone interested in the contract logic can read and audit it right there.

Specification

The syntax below is completely arbitrary, and should be replaced with one that has been agreed upon by the community.

Fragment files define contract fragments. They are not valid Viper files themselves, but contain Viper code. My suggestion for file extension is .vfrag.py. A fragment file can contain an aspect (something to be inserted into a contract) or a root (a partial definition of a contract that will become a Viper contract with aspects inserted).

ownable.vfrag.py:

@aspect(Ownable)

_owner: public(address)

@private
def _setOwner():
  self._owner = msg.sender

@private
def _requireOwner():
  assert self._owner == msg.sender

helloworld.vfrag.py:

@root
@hasAspect('./ownable.vfrag.py')

message: public(bytes)

@public
def __init__(message: bytes):
  self.Ownable_setOwner()
  self.message = message

@public
def changeMessage(message: bytes):
  self.Ownable_requireOwner()
  self.message = message

...all this should, in the end, be composed into:

helloworld.v.py:

Ownable_owner: public(address)

@private
def Ownable_setOwner():
  self.Ownable_owner = msg.sender

@private
def Ownable_requireOwner():
  assert self.Ownable_owner == msg.sender

message: public(bytes)

@public
def __init__(message: bytes):
  self.Ownable_setOwner()
  self.message = message

@public
def changeMessage(message: bytes):
  self.Ownable_requireOwner()
  self.message = message

Backwards Compatibility

Given that this VIP should include no change to the core Viper language itself, there are no backwards compatibility issues to speak of.

Copyright

Copyright and related rights waived via CC0

[fubuloubu]

I don't think this belongs in the core Viper spec by itself, but it could be a useful external tool for managing and creating large projects. It would probably be best to attack this a little later on when the syntax is more locked down.

[fubuloubu]

Also, another idiosyncrasy is that all globals must go at the top of the file.

[fubuloubu]

Interesting thought: what if macros were managed in a macro package-index, people could add their macros to the store (if developers find it useful and want to leverage it). You can download and use whatever macros, it all creates the source code that actually compiles to bytecode, making developer's jobs easier while maintaining auditability.

Similar to your suggestion, but slightly smaller code fragments and you "register" the macros with the compiler so you don't have to have a whole separate import flow

[dani-corie]

Sounds like a watering hole for predators to focus their attention on... I don't think the Ethereum Foundation could possibly guarantee a level of code review that will ensure safety without doubt. Also, there's a whole can of worms around "why this was approved and that not", "why is that name taken" (see the npm kik debacle, lol)... So I don't think it's a good idea.

[fubuloubu]

Sure, all package management needs strong governance to avoid those concerns. I think there's a use case, and it needs to be a well thought out one

[fubuloubu]

But like, how many times do I have to write an access-controlled setter for a specific variable before I make a costly mistake?

[fubuloubu]

Meeting minutes:

Need to talk about this WAY more, but interesting idea.

Would probably implement as a pre-processor, with hooks that allow custom syntax and a project-wide means of adding to "batteries included" standard library of fragments

[fubuloubu]

Interesting suggestion from @vbuterin (summarized):

Snippets add "Internal Contract" interfaces for the specific functions e.g.

from std import Owner
def __init__():
    self.owner = Owner()
    # OR
    self.owner is Owner

Then the transaction would call (using the ABI) owner.changeOwner(newOwner)

[fubuloubu]

I think the above suggestion touches on a key point: the two key uses of these "snippets" is providing certain addresses with standardized abilities e.g. Owner, Maintainer and giving variables certain characteristics or abilities e.g. access-restricted updating, external contract calling. This is actually fairly similar to Python's own "every type is an object" ethos.

Consider the following viper snippets

1. External address variable with ability to call ERC20-compatible methods from address:

   token: ERC20(address)
   # Allows external calls:
   self.token.transfer(addr, amt)

2. Variable with a getter:

   my_var: Public(num) = 0
   # Creates method:
   def get_my_var() -> num: return self.my_var

3. Time-limiting contract methods via a timer:

   
   owner: Public(address)
   starttime: timestamp
   deadline: Public(timedelta) = 2000 # Blocks
   is_passed: Public(bool) = False # Set when deadline is passed and contract is locked

def init(): self.owner = msg.sender self.starttime = blk.timestamp def set_deadline_passed(): assert msg.sender is self.owner assert blk.timestamp >= self.deadline + self.starttime self.is_passed = True def only_when_active(): assert self.is_passed is False ... # Do something

1 and 2 is currently implemented and simplifies the creation of these additional abilities, however 3 is something that I think is fairly standardized conceptually but difficult to write in the current way of doing things. What if instead for number 3 this concept was implemented as variable that had standard methods from a `Timer` class e.g.:
```python
from std import Timer
owner: Public(address)
timer: Timer(controller=owner, interval=2000)
# 'controller=' creates a symbolic link to self.owner for access rights
# Not specifying a controller lets anyone access it
# 'interval=' is the timer interval required before 'set_complete()' could be called

def __init__():
    self.owner = msg.sender
    # implicitly sets 'timer.starttime = blk.timestamp' and 'timer.active = False'
    # Could also set interval or controller here
    # If controller set to 'msg.sender' here, it would be locked to that address not 'self.owner'
    self.timer.start()
# Adds a timer.set_complete() method to ABI
# Checks for access rights, checks that interval + starttime meets or exceeds block timestamp
def only_when_active():
    assert self.timer.active
    ... # Do something

This is different from Solidity (and I think avoids lots of problems) because the contract itself doesn't have any inheritance, only the variables it has which leads to something much simpler conceptually. Additionally, these "classes" can be implemented as a pre-processor function in the compiler, so the class would translate into a more traditional set of variables and methods that Viper would allow. In order to accomplish the pre-processor implementation, some modifications would be required of the underlying syntax to allow '.' chars in names. The Timer "class" would produce as output of this pre-processor stage:

owner: Public(address)
# Might need some syntactic magic for timer e.g. 'timer: struct' and the following as members
timer.interval: timedelta = 2000
timer.starttime = timestamp
timer.active = Public(bool)

def __init__():
    self.owner = msg.sender
    self.timer.starttime = blk.timestamp
    self.timer.active = True
def timer.set_complete(): # Added method
    assert self.owner == msg.sender # self.owner implicitly substituted for timer.controller
    assert self.timer.interval + self.timer.starttime <= blk.timestamp
    self.timer.active = False
def only_when_active():
    assert self.timer.active
    ... # Do something

We could also change over some of the existing usages like Public() and ERC20() to be these types of classes. var: Public() would simply create a var.get() method in the underlying contract, and token: ERC20() would safely delegate calls to the external contract and additionally allow a setter syntax e.g. self.token = token_address and perhaps a method token.updateAddress(token_address) (with access restrictions) that allows someone to change the token address (if access restrictions not provided, this method isn't generated).

[fubuloubu]

Another interesting thing this could enable is other kinds of uses like:

from std import Role # General case of Owner
owner: Role() # Set to an address that has specific rights in the code, can change itself
board: Role()[5] # 5 Board members, who all have Role abilities
candidate: address
votes: Public(num) = 0
voted: map(address => bool)

def change_owner(_candidate: address):
    assert msg.sender in self.board
    if self.candidate == _candidate:
        self.votes += 1
        self.voted[msg.sender] = True
    else:
        self.candidate = _candidate
        self.votes = 1
        for member in self.board: # member is an 'address' local var
            self.voted[member] = False
        self.voted[msg.sender] = True

def accept_ownership():
    assert msg.sender is self.owner_candidate
    assert self.votes == self.board.len() # now we don't need to carry around lengths!
    self.owner = self.candidate # owner is an 'address' global when assigned directly

The owner can change themselves at any time by calling owner.changeAddress(addr), where changeAddress() is a method provided by the Role() built-in class. The method checks that self.owner is msg.sender before allowing the update. Any one of the board members can also change their address at any time by calling board[i].changeAddress(addr) which asserts that address self.board[i] is the caller.

[DavidKnott]

I prefer the self.owner syntax to self.x.owner syntax as I think it's clearer (it also translates easier to the abi).