modify msg.sender to _msgSender() to support metaTxs (#436)

* modify msg.sender to _msgSender() to use forwarder

* gas optimization for _msgSender() and add sender parameter to pause/unpause functions

* add comment for pause and unpause functions

* add tests using forwarder

contracts/Bridge.sol

@@ -38,6 +38,8 @@ contract Bridge is Pausable, AccessControl, SafeMath {
     mapping(uint8 => uint64) public _depositCounts;
     // resourceID => handler address
     mapping(bytes32 => address) public _resourceIDToHandlerAddress;
+    // forwarder address => is Valid
+    mapping(address => bool) public isValidForwarder;
     // destinationDomainID + depositNonce => dataHash => Proposal
     mapping(uint72 => mapping(bytes32 => Proposal)) private _proposals;
 
@@ -86,16 +88,17 @@ contract Bridge is Pausable, AccessControl, SafeMath {
     }
 
     function _onlyAdminOrRelayer() private view {
-        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender) || hasRole(RELAYER_ROLE, msg.sender),
+        address sender = _msgSender();
+        require(hasRole(DEFAULT_ADMIN_ROLE, sender) || hasRole(RELAYER_ROLE, sender),
             "sender is not relayer or admin");
     }
 
     function _onlyAdmin() private view {
-        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
+        require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "sender doesn't have admin role");
     }
 
     function _onlyRelayers() private view {
-        require(hasRole(RELAYER_ROLE, msg.sender), "sender doesn't have relayer role");
+        require(hasRole(RELAYER_ROLE, _msgSender()), "sender doesn't have relayer role");
     }
 
     function _relayerBit(address relayer) private view returns(uint) {
@@ -106,8 +109,18 @@ contract Bridge is Pausable, AccessControl, SafeMath {
         return (_relayerBit(relayer) & uint(proposal._yesVotes)) > 0;
     }
 
+    function _msgSender() internal override view returns (address payable) {
+        address payable signer = msg.sender;
+        if (msg.data.length >= 20 && isValidForwarder[signer]) {
+            assembly {
+                signer := shr(96, calldataload(sub(calldatasize(), 20)))
+            }
+        }
+        return signer;
+    }
+
     /**
-        @notice Initializes Bridge, creates and grants {msg.sender} the admin role,
+        @notice Initializes Bridge, creates and grants {_msgSender()} the admin role,
         creates and grants {initialRelayers} the relayer role.
         @param domainID ID of chain the Bridge contract exists on.
         @param initialRelayers Addresses that should be initially granted the relayer role.
@@ -119,7 +132,7 @@ contract Bridge is Pausable, AccessControl, SafeMath {
         _fee = fee.toUint128();
         _expiry = expiry.toUint40();
 
-        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
+        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
 
         for (uint256 i; i < initialRelayers.length; i++) {
             grantRole(RELAYER_ROLE, initialRelayers[i]);
@@ -146,30 +159,31 @@ contract Bridge is Pausable, AccessControl, SafeMath {
     }
 
     /**
-        @notice Removes admin role from {msg.sender} and grants it to {newAdmin}.
+        @notice Removes admin role from {_msgSender()} and grants it to {newAdmin}.
         @notice Only callable by an address that currently has the admin role.
         @param newAdmin Address that admin role will be granted to.
      */
     function renounceAdmin(address newAdmin) external onlyAdmin {
-        require(msg.sender != newAdmin, 'Cannot renounce oneself');
+        address sender = _msgSender();
+        require(sender != newAdmin, 'Cannot renounce oneself');
         grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
-        renounceRole(DEFAULT_ADMIN_ROLE, msg.sender);
+        renounceRole(DEFAULT_ADMIN_ROLE, sender);
     }
 
     /**
         @notice Pauses deposits, proposal creation and voting, and deposit executions.
         @notice Only callable by an address that currently has the admin role.
      */
     function adminPauseTransfers() external onlyAdmin {
-        _pause();
+        _pause(_msgSender());
     }
 
     /**
         @notice Unpauses deposits, proposal creation and voting, and deposit executions.
         @notice Only callable by an address that currently has the admin role.
      */
     function adminUnpauseTransfers() external onlyAdmin {
-        _unpause();
+        _unpause(_msgSender());
     }
 
     /**
@@ -267,6 +281,16 @@ contract Bridge is Pausable, AccessControl, SafeMath {
         _depositCounts[domainID] = nonce;
     }
 
+    /**
+        @notice Set a forwarder to be used.
+        @notice Only callable by an address that currently has the admin role.
+        @param forwarder Forwarder address to be added.
+        @param valid Decision for the specific forwarder.
+     */
+    function adminSetForwarder(address forwarder, bool valid) external onlyAdmin {
+        isValidForwarder[forwarder] = valid;
+    }
+
     /**
         @notice Returns a proposal.
         @param originDomainID Chain ID deposit originated from.
@@ -332,21 +356,22 @@ contract Bridge is Pausable, AccessControl, SafeMath {
         require(handler != address(0), "resourceID not mapped to handler");
 
         uint64 depositNonce = ++_depositCounts[destinationDomainID];
+        address sender = _msgSender();
 
         IDepositExecute depositHandler = IDepositExecute(handler);
-        bytes memory handlerResponse = depositHandler.deposit(resourceID, msg.sender, data);
+        bytes memory handlerResponse = depositHandler.deposit(resourceID, sender, data);
 
-        emit Deposit(destinationDomainID, resourceID, depositNonce, msg.sender, data, handlerResponse);
+        emit Deposit(destinationDomainID, resourceID, depositNonce, sender, data, handlerResponse);
     }
 
     /**
-        @notice When called, {msg.sender} will be marked as voting in favor of proposal.
+        @notice When called, {_msgSender()} will be marked as voting in favor of proposal.
         @notice Only callable by relayers when Bridge is not paused.
         @param domainID ID of chain deposit originated from.
         @param depositNonce ID of deposited generated by origin Bridge contract.
         @param data Data originally provided when deposit was made.
         @notice Proposal must not have already been passed or executed.
-        @notice {msg.sender} must not have already voted on proposal.
+        @notice {_msgSender()} must not have already voted on proposal.
         @notice Emits {ProposalEvent} event with status indicating the proposal status.
         @notice Emits {ProposalVote} event.
      */
@@ -362,9 +387,11 @@ contract Bridge is Pausable, AccessControl, SafeMath {
             executeProposal(domainID, depositNonce, data, resourceID, true);
             return;
         }
+
+        address sender = _msgSender();
 
         require(uint(proposal._status) <= 1, "proposal already executed/cancelled");
-        require(!_hasVoted(proposal, msg.sender), "relayer already voted");
+        require(!_hasVoted(proposal, sender), "relayer already voted");
 
         if (proposal._status == ProposalStatus.Inactive) {
             proposal = Proposal({
@@ -384,7 +411,7 @@ contract Bridge is Pausable, AccessControl, SafeMath {
         }
 
         if (proposal._status != ProposalStatus.Cancelled) {
-            proposal._yesVotes = (proposal._yesVotes | _relayerBit(msg.sender)).toUint200();
+            proposal._yesVotes = (proposal._yesVotes | _relayerBit(sender)).toUint200();
             proposal._yesVotesTotal++; // TODO: check if bit counting is cheaper.
 
             emit ProposalVote(domainID, depositNonce, proposal._status, dataHash);

contracts/TestContracts.sol

@@ -79,3 +79,11 @@ contract HandlerRevert is HandlerHelpers {
         _totalAmount = amount;
     }
 }
+
+contract Forwarder {
+    function execute(bytes memory data, address to, address sender) external {
+        bytes memory callData = abi.encodePacked(data, sender);
+        (bool success, ) = to.call(callData);
+        require(success, "Relay call failed");
+    }
+}

contracts/utils/Pausable.sol

@@ -72,25 +72,27 @@ contract Pausable {
 
     /**
      * @dev Triggers stopped state.
+     * @param sender Address which executes pause.
      *
      * Requirements:
      *
      * - The contract must not be paused.
      */
-    function _pause() internal virtual whenNotPaused {
+    function _pause(address sender) internal virtual whenNotPaused {
         _paused = true;
-        emit Paused(msg.sender);
+        emit Paused(sender);
     }
 
     /**
      * @dev Returns to normal state.
+     * @param sender Address which executes unpause.
      *
      * Requirements:
      *
      * - The contract must be paused.
      */
-    function _unpause() internal virtual whenPaused {
+    function _unpause(address sender) internal virtual whenPaused {
         _paused = false;
-        emit Unpaused(msg.sender);
+        emit Unpaused(sender);
     }
 }