Apply the msgSender trick

[danfinlay]

Hi, I'm a developer interested in enabling new authorization patterns, like metaTransactions and delegation.

Those approaches require authority-checking facets on the same diamond not use msg.sender directly, but instead call an internal method called _msgSender(), which allows internal methods to implement custom authorization code. This is sometimes called the msgSender() trick.

It looks like this:

    /*
     * @notice Overrides the msgSender to enable delegation message signing.
     * @returns address - The account whose authority is being acted on.
     */
    function _msgSender()
        internal
        view
        virtual
        override(DelegatableCore)
        returns (address sender)
    {
        if (msg.sender == address(this)) {
            bytes memory array = msg.data;
            uint256 index = msg.data.length;
            assembly {
                sender := and(
                    mload(add(array, index)),
                    0xffffffffffffffffffffffffffffffffffffffff
                )
            }
        } else {
            sender = msg.sender;
        }
        return sender;
    }

Adding this change should not add any insecurity to any facets, other than any issues brought on by facets that perform some custom authorization logic to append the intended sender to the calldata:

    function _execute(
        address to,
        bytes memory data,
        uint256 gasLimit,
        address sender
    ) internal returns (bool success) {
        bytes memory full = abi.encodePacked(data, sender);
        assembly {
            success := call(gasLimit, to, 0, add(full, 0x20), mload(full), 0, 0)
        }
    }

Anyways, I know this is probably a sensitive change to propose, and I'd be willing to go to some measures to help it get security audited, but I wanted to know if you'd be interested in merging such a change, I think it would make these facets even more versatile.

If not, no hard feelings, I am happy to just start by making a simple fork for my own usage. I will PR my proposed changes either way so you can see what I am talking about.