diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b6f831f445..d32313a200f 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/contracts/GSN/GSNRecipient.sol b/contracts/GSN/GSNRecipient.sol index cfde4763b2c..8393056f115 100644 --- a/contracts/GSN/GSNRecipient.sol +++ b/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; - } } diff --git a/contracts/mocks/GSNRecipientMock.sol b/contracts/mocks/GSNRecipientMock.sol index 2b9c839640b..f0c70b79363 100644 --- a/contracts/mocks/GSNRecipientMock.sol +++ b/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(); } } diff --git a/contracts/utils/Context.sol b/contracts/utils/Context.sol index b8fca57b048..11e886f1080 100644 --- a/contracts/utils/Context.sol +++ b/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; }