Make Context._msgData return "bytes calldata" (#2492)

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+* `Context`: making `_msgData` return `bytes calldata` instead of `bytes memory` ([#2492](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2492))
+
 ## Unreleased
 
  * `BeaconProxy`: added new kind of proxy that allows simultaneous atomic upgrades. ([#2411](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2411))

contracts/GSN/GSNRecipient.sol

@@ -87,11 +87,11 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      *
      * IMPORTANT: Contracts derived from {GSNRecipient} should never use `msg.sender`, and use {_msgSender} instead.
      */
-    function _msgSender() internal view virtual override returns (address) {
-        if (msg.sender != getHubAddr()) {
-            return msg.sender;
+    function _msgSender() internal view virtual override returns (address msgSender) {
+        if (msg.sender == getHubAddr()) {
+            assembly { msgSender := shr(96, calldataload(sub(calldatasize(), 20))) }
         } else {
-            return _getRelayedCallSender();
+            return msg.sender;
         }
     }
 
@@ -101,11 +101,11 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      *
      * IMPORTANT: Contracts derived from {GSNRecipient} should never use `msg.data`, and use {_msgData} instead.
      */
-    function _msgData() internal view virtual override returns (bytes memory) {
-        if (msg.sender != getHubAddr()) {
-            return msg.data;
+    function _msgData() internal view virtual override returns (bytes calldata) {
+        if (msg.sender == getHubAddr()) {
+            return msg.data[:msg.data.length - 20];
         } else {
-            return _getRelayedCallData();
+            return msg.data;
         }
     }
 
@@ -191,40 +191,4 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
         // charged for 1.4 times the spent amount.
         return (gas * gasPrice * (100 + serviceFee)) / 100;
     }
-
-    function _getRelayedCallSender() private pure returns (address result) {
-        // We need to read 20 bytes (an address) located at array index msg.data.length - 20. In memory, the array
-        // is prefixed with a 32-byte length value, so we first add 32 to get the memory read index. However, doing
-        // so would leave the address in the upper 20 bytes of the 32-byte word, which is inconvenient and would
-        // require bit shifting. We therefore subtract 12 from the read index so the address lands on the lower 20
-        // bytes. This can always be done due to the 32-byte prefix.
-
-        // The final memory read index is msg.data.length - 20 + 32 - 12 = msg.data.length. Using inline assembly is the
-        // easiest/most-efficient way to perform this operation.
-
-        // These fields are not accessible from assembly
-        bytes memory array = msg.data;
-        uint256 index = msg.data.length;
-
-        // solhint-disable-next-line no-inline-assembly
-        assembly {
-            // Load the 32 bytes word from memory with the address on the lower 20 bytes, and mask those.
-            result := and(mload(add(array, index)), 0xffffffffffffffffffffffffffffffffffffffff)
-        }
-        return result;
-    }
-
-    function _getRelayedCallData() private pure returns (bytes memory) {
-        // RelayHub appends the sender address at the end of the calldata, so in order to retrieve the actual msg.data,
-        // we must strip the last 20 bytes (length of an address type) from it.
-
-        uint256 actualDataLength = msg.data.length - 20;
-        bytes memory actualData = new bytes(actualDataLength);
-
-        for (uint256 i = 0; i < actualDataLength; ++i) {
-            actualData[i] = msg.data[i];
-        }
-
-        return actualData;
-    }
 }

contracts/mocks/GSNRecipientMock.sol

@@ -32,7 +32,7 @@ contract GSNRecipientMock is ContextMock, GSNRecipient {
         return GSNRecipient._msgSender();
     }
 
-    function _msgData() internal override(Context, GSNRecipient) view virtual returns (bytes memory) {
+    function _msgData() internal override(Context, GSNRecipient) view virtual returns (bytes calldata) {
         return GSNRecipient._msgData();
     }
 }

contracts/utils/Context.sol

@@ -17,7 +17,7 @@ abstract contract Context {
         return msg.sender;
     }
 
-    function _msgData() internal view virtual returns (bytes memory) {
+    function _msgData() internal view virtual returns (bytes calldata) {
         this; // silence state mutability warning without generating bytecode - see https://github.com/ethereum/solidity/issues/2691
         return msg.data;
     }