Make Context._msgData return "bytes calldata"

CHANGELOG.md

@@ -16,6 +16,7 @@
  * `ERC165Checker`: added batch `getSupportedInterfaces`. ([#2469](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2469))
  * `RefundEscrow`: `beneficiaryWithdraw` will forward all available gas to the beneficiary. ([#2480](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2480))
  * Many view and pure functions have been made virtual to customize them via overrides. In many cases this will not imply that other functions in the contract will automatically adapt to the overridden definitions. People who wish to override should consult the source code to understand the impact and if they need to override any additional functions to achieve the desired behavior.
+ * `Context`: making `_msgData` return `bytes calldata` instead of `bytes memory` ([#2492](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2492))
 
 ### Security Fixes
 

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;
     }