feat: proxies

[pegahcarter]

I've added OP's Proxy.sol with a slight modification so that the proxy is Ownable. We will be using this Proxy for Fraxtal's Vested FXS, L1VeFXS, etc.

@tom2o17 @denett

[tom2o17]

Did we gap the implementation up to 200 ?

Otherwise were going to get storage collisions

[tom2o17]

This is an example of a proxies storage layout I have used in the past.

[pegahcarter]

Did we gap the implementation up to 200 ?

Otherwise were going to get storage collisions

Are you looking at storage for OZ 5.x? I believe 4.9 would have the storage slot issues that you're mentioning.

[pegahcarter]

@tom2o17 ready for review.

Notable change that the admin is no longer two-step. This should be fine for the time being unless you have a strong opinion in favor of 2-step.

[denett]

What was wrong with the original proxy from OP?

[pegahcarter]

Nothing was necessarily wrong: I simply removed the constructor arguments so now we can use pre-deterministic addresses.

[pegahcarter]

@tom2o17 @denett any issues in merging here?

[tom2o17]

LGTM: Do we want to enforce strict package imports ?

[pegahcarter]

Probably a good idea, although there are currently no strict package versions. I can make a separate PR specifically for version control.

[tom2o17]

SG, do we want to enforce a proxyAdmin contract you can run into issues if you have selector collision between the proxy / impl given the owner will not call to the impl if it is the admin.

But this is solvable, we for example would not be able to have changeAdmin(address) on the implementation and expect the owner of the proxy to interact w/ this function.

pretty NIT, but also not an issue if we enforce that that the proxyAdmin will not interact with the proxy outsidie of managing the versioning

[pegahcarter]

Are you talking specifically about this modifier?

frax-standard-solidity/src/proxies/Proxy.sol

Line 88 in c58e76e

function changeAdmin(address _admin) public virtual proxyCallIfNotAdmin {

Are you saying there could be a conflict if the owner is, say, a msig who can call changeAdmin() on both directly the implementation and proxy? Would changing state of the implementation after it's connected to the proxy change the state of the proxy?

[tom2o17]

@pegahcarter

if the implementation were to implement a function called changeAdmin(address) which changed the owner variable of the implementation logic. This function would not be accessible by the admin of the proxy.

This is because of:

    /// @notice A modifier that reverts if not called by the owner or by address(0) to allow
    ///         eth_call to interact with this proxy without needing to use low-level storage
    ///         inspection. We assume that nobody is able to trigger calls from address(0) during
    ///         normal EVM execution.
    modifier proxyCallIfNotAdmin() {
        if (msg.sender == _getAdmin() || msg.sender == address(0)) {
            _;
        } else {
            // This WILL halt the call frame on completion.
            _doProxyCall();
        }
    }

There for, if the admin were to call changeAdmin(address) to the proxy it would only change the admin of the proxy contract. [No delegate call] It will retain ownership of the implementation logic.

Are you saying there could be a conflict if the owner is, say, a msig who can call changeAdmin() on both directly the implementation and proxy?

It cannot call changeAdmin(address) on the implementation there will be no delegate call.

Would changing state of the implementation after it's connected to the proxy change the state of the proxy?

No (might be a little confused as to the question), changing the state of the implementation does not impact the state of the proxy. Changing the implementation contract changes the state of the proxy, but this is fine because it is changing the state of the eip1967 assigned slot.