Skip to content
This repository has been archived by the owner on Jul 11, 2019. It is now read-only.

Consider implementing a mechanism that "locks" implementations #33

Open
eternauta1337 opened this issue Apr 11, 2018 · 5 comments
Open
Labels
kind:research Research on a topic before implementing it status:blocked Blocked issue
Milestone

Comments

@eternauta1337
Copy link
Contributor

When registering an implementation, it is stored in Registry.sol to be used by a Proxy. If someone interacts with the implementation directly, we don't really have a problem because they are dealing with a separate storage from that of the proxy.

However, a user may accidentally send ether or tokens to the implementation, and even though this doesn't represent a problem for the "proxied" version, it may represent a problem to the user. It could also be confusing for developers adopting zos.

We could consider implementing a solution that "locks up" implementations that are not wrapped by a proxy. I.e. some way to make the registered implementations unresponsive, non payable, etc, if they are not being called via a proxy.

@spalladino spalladino added the kind:research Research on a topic before implementing it label Apr 11, 2018
@spalladino
Copy link
Contributor

I had given this some thoughts. The easy way to do this is to store the contract address in construction time (as the constructor is only run for the implementation and not for the proxies) and compare that address in a modifier that must be added in every public method. Something like:

contract OnlyFromProxy {
  address _address private;
  OnlyFromProxy() {
    _address = this;
  }

  modifier onlyFromProxy() {
    require(this != _address);
    _;
  }
}

@spalladino
Copy link
Contributor

A more complex alternative, but that does not require repeating that same modifier in every function, would be to (somehow) manually inject bytecode to perform the equivalent of the onlyFromproxy check before the msg signature is evaluated to determine which function must be called.

@spalladino
Copy link
Contributor

Note that Solidity already uses this approach under the hood for locking calls to libraries. From Call protection for libraries:

The EVM does not provide a direct way for a contract to detect whether it was called using CALL or not, but a contract can use the ADDRESS opcode to find out “where” it is currently running. The generated code compares this address to the address used at construction time to determine the mode of calling.

More specifically, the runtime code of a library always starts with a push instruction, which is a zero of 20 bytes at compilation time. When the deploy code runs, this constant is replaced in memory by the current address and this modified code is stored in the contract. At runtime, this causes the deploy time address to be the first constant to be pushed onto the stack and the dispatcher code compares the current address against this constant for any non-view and non-pure function.

@spalladino spalladino added this to the v0.1.0 (Kernel MVP) milestone Apr 12, 2018
@spalladino
Copy link
Contributor

We've decided to wait until ethereum/solidity#3864 or an equivalent feature is implemented to go ahead with this feature. Even though this is a convenient feature, we prefer not to fork the solidity compiler at this stage just to support it.

@spalladino spalladino modified the milestones: v0.1.0 (Kernel MVP), v0.2.0 Apr 18, 2018
@spalladino spalladino added the status:blocked Blocked issue label Apr 18, 2018
@spalladino
Copy link
Contributor

We should think if there is a way of implementing this without requiring any changes to the contract code, or via changes that can be automated by the CLI, so the end user does not need to implement anything.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind:research Research on a topic before implementing it status:blocked Blocked issue
Projects
None yet
Development

No branches or pull requests

2 participants