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 be able to specify safeTxGas on factory #45

Open
rmeissner opened this issue Apr 2, 2020 · 1 comment
Open

Should be able to specify safeTxGas on factory #45

rmeissner opened this issue Apr 2, 2020 · 1 comment
Labels
contracts This issue is related to contract changes

Comments

@rmeissner
Copy link
Member

rmeissner commented Apr 2, 2020

To ensure that enough gas is available for executing the Safe transaction on deployment (which is required so that other wallets can properly estimate the required gas) it should be possible to specify the safeTxGas for the Safe transaction performed on deployment

see https://github.com/gnosis/contract-proxy-kit/blob/master/contracts/CPKFactory.sol#L41

Currently if you try to perform a transaction with a gas costs together with deployment, metamask will fail to properly estimate that.

Work around for now could be that we to binary search on the returned success value and set this as the proposed gas value on the tx send to the connected wallet.

@germartinez germartinez added the contracts This issue is related to contract changes label Apr 2, 2020
@rmeissner
Copy link
Member Author

rmeissner commented Apr 2, 2020

Geth support code injection for addresses. So we can create the runtime code for a simple estimation contract:

contract Snipped {
    enum Operation {
        Call,
        DelegateCall
    }
    
    function execute(address to, uint256 value, bytes memory data, Operation operation, uint256 txGas)
        internal
        returns (bool success)
    {
        if (operation == Operation.Call)
            success = executeCall(to, value, data, txGas);
        else if (operation == Operation.DelegateCall)
            success = executeDelegateCall(to, data, txGas);
        else
            success = false;
    }

    function executeCall(address to, uint256 value, bytes memory data, uint256 txGas)
        internal
        returns (bool success)
    {
        // solium-disable-next-line security/no-inline-assembly
        assembly {
            success := call(txGas, to, value, add(data, 0x20), mload(data), 0, 0)
        }
    }

    function executeDelegateCall(address to, bytes memory data, uint256 txGas)
        internal
        returns (bool success)
    {
        // solium-disable-next-line security/no-inline-assembly
        assembly {
            success := delegatecall(txGas, to, add(data, 0x20), mload(data), 0, 0)
        }
    }
    
     function requiredTxGas(address to, uint256 value, bytes calldata data, Operation operation)
        external
        returns (uint256)
    {
        uint256 startGas = gasleft();
        // We don't provide an error message here, as we use it to return the estimate
        // solium-disable-next-line error-reason
        require(execute(to, value, data, operation, gasleft()));
        uint256 requiredGas = startGas - gasleft();
        // Convert response to string and return via error message
        return requiredGas;
    }
}

and then call it with an rpc call

{
        "jsonrpc": "2.0",
        "method": "eth_call",
        "params": [
            {
                "to": "0x9913B9180C20C6b0F21B6480c84422F6ebc4B808",
                "data": "0xc4ca3a9c00000000000000000000000005c85ab5b09eb8a55020d72daf6091e04e264af9000000000000000000000000000000000000000000000000000009184e72a000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
            },
            "latest",
            {
                "0x9913B9180C20C6b0F21B6480c84422F6ebc4B808": {
                    "code": "0x608060405234801561001057600080fd5b506004361061002b5760003560e01c8063c4ca3a9c14610030575b600080fd5b6100de6004803603608081101561004657600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff169060200190929190803590602001909291908035906020019064010000000081111561008d57600080fd5b82018360208201111561009f57600080fd5b803590602001918460018302840111640100000000831117156100c157600080fd5b9091929391929390803560ff1690602001909291905050506100f4565b6040518082815260200191505060405180910390f35b6000805a905061014b878787878080601f016020809104026020016040519081016040528093929190818152602001838380828437600081840152601f19601f82011690508083019250505050505050865a610169565b61015457600080fd5b60005a82039050809250505095945050505050565b600080600181111561017757fe5b83600181111561018357fe5b141561019c57610195868686856101db565b90506101d2565b6001808111156101a857fe5b8360018111156101b457fe5b14156101cc576101c58685846101f4565b90506101d1565b600090505b5b95945050505050565b6000806000845160208601878987f19050949350505050565b60008060008451602086018786f49050939250505056fea2646970667358221220fddab9f0edfc309a71ee22a54f20dfcd83fbb6dad9c674876f98ecaae5b2bd6564736f6c63430006040033"
                }
            }
        ],
        "id": 1
}

this way we get back the estimated gas

{
        "jsonrpc": "2.0",
        "id": 1,
        "result": "0x0000000000000000000000000000000000000000000000000000000000001ea5"
}

The really nice part about this is, that this will also work even if the Safe is not deployed yet, so we could estimate the gas for the transaction nested into the deployment in the CPKFactory

Edit: contract could be simplified, but I wanted to have the same interface as the Safe contract ;)

@cag cag mentioned this issue May 1, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts This issue is related to contract changes
Projects
None yet
Development

No branches or pull requests

2 participants