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.

Sign up for GitHub

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

Add _msgSender() trick #133

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccffe67
Add _msgSender() trick
danfinlay Aug 22, 2022
a0b566e
Move msgSender trick to utils file
danfinlay Aug 26, 2022
3e876b3
Bring in the better documented version of _msgSender
danfinlay Aug 26, 2022
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
26 changes: 23 additions & 3 deletions contracts/access/ownable/OwnableInternal.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ abstract contract OwnableInternal is IOwnableInternal {
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 +44,26 @@ abstract contract OwnableInternal is IOwnableInternal {

function _transferOwnership(address account) internal virtual {
OwnableStorage.layout().setOwner(account);
emit OwnershipTransferred(msg.sender, account);
emit OwnershipTransferred(_msgSender(), account);
}

/*
* @notice Overrides the msgSender to enable delegation message signing.
* @returns address - The account whose authority is being acted on.
*/
function _msgSender() internal view virtual returns (address sender) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to define this in a library in utils/, to improve re-usability and avoid code duplication.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I was thinking similarly.

if (msg.sender == address(this)) {
bytes memory array = msg.data;
uint256 index = msg.data.length;
assembly {
sender := and(
mload(add(array, index)),
0xffffffffffffffffffffffffffffffffffffffff
)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe these steps in more detail?

} else {
sender = msg.sender;
}
return sender;
}
}
24 changes: 22 additions & 2 deletions contracts/security/PausableInternal.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,34 @@ 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());
}

/*
* @notice Overrides the msgSender to enable delegation message signing.
* @returns address - The account whose authority is being acted on.
*/
function _msgSender() internal view virtual returns (address sender) {
if (msg.sender == address(this)) {
bytes memory array = msg.data;
uint256 index = msg.data.length;
assembly {
sender := and(
mload(add(array, index)),
0xffffffffffffffffffffffffffffffffffffffff
)
}
} else {
sender = msg.sender;
}
return sender;
}
}
14 changes: 7 additions & 7 deletions contracts/token/ERC1155/base/ERC1155Base.sol
View file
Open in desktop
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);
}
}
47 changes: 37 additions & 10 deletions contracts/token/ERC1155/base/ERC1155BaseInternal.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,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 +60,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 +79,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 +109,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
);

_beforeTokenTransfer(
msg.sender,
_msgSender(),
address(0),
account,
ids,
Expand All @@ -127,7 +127,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 +146,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
_mintBatch(account, ids, amounts, data);

_doSafeBatchTransferAcceptanceCheck(
msg.sender,
_msgSender(),
address(0),
account,
ids,
Expand All @@ -169,7 +169,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 +189,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 +209,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 +232,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 Expand Up @@ -504,4 +511,24 @@ abstract contract ERC1155BaseInternal is IERC1155Internal {
uint256[] memory amounts,
bytes memory data
) internal virtual {}

/*
* @notice Overrides the msgSender to enable delegation message signing.
* @returns address - The account whose authority is being acted on.
*/
function _msgSender() internal view virtual returns (address sender) {
if (msg.sender == address(this)) {
bytes memory array = msg.data;
uint256 index = msg.data.length;
assembly {
sender := and(
mload(add(array, index)),
0xffffffffffffffffffffffffffffffffffffffff
)
}
} else {
sender = msg.sender;
}
return sender;
}
}
4 changes: 2 additions & 2 deletions contracts/token/ERC20/base/ERC20Base.sol
View file
Open in desktop
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
24 changes: 22 additions & 2 deletions contracts/token/ERC20/base/ERC20BaseInternal.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,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 All @@ -179,4 +179,24 @@ abstract contract ERC20BaseInternal is IERC20BaseInternal {
address to,
uint256 amount
) internal virtual {}

/*
* @notice Overrides the msgSender to enable delegation message signing.
* @returns address - The account whose authority is being acted on.
*/
function _msgSender() internal view virtual returns (address sender) {
if (msg.sender == address(this)) {
bytes memory array = msg.data;
uint256 index = msg.data.length;
assembly {
sender := and(
mload(add(array, index)),
0xffffffffffffffffffffffffffffffffffffffff
)
}
} else {
sender = msg.sender;
}
return sender;
}
}
12 changes: 6 additions & 6 deletions contracts/token/ERC721/base/ERC721Base.sol
View file
Open in desktop
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);
}
}
22 changes: 21 additions & 1 deletion contracts/token/ERC721/base/ERC721BaseInternal.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,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 Expand Up @@ -229,4 +229,24 @@ abstract contract ERC721BaseInternal is IERC721Internal {
address to,
uint256 tokenId
) internal virtual {}

/*
* @notice Overrides the msgSender to enable delegation message signing.
* @returns address - The account whose authority is being acted on.
*/
function _msgSender() internal view virtual returns (address sender) {
if (msg.sender == address(this)) {
bytes memory array = msg.data;
uint256 index = msg.data.length;
assembly {
sender := and(
mload(add(array, index)),
0xffffffffffffffffffffffffffffffffffffffff
)
}
} else {
sender = msg.sender;
}
return sender;
}
}