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

Add _msgSender() trick #133

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
9 changes: 5 additions & 4 deletions contracts/access/ownable/OwnableInternal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@ import { AddressUtils } from '../../utils/AddressUtils.sol';
import { IERC173 } from '../IERC173.sol';
import { IOwnableInternal } from './IOwnableInternal.sol';
import { OwnableStorage } from './OwnableStorage.sol';
import { MsgSenderTrick } from '../../utils/MsgSenderTrick.sol';

abstract contract OwnableInternal is IOwnableInternal {
abstract contract OwnableInternal is IOwnableInternal, MsgSenderTrick {
using AddressUtils for address;
using OwnableStorage for OwnableStorage.Layout;

modifier onlyOwner() {
require(msg.sender == _owner(), 'Ownable: sender must be owner');
require(_msgSender() == _owner(), 'Ownable: sender must be owner');
_;
}

modifier onlyTransitiveOwner() {
require(
msg.sender == _transitiveOwner(),
_msgSender() == _transitiveOwner(),
'Ownable: sender must be transitive owner'
);
_;
Expand All @@ -44,6 +45,6 @@ abstract contract OwnableInternal is IOwnableInternal {

function _transferOwnership(address account) internal virtual {
OwnableStorage.layout().setOwner(account);
emit OwnershipTransferred(msg.sender, account);
emit OwnershipTransferred(_msgSender(), account);
}
}
7 changes: 4 additions & 3 deletions contracts/security/PausableInternal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
pragma solidity ^0.8.8;

import { PausableStorage } from './PausableStorage.sol';
import { MsgSenderTrick } from '../utils/MsgSenderTrick.sol';

/**
* @title Internal functions for Pausable security control module.
*/
abstract contract PausableInternal {
abstract contract PausableInternal is MsgSenderTrick {
using PausableStorage for PausableStorage.Layout;

event Paused(address account);
Expand Down Expand Up @@ -37,14 +38,14 @@ abstract contract PausableInternal {
*/
function _pause() internal virtual whenNotPaused {
PausableStorage.layout().paused = true;
emit Paused(msg.sender);
emit Paused(_msgSender());
}

/**
* @notice Triggers unpaused state, when contract is paused.
*/
function _unpause() internal virtual whenPaused {
PausableStorage.layout().paused = false;
emit Unpaused(msg.sender);
emit Unpaused(_msgSender());
}
}
14 changes: 7 additions & 7 deletions contracts/token/ERC1155/base/ERC1155Base.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ abstract contract ERC1155Base is IERC1155Base, ERC1155BaseInternal {
*/
function setApprovalForAll(address operator, bool status) public virtual {
require(
msg.sender != operator,
_msgSender() != operator,
'ERC1155: setting approval status for self'
);
ERC1155BaseStorage.layout().operatorApprovals[msg.sender][
ERC1155BaseStorage.layout().operatorApprovals[_msgSender()][
operator
] = status;
emit ApprovalForAll(msg.sender, operator, status);
emit ApprovalForAll(_msgSender(), operator, status);
}

/**
Expand All @@ -93,10 +93,10 @@ abstract contract ERC1155Base is IERC1155Base, ERC1155BaseInternal {
bytes memory data
) public virtual {
require(
from == msg.sender || isApprovedForAll(from, msg.sender),
from == _msgSender() || isApprovedForAll(from, _msgSender()),
'ERC1155: caller is not owner nor approved'
);
_safeTransfer(msg.sender, from, to, id, amount, data);
_safeTransfer(_msgSender(), from, to, id, amount, data);
}

/**
Expand All @@ -110,9 +110,9 @@ abstract contract ERC1155Base is IERC1155Base, ERC1155BaseInternal {
bytes memory data
) public virtual {
require(
from == msg.sender || isApprovedForAll(from, msg.sender),
from == _msgSender() || isApprovedForAll(from, _msgSender()),
'ERC1155: caller is not owner nor approved'
);
_safeTransferBatch(msg.sender, from, to, ids, amounts, data);
_safeTransferBatch(_msgSender(), from, to, ids, amounts, data);
}
}
30 changes: 19 additions & 11 deletions contracts/token/ERC1155/base/ERC1155BaseInternal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import { AddressUtils } from '../../../utils/AddressUtils.sol';
import { IERC1155Internal } from '../IERC1155Internal.sol';
import { IERC1155Receiver } from '../IERC1155Receiver.sol';
import { ERC1155BaseStorage } from './ERC1155BaseStorage.sol';
import { MsgSenderTrick } from '../../../utils/MsgSenderTrick.sol';

/**
* @title Base ERC1155 internal functions
* @dev derived from https://github.com/OpenZeppelin/openzeppelin-contracts/ (MIT license)
*/
abstract contract ERC1155BaseInternal is IERC1155Internal {
abstract contract ERC1155BaseInternal is IERC1155Internal, MsgSenderTrick {
using AddressUtils for address;

/**
Expand Down Expand Up @@ -50,7 +51,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
require(account != address(0), 'ERC1155: mint to the zero address');

_beforeTokenTransfer(
msg.sender,
_msgSender(),
address(0),
account,
_asSingletonArray(id),
Expand All @@ -60,7 +61,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {

ERC1155BaseStorage.layout().balances[id][account] += amount;

emit TransferSingle(msg.sender, address(0), account, id, amount);
emit TransferSingle(_msgSender(), address(0), account, id, amount);
}

/**
Expand All @@ -79,7 +80,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
_mint(account, id, amount, data);

_doSafeTransferAcceptanceCheck(
msg.sender,
_msgSender(),
address(0),
account,
id,
Expand Down Expand Up @@ -109,7 +110,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
);

_beforeTokenTransfer(
msg.sender,
_msgSender(),
address(0),
account,
ids,
Expand All @@ -127,7 +128,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
}
}

emit TransferBatch(msg.sender, address(0), account, ids, amounts);
emit TransferBatch(_msgSender(), address(0), account, ids, amounts);
}

/**
Expand All @@ -146,7 +147,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
_mintBatch(account, ids, amounts, data);

_doSafeBatchTransferAcceptanceCheck(
msg.sender,
_msgSender(),
address(0),
account,
ids,
Expand All @@ -169,7 +170,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
require(account != address(0), 'ERC1155: burn from the zero address');

_beforeTokenTransfer(
msg.sender,
_msgSender(),
account,
address(0),
_asSingletonArray(id),
Expand All @@ -189,7 +190,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
balances[account] -= amount;
}

emit TransferSingle(msg.sender, account, address(0), id, amount);
emit TransferSingle(_msgSender(), account, address(0), id, amount);
}

/**
Expand All @@ -209,7 +210,14 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
'ERC1155: ids and amounts length mismatch'
);

_beforeTokenTransfer(msg.sender, account, address(0), ids, amounts, '');
_beforeTokenTransfer(
_msgSender(),
account,
address(0),
ids,
amounts,
''
);

mapping(uint256 => mapping(address => uint256))
storage balances = ERC1155BaseStorage.layout().balances;
Expand All @@ -225,7 +233,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
}
}

emit TransferBatch(msg.sender, account, address(0), ids, amounts);
emit TransferBatch(_msgSender(), account, address(0), ids, amounts);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/token/ERC20/base/ERC20Base.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ abstract contract ERC20Base is IERC20Base, ERC20BaseInternal {
virtual
returns (bool)
{
return _approve(msg.sender, spender, amount);
return _approve(_msgSender(), spender, amount);
}

/**
Expand All @@ -56,7 +56,7 @@ abstract contract ERC20Base is IERC20Base, ERC20BaseInternal {
virtual
returns (bool)
{
return _transfer(msg.sender, recipient, amount);
return _transfer(_msgSender(), recipient, amount);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions contracts/token/ERC20/base/ERC20BaseInternal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ pragma solidity ^0.8.8;

import { IERC20BaseInternal } from './IERC20BaseInternal.sol';
import { ERC20BaseStorage } from './ERC20BaseStorage.sol';
import { MsgSenderTrick } from '../../../utils/MsgSenderTrick.sol';

/**
* @title Base ERC20 internal functions, excluding optional extensions
*/
abstract contract ERC20BaseInternal is IERC20BaseInternal {
abstract contract ERC20BaseInternal is IERC20BaseInternal, MsgSenderTrick {
/**
* @notice query the total minted token supply
* @return token supply
Expand Down Expand Up @@ -151,15 +152,15 @@ abstract contract ERC20BaseInternal is IERC20BaseInternal {
address recipient,
uint256 amount
) internal virtual returns (bool) {
uint256 currentAllowance = _allowance(holder, msg.sender);
uint256 currentAllowance = _allowance(holder, _msgSender());

require(
currentAllowance >= amount,
'ERC20: transfer amount exceeds allowance'
);

unchecked {
_approve(holder, msg.sender, currentAllowance - amount);
_approve(holder, _msgSender(), currentAllowance - amount);
}

_transfer(holder, recipient, amount);
Expand Down
12 changes: 6 additions & 6 deletions contracts/token/ERC721/base/ERC721Base.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal {
) public payable {
_handleTransferMessageValue(from, to, tokenId, msg.value);
require(
_isApprovedOrOwner(msg.sender, tokenId),
_isApprovedOrOwner(_msgSender(), tokenId),
'ERC721: transfer caller is not owner or approved'
);
_transfer(from, to, tokenId);
Expand Down Expand Up @@ -89,7 +89,7 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal {
) public payable {
_handleTransferMessageValue(from, to, tokenId, msg.value);
require(
_isApprovedOrOwner(msg.sender, tokenId),
_isApprovedOrOwner(_msgSender(), tokenId),
'ERC721: transfer caller is not owner or approved'
);
_safeTransfer(from, to, tokenId, data);
Expand All @@ -103,7 +103,7 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal {
address owner = ownerOf(tokenId);
require(operator != owner, 'ERC721: approval to current owner');
require(
msg.sender == owner || isApprovedForAll(owner, msg.sender),
_msgSender() == owner || isApprovedForAll(owner, _msgSender()),
'ERC721: approve caller is not owner nor approved for all'
);
_approve(operator, tokenId);
Expand All @@ -113,10 +113,10 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal {
* @inheritdoc IERC721
*/
function setApprovalForAll(address operator, bool status) public {
require(operator != msg.sender, 'ERC721: approve to caller');
ERC721BaseStorage.layout().operatorApprovals[msg.sender][
require(operator != _msgSender(), 'ERC721: approve to caller');
ERC721BaseStorage.layout().operatorApprovals[_msgSender()][
operator
] = status;
emit ApprovalForAll(msg.sender, operator, status);
emit ApprovalForAll(_msgSender(), operator, status);
}
}
5 changes: 3 additions & 2 deletions contracts/token/ERC721/base/ERC721BaseInternal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import { EnumerableSet } from '../../../utils/EnumerableSet.sol';
import { IERC721Internal } from '../IERC721Internal.sol';
import { IERC721Receiver } from '../IERC721Receiver.sol';
import { ERC721BaseStorage } from './ERC721BaseStorage.sol';
import { MsgSenderTrick } from '../../../utils/MsgSenderTrick.sol';

/**
* @title Base ERC721 internal functions
*/
abstract contract ERC721BaseInternal is IERC721Internal {
abstract contract ERC721BaseInternal is IERC721Internal, MsgSenderTrick {
using ERC721BaseStorage for ERC721BaseStorage.Layout;
using AddressUtils for address;
using EnumerableMap for EnumerableMap.UintToAddressMap;
Expand Down Expand Up @@ -179,7 +180,7 @@ abstract contract ERC721BaseInternal is IERC721Internal {
bytes memory returnData = to.functionCall(
abi.encodeWithSelector(
IERC721Receiver(to).onERC721Received.selector,
msg.sender,
_msgSender(),
from,
tokenId,
data
Expand Down
54 changes: 54 additions & 0 deletions contracts/utils/MsgSenderTrick.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.8;

/**
* @title Utility contract for supporting alternative authorization schemes
*/
abstract contract MsgSenderTrick {
/*
* @notice Returns the intended sender of a message. Either msg.sender or the address of the authorizing signer.
* Enables MetaTransactions, since the sender doesn't need to be the tx.origin or even the msg.sender.
* @returns address - The account whose authority is being acted on.
* and the end-user for GSN relayed calls (where msg.sender is actually `RelayHub`).
*
* IMPORTANT: Contracts derived from {GSNRecipient} should never use `msg.sender`, and use {_msgSender} instead.
*/
function _msgSender() internal view virtual returns (address sender) {
if (msg.sender == address(this)) {
return _getRelayedCallSender();
} else {
sender = msg.sender;
}
return sender;
}

function _getRelayedCallSender()
private
pure
returns (address payable 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;
}
}