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

ERC: Proxy standard #121

Closed
SilentCicero opened this issue Jun 23, 2016 · 30 comments
Closed

ERC: Proxy standard #121

SilentCicero opened this issue Jun 23, 2016 · 30 comments
Labels

Comments

@SilentCicero
Copy link

SilentCicero commented Jun 23, 2016

ERC
Title: Proxy standard
Status: Draft
Type: Informational
Created: 22.06.2016
Resolution: https://github.com/ethereum/wiki/wiki/Standardized_Contract_APIs

Abstract

The following describes standard functions a proxy contract can implement.

Motivation

Many Ethereum users and smart-contracts hold and manage digital assets. A unified way to hold, acquire and transfer digital assets could greatly simplify and unify the designs of many contractual services making them more inter-operable with each other, while also gaining from the design benefits of standardization. Many contractual services also require a single persistent account at which to transact within the ecosystem. These services could also greatly benefit from a single unified contractual design enabling this practice to happen in a predictable and consistent way.

Rational

This allows Ethereum users/services to store, transfer and manage multiple kinds of digital assets in a single simple vetted standardized smart-contract. By doing so, we simplify digital asset acquisition, storage and management. A proxy standard also simplifies and/or unified contractual design of governance, identity or financial services on Ethereum. While there is always the potential that the implementer contract of a Proxy could be compromised or poorly design and all assets exposed to attack or freezing, we can expect at the very least the proxy contract implementer to be able to transfer its proxy's ownership (and the digital assets contained therein).

Specification

  1. A single contract with two methods (i.e. transfer_implementer, and forward_transaction) and one variable (i.e. implementer).
  2. The transfer_implementer method is solely responsible for changing ownership from the stored implementer to a new implementer and is access restricted to the implementer and the contract itself.
  3. The forwarding method forward_transaction has three inputs: address _destination, uint _value, bytes _bytecode.
  4. The forward_transaction method is access restricted to the implementer only.
  5. The forward_transaction forwards transactions to the destination address as the ProxyContract address.
  6. The StandardProxy contract implements the Proxy interface contract containing the forward_transaction and transfer_ownership methods.

Code for Proxy contract interface

contract Proxy {
    function forward_transaction (address _destination, uint _value, bytes _bytecode) {}
    function transfer_implementer (address _implementer) {}
}

Code for the StandardProxy contract

import "Proxy.sol";

contract StandardProxy is Proxy {

    function forward_transaction (address _destination, uint _value, bytes _bytecode) {
        if(msg.sender == implementer) {
            if(!_destination.call.value(_value)(_bytecode)) {
                throw;
            }
        }
    }

    function transfer_implementer (address _implementer) {
        if(msg.sender == implementer || msg.sender == address(this)) {
            implementer = _implementer;
        }
    }

    address implementer;
}

StandardProxy contract in use with a factory contract

import "StandardProxy.sol";

contract ImplementedStandardProxy is StandardProxy {
    function ImplementedStandardProxy(address _implementer) {
        implementer = _implementer;
    }

    function getImplementer() constant returns (address) {
       return implementer;
    }
}

contract ImplementedStandardProxyFactory {
    function createStandardProxy(address _implementer) returns (address standardProxyAddress) {
        standardProxyAddress = new ImplementedStandardProxy(_implementer);
    }
}

Potential Attack Vectors/Pitfalls

  • rare use of self access restriction: this is a very uncommon use of contract access restriction and is potentially vulnerable because of this fact (just an odd use of access restriction).
  • re-entry: outward call contract re-entry is a worry in the forward_transaction method.
  • compromised implementer contract/user account: if the implementer contract is compromised by an attacker all digital assets held by the proxy are potentially exposed to attack and theft. The proxy is not suppose to solve this. It is merely suppose to provide a contractual base on which to bond digital assets too.
  • poorly designed implementer contracts: an implementer contract may end up being poorly design or broken, potentially locking the digital assets in the proxy forever. If a contract supports the use of proxy contracts we can then as contract architects expect them to, at the very least, be able to transfer ownership of the proxy. Thus simplifying our base expectations and concerns for contracts that manage digital assets.
  • transaction forwarding/routing costs: if implementer systems use a proxy to manage digital assets and to represent an identity of that system, there is the additional cost incurred by forwarding transactions through the abstracted implementer, into the proxy and out into the ecosystem. This adds additional gas costs that need to be considered.
  • empty/invalid bytecode inputs: there may be potential issues with forwarding transactions that have no bytecode (I cant think of any, but it does seem like it could present issues in the future -- this is merely my intuition here, nothing else).

Examples of proxy contract designs

Notes

  • re-entry must be addressed in the forward_transaction method.
  • proxies should be kept, simple, robust, reusable and stupid (they are meant to be used by other contract interfaces)
  • use the "owner" contract instead of "implementer" term/concept (in defense of the implementer term/concept: "ownership" is certainly a valid term/concept here, however, implementer does seem more metaphysically accurate, as the term "owner" only implies ownership, where as implementer implies that which can implement state change within and as the contract -- this is to be debated however and perhaps it just has too much overlap with the "owner" contract infrastructure).
  • perhaps "proxy" is not the right term here, the dappsys ecosystem uses the term "BaseActor", which also seems appropriate and accurate. However, in conversation and in deep mediation, I believe "proxy" to be a better descriptive term here.
  • proxy contracts (1) simplify, unify and standardize digital asset acquisition and management while also providing (2) a persistent account identity at which services and users can transact with the ecosystem, although I do believe there to be more benefits
  • proxy contracts also help us manage expectations of contracts that hold digital assets or need a persistent account within the ecosystem

A special thanks and credit goes to Christian Lundkvist, Nikolai Mushegian, Simon de la Rouviere, Niran Babalola, and Peter Borah for furthering the design of proxy and proxy like smart-contracts on Ethereum.

@danfinlay
Copy link
Contributor

I would consider the wording of owner and transfer_owner or transfer_ownership over implementor. Implementor describes who created it, owner describes who owns it, which seems to be the intended meaning here.

@SilentCicero
Copy link
Author

SilentCicero commented Jun 23, 2016

I would tend to agree, I'm just into the implementer concept.

@PeterBorah
Copy link

PeterBorah commented Jun 23, 2016

While it is much more complex than the simple proxy contract here, it may be worth trying to align with Mist's wallet contract, which has the following signature for the forwarding function:

execute(address, uint, bytes)

The version I've written for ownage uses the above signature, as well as this one for transferring ownership:

function setAdmin(address)

(It also has a few more things specific to its context.)

I think reaching alignment on the forwarding function is the biggest priority. As the Mist example shows, concepts like ownership may be different for different contracts.

@danfinlay
Copy link
Contributor

danfinlay commented Jun 23, 2016

I think reaching alignment on the forwarding function is the biggest priority. As the Mist example shows, concepts like ownership may be different for different contracts.

One of the reasons I think a standard proxy contract makes a lot of sense is that it isolates concerns very neatly.

The Mist Wallet conflates the concerns of a proxy contract with the responsibilities of a multi-sig authorizer.

With the proposed architecture, you could always assign the "implementor" to be a multi-sig contract, which itself could be any sort of advanced authorization logic, and replicate the behavior of the Mist wallet.

I think it's very important that these types of standards be developed, and be atomic, so that larger Dapps tomorrow could be composed entirely out of tiny, well audited, highly reusable contracts. Perhaps I'd like a Mist-like wallet, but with different authorization logic. This type of proposed Proxy standard allows that upgrade to be easily deployed.

@PeterBorah
Copy link

@FlySwatter It's not completely clear to me that a single configurable owner address is the best design for this. There are other plausible ways to secure and upgrade a proxy contract, such as having an attached validator contract, or using delegatecall for configurable behavior.

Another consideration should be that EIP 101 will make this pattern substantially less necessary.

@danfinlay
Copy link
Contributor

I don't think there needs to be a single final proxy contract architecture. I like the idea of a validator-based proxy, too. I more think that these tiny patterns should become well known to the point of naming and standardizing, so they can be easily audited and reused.

Maybe a more diplomatic name would be OwnedProxy, to represent this type of proxy has an owner, while the other could be a ValidatedProxy.

@nmushegian
Copy link

nmushegian commented Jun 23, 2016

  • Why not derive from the already well-established owned interface?
  • I think an "action" is more than just target/value/calldata or even a whole message:
struct Action {
    address target;
    uint gas; // magic value 0 means "no restriction"
    uint value;
    bytes calldata;
    bool must_succeed; // Is the proxy contract supposed to re-throw on exception, or report it as "executed"?
    bytes ret; // once we can read unknown-length return values. For now, could be bytes32
}

@nmushegian
Copy link

I agree with peter that it's not obvious that this is the most natural way to structure delegations/proxy/configurable behavior (are those even the same problem?) but having some kind of common "target" for governance-like contracts would be immediately useful, and this seems to work fine

@PeterBorah
Copy link

bytes ret; // once we can read unknown-length return values. For now, could be bytes32

This is doable with inline assembly.

https://github.com/ownage-ltd/ether-router/blob/master/contracts/EtherRouter.sol#L24-L32

(There's other stuff going on in that file, but the key is that the outsize variable is doing what your ret does.)

@coder5876
Copy link

Thanks for the writeup @SilentCicero! Not surprisingly I think this is a very important question since my view is that this could be the standard way to do identity on Ethereum.

@nmushegian:

Why not derive from the already well-established owned interface?
In uPort we are using the following simple contract deriving from Owned:

import "Owned.sol";

contract Proxy is Owned {
    function forward(address destination, uint value, bytes data) onlyOwner {
        // If a contract tries to CALL or CREATE a contract with either
        // (i) insufficient balance, or (ii) stack depth already at maximum (1024),
        // the sub-execution and transfer do not occur at all, no gas gets consumed, and 0 is added to the stack.
        // see: https://github.com/ethereum/wiki/wiki/Subtleties#exceptional-conditions
        if (!destination.call.value(value)(data)) {
            throw;
        }
    }
}

Interesting using a more general Action (you've accidentally written Auction btw). I don't quite see how to capture the return value. Would you have a storage field or Event in the Proxy that gets set to that return value somehow?

In general I'm interested in making the Proxy contracts as simple as possible.

@PeterBorah How would you design these things differently once BIP 101 is in place? To me it seems that there would still be the need for proxy contracts defining a persistent identifier for identities.

@PeterBorah
Copy link

PeterBorah commented Jun 23, 2016

With EIP 101, your "account" and "proxy contract" can be the same thing, since you can send messages directly to contracts. So we probably won't want to assume that our proxy contract is receiving messages from an account, but rather have the "account"'s verification logic be configurable.

@coder5876
Copy link

@PeterBorah I don't see how this is different from the current approach of having a proxy contract relay messages from an implementer/owner/controller contract which contains the verification logic. There is not really any assumption of an external account in the structure.

As I understand it EIP 101 is basically that all messages/transactions now originate from 0x0 and the authentication/verification logic (like digital signatures) are now abstracted away into the data field. Plz correct if I'm wrong!

@aakilfernandes
Copy link

I would like to see a way to forward an arbitrary number of transactions in order. This is important from a UX perspective, so dapp developers can make "all or nothing" transactions which are dependent on each other.

function forward_transactions (address[] _destinations, uint[] _values, bytes[] _bytecodes) {}

For a larger discussion of why this is necessary, refer to this http://ethereum.stackexchange.com/questions/1561/is-it-possible-to-chain-mulitiple-individual-transactions-into-a-single-all-or/

@nmushegian
Copy link

@aakilfernandes That abstraction can be built out of the simpler standard proposed. If you want to include sequences, I'd counter that we should include an entire interpreted message language.

@aakilfernandes
Copy link

@nmushegian Using another abstraction would make things difficult, since the user would then have to authorize the abstraction to call the proxy. We would need to include authorization functionality in the proxy which would complicate things even further.

@nmushegian
Copy link

^ That's the idea behind delegation via proxy, isn't it? I'm imagining a "keyring manager" proxied to a "execute sequence of statements" proxied to a "rate-limiting withdrawal actor" a proxy contract as an example of this pattern.

What is the purpose of the proxy contract at all, if it's not intended to be extended/chained? The base implementation doesn't add any extra behavior except the ability to transfer..

@coder5876
Copy link

@aakilfernandes Yeah like @nmushegian said the point of the Proxy is to be very simple so that it can be stable and it's Owner can be a complex contract or constellation of contracts.

since the user would then have to authorize the abstraction to call the proxy

That's how it would be set up for the user to begin with. The complexity would be hidden from the user ideally.

@aakilfernandes
Copy link

aakilfernandes commented Jun 29, 2016

@nmushegian @christianlundkvist if we create a different contract capable of forwarding an arbitrary number of transactions, wouldn't that make this contract obsolete? Why would a contract capable of forwarding an arbitrary number of transactions bother calling this contract?

@nmushegian
Copy link

yes it would make it obsolete but your version would quickly become obsolete as well. We should go with the simplest future-proof version possible, or we might as well do a whole independent effort to figure out the complete feature set for interpreter contracts

@SilentCicero
Copy link
Author

SilentCicero commented Jul 1, 2016

import "owned.sol";

contract Proxy {
  function forward_transaction(address _destination, uint _value, bytes _calldata) {}
}

contract OwnedProxy is owned, Proxy {
  modifier onlyowner {
    if(msg.sender == address(this) || msg.sender == owner) _
  }

  function forward_transaction(address _destination, uint _value, bytes _calldata) onlyowner {
    if(!_destination.call.value(_value)(_calldata)) {
      throw;
    }
  }

  function transfer_ownership(address _owner) onlyowner {
    owner = _owner;
  }
}

An OwnedProxy example @nmushegian @FlySwatter @aakilfernandes @christianlundkvist thoughts?

@ethers
Copy link
Member

ethers commented Jul 7, 2016

Good change to name it _calldata. Other suggestions:

import "owned.sol";

contract Proxy {
  function proxy(address _to, uint _value, bytes _calldata);
}

contract OwnedProxy is owned, Proxy {
  function proxy(address _to, uint _value, bytes _calldata) onlyowner {
    if(!_to.call.value(_value)(_calldata)) {
      throw;
    }
  }

  function setOwner(address _owner) onlyowner {
    owner = _owner;
  }
}
  • No need to tweak the onlyowner modifier.
  • Simplified the function names.
  • Using _to to mimic current terms of from and to. to is also more suggestive of potential proxy chaining, instead of a "destination" which suggests an end.

@nmushegian
Copy link

Any reason we're not taking gas as an argument too? The proxy implementation is dynamic, so burdening the caller with figuring out the correct limit doesn't seem right. A related discussion is whether we want the proxy to be smart about exceptions or not (see must_succeed above).

Simplicity is a good argument to avoid it, but we should then explicitly recognize what usage constraints it adds.

@PeterBorah
Copy link

@nmushegian I don't think I fully understand your gas suggestion. Is there a substantial difference between:

proxy.execute(to, value, calldata, gas)

and

proxy.execute.gas(gas)(to, value, calldata)

?

An advantage of the latter is that you don't have to do any work to express "pass all gas", which is arguably the default.

@nmushegian
Copy link

The use case is if I want to execute the action with exactly N gas, but I don't know if the proxy will add 100 or 1m gas.

@PeterBorah
Copy link

I see! I don't think I've ever needed to do that, but I can invent situations where I might. (Though usually I would handle that at some other layer of the stack.)

Solidity's method overloading means that we could easily have both in a proxy contract.

@ethers
Copy link
Member

ethers commented Jul 8, 2016

@nmushegian I did like your Action struct, with more data like gas, etc. #121 (comment) Not sure where that got hung up. The return value? Or it seems people prefer the simplest. (A hybrid could have both proxy and proxy_action(Action action), but I'm guessing hybrid is even less popular for a "standard".)

@danfinlay
Copy link
Contributor

I now think the transfer_implementer method doesn't need to be part of this standard.

Someone could inherit from the class, and with only forward_transaction, they could implement that method, and mix in their own onlyowner or auth logic on that method to restrict its access.

This allows a smaller standard surface area (a simple contract could proxy without bothering with upgradeability), while being easily compatible with established auth patterns.

@nmushegian
Copy link

nmushegian commented Feb 11, 2017

@FlySwatter has it right!

After ownership logic I think the only thing left to discuss is whether "interpreter"/"atomic sequences"/"return threading" and other proxy power-ups are in scope for this EIP, or if we want to revisit them after some EVM opcode EIPs are addressed to make it cleaner. Thoughts from uport/boardroom teams?

@github-actions
Copy link

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jan 16, 2022
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

drortirosh added a commit to drortirosh/EIPs that referenced this issue Dec 29, 2022
Update the EIP to the working version from
https://github.com/eth-infinitism/account-abstraction/blob/develop/eip/EIPS/eip-4337.md

Changes:

    AA-94 update keccak rules.
    AA-93 Adding debug RPC APIs for the Bundler to use (ethereum#153)
    AA 92 simulate execution (ethereum#152)
    AA 73 unify reputation (ethereum#144)
    AA-68 rpc calls (ethereum#132)
    AA-61 rename wallet to account (ethereum#134)
    AA-69 wallet support for simulation without signing (ethereum#133)
    AA-70 rename requestId to userOpHash (ethereum#138)
    AA-67 relax storage rules in opcode banning (ethereum#121)
    AA-63 remove paymaster stake value from EntryPoint (ethereum#119)
    AA-51 simpler simulation api, including aggregation
    AA-60 validate timestamp (ethereum#117)
Clarify wallet factory behavior when the wallet already exists
(ethereum#118)
drortirosh added a commit to drortirosh/EIPs that referenced this issue Dec 29, 2022
Update the EIP to the working version from
https://github.com/eth-infinitism/account-abstraction/blob/develop/eip/EIPS/eip-4337.md

Changes:

    AA-94 update keccak rules.
    AA-93 Adding debug RPC APIs for the Bundler to use (ethereum#153)
    AA 92 simulate execution (ethereum#152)
    AA 73 unify reputation (ethereum#144)
    AA-68 rpc calls (ethereum#132)
    AA-61 rename wallet to account (ethereum#134)
    AA-69 wallet support for simulation without signing (ethereum#133)
    AA-70 rename requestId to userOpHash (ethereum#138)
    AA-67 relax storage rules in opcode banning (ethereum#121)
    AA-63 remove paymaster stake value from EntryPoint (ethereum#119)
    AA-51 simpler simulation api, including aggregation
    AA-60 validate timestamp (ethereum#117)
Clarify wallet factory behavior when the wallet already exists
(ethereum#118)
drortirosh added a commit to drortirosh/EIPs that referenced this issue Dec 29, 2022
Update the EIP to the working version from
https://github.com/eth-infinitism/account-abstraction/blob/develop/eip/EIPS/eip-4337.md

Changes:

    AA-94 update keccak rules.
    AA-93 Adding debug RPC APIs for the Bundler to use (ethereum#153)
    AA 92 simulate execution (ethereum#152)
    AA 73 unify reputation (ethereum#144)
    AA-68 rpc calls (ethereum#132)
    AA-61 rename wallet to account (ethereum#134)
    AA-69 wallet support for simulation without signing (ethereum#133)
    AA-70 rename requestId to userOpHash (ethereum#138)
    AA-67 relax storage rules in opcode banning (ethereum#121)
    AA-63 remove paymaster stake value from EntryPoint (ethereum#119)
    AA-51 simpler simulation api, including aggregation
    AA-60 validate timestamp (ethereum#117)
Clarify wallet factory behavior when the wallet already exists
(ethereum#118)
eth-bot pushed a commit that referenced this issue Dec 29, 2022
* Update to latest working version

Update the EIP to the working version from
https://github.com/eth-infinitism/account-abstraction/blob/develop/eip/EIPS/eip-4337.md

Changes:

    AA-94 update keccak rules.
    AA-93 Adding debug RPC APIs for the Bundler to use (#153)
    AA 92 simulate execution (#152)
    AA 73 unify reputation (#144)
    AA-68 rpc calls (#132)
    AA-61 rename wallet to account (#134)
    AA-69 wallet support for simulation without signing (#133)
    AA-70 rename requestId to userOpHash (#138)
    AA-67 relax storage rules in opcode banning (#121)
    AA-63 remove paymaster stake value from EntryPoint (#119)
    AA-51 simpler simulation api, including aggregation
    AA-60 validate timestamp (#117)
Clarify wallet factory behavior when the wallet already exists
(#118)

* lint fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants