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

msgSender() in ContextMixin #48

Open
mudgen opened this issue Sep 7, 2020 · 4 comments
Open

msgSender() in ContextMixin #48

mudgen opened this issue Sep 7, 2020 · 4 comments

Comments

@mudgen
Copy link

mudgen commented Sep 7, 2020

@arthcp can you explain this code: https://github.com/maticnetwork/pos-portal/blob/master/contracts/common/ContextMixin.sol

Specifically why do you expect there to be an address at add(array, index) in memory?

Also, why are you using a mask there? I don't see a need to use a mask there to clean bits because the value is going into a 20 byte address variable.

@arthcp
Copy link
Contributor

arthcp commented Sep 7, 2020

@mudgen Meta transactions are executed by calling the executeMetaTransaction function of NativeMetaTransaction contract.

This function calls self contract with calldata and userAddress as can be seen here: https://github.com/maticnetwork/pos-portal/blob/master/contracts/common/NativeMetaTransaction.sol#L59-L61

Due to how it is being called, we expect the user address to be at add(array, index).

Thanks for pointing out that masking is not needed here. I think you are correct about that. I will run some tests to verify it and raise a PR.

@mudgen
Copy link
Author

mudgen commented Sep 7, 2020

@arthcp I see, cool.

Instead of copying calldata to memory and dropping to assembly you can use calldata slices. For example:

sender = abi.decode(msg.data[msg.data.length-32:], (address));

Array slices info here: https://solidity.readthedocs.io/en/v0.7.1/types.html#array-slices

@mudgen
Copy link
Author

mudgen commented Sep 7, 2020

Nevermind, you won't be able to decode it that way because it is encoded with abi.encodePacked. But if you encoded it with abi.encode instead then you could.

@mudgen
Copy link
Author

mudgen commented Sep 7, 2020

Since the address is encoded with abi.encodePacked, you could do it like this:

pragma solidity 0.6.6;

abstract contract ContextMixin {
    function msgSender()
        internal
        view
        returns (address payable sender)
    {
        if (msg.sender == address(this)) {
            uint256 index = msg.data.length - 20;
            assembly {
                // Load the 32 bytes word from calldata with the address on the lower 20 bytes.
                sender := calldataload(index)
            }
        } else {
            sender = msg.sender;
        }
        return sender;
    }
}

Saves some gas from having to copy calldata to memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants