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

Should execute revert if data is not empty while calling a target with empty code? #14

Open
darwintree opened this issue Aug 29, 2023 · 2 comments

Comments

@darwintree
Copy link

Currently, the reference implementation will execute call regardless of target

    function execute(
        address _target,
        uint256 _value,
        bytes calldata _data,
        uint256 _operation
    ) external payable override returns (bytes memory _result) {
        require(_isValidSigner(msg.sender), "Caller is not owner");
        require(_operation == 0, "Only call operations are supported");
        ++state;
        bool success;
        // solhint-disable-next-line avoid-low-level-calls
        (success, _result) = _target.call{value: _value}(_data);
        require(success, string(_result));
        return _result;
    }

However, it is problem-pone if the _target is set to a wrong address: nothing will happen after the unexpected call. Should the interface revert if data is not empty and target is not a contract?

@sullof
Copy link
Contributor

sullof commented Sep 29, 2023

If the target is invalid, the result won't be a success and the function will revert.
BTW, target cannot be limited to contracts, if not you won't be able to transfer any asset to your EOA wallet, etc.
Also, data must contain something, if not there is nothing to execute. The contract may validate the data, but that would be very expensive on gas.

@darwintree
Copy link
Author

If the target is invalid, the result won't be a success and the function will revert. BTW, target cannot be limited to contracts, if not you won't be able to transfer any asset to your EOA wallet, etc. Also, data must contain something, if not there is nothing to execute. The contract may validate the data, but that would be very expensive on gas.

I created this issue for the reason that I mischoose the _target parameter: I wanted to initiate an ERC20 transfer from the TBA to an EOA, but use the EOA address as the _target rather than the ERC20 contract. I constructed and send the transaction, and nothing happens, and I thought it was a success. However, you would know that the situation is quite opposite. And somewhat you yourself even made similar mistake as me: BTW, target cannot be limited to contracts, if not you won't be able to transfer any asset to your EOA wallet, etc. It was true if the asset refers to ETH, but never ERC20/721 or other innative assets

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