Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Context._msgData return "bytes calldata" #2492

Merged
merged 4 commits into from
Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
52 changes: 8 additions & 44 deletions contracts/GSN/GSNRecipient.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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;
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/GSNRecipientMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
2 changes: 1 addition & 1 deletion contracts/utils/Context.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down