Overload resolution for .value()-modified functions

[axic]

I have a method, which gets called with value transfer using the <method>.value(<value>)(<args>) semantic. This fails when overloading.

Works:

contract ShapeshiftBot {
  function transfer(string coin, string recipient, bool acceptReturn, address returnAddress) returns (bytes32 myid);
  function transfer(string coin, string recipient, bool acceptReturn) returns (bytes32 myid);
  function transfer(string coin, string recipient) returns (bytes32 myid);
}

contract ShapeshiftBotLookup {
  function getAddress() returns (address _addr);
}

contract usingShapeshift {
  function shapeshiftInstance() returns (ShapeshiftBot) {
    ShapeshiftBotLookup lookup = ShapeshiftBotLookup(0x0);
    return ShapeshiftBot(lookup.getAddress());
  }

  function shapeshiftTransfer(uint value, string coin, string recipient, bool acceptReturn, address returnAddress) internal returns (bytes32 myid) {
    return shapeshiftInstance().transfer(coin, recipient, acceptReturn, returnAddress);
  }

  function shapeshiftTransfer(uint value, string coin, string recipient, bool acceptReturn) internal returns (bytes32 myid) {
    return shapeshiftInstance().transfer(coin, recipient, acceptReturn);
  }

  function shapeshiftTransfer(uint value, string coin, string recipient) internal returns (bytes32 myid) {
    return shapeshiftInstance().transfer(coin, recipient);
  }
}

Fails when adding .value() in usingShapeshift:

Error: Member "transfer" not unique after argument-dependent lookup in contract ShapeshiftBot
    return shapeshiftInstance().transfer.value(value)(coin, recipient, acceptReturn, returnAddress);
           ^---------------------------^

[axic]

A reduced example for clarity.

Working:

contract ShapeshiftBot {
  function transfer(string a, string b);
  function transfer(string a);
}

contract usingShapeshift {
  ShapeshiftBot bot = ShapeshiftBot(0x0);

  function transfer(string a, string b) {
    bot.transfer(a, b);
  }

  function transfer(string a) {
    bot.transfer(a);
  }
}

Failing:

contract ShapeshiftBot {
  function transfer(string a, string b);
  function transfer(string a);
}

contract usingShapeshift {
  ShapeshiftBot bot = ShapeshiftBot(0x0);

  function transfer(string a, string b) {
    bot.transfer.value(1)(a, b);
  }

  function transfer(string a) {
    bot.transfer.value(1)(a);
  }
}

Working with .value(), but no overloading:

contract ShapeshiftBot {
  function transfer(string a);
}

contract usingShapeshift {
  ShapeshiftBot bot = ShapeshiftBot(0x0);

  function transfer(string a) {
    bot.transfer.value(1)(a);
  }
}

The problem appears with .gas() too.

[chriseth]

Yes, we have a pending story for that. Sorry for the inconvenience. It is a bit hard to fix because you can only do overload resolution when you know the arguments but you need to know the type of the function so that you can tell whether it has a .vaule function or not.

[axic]

Not fully aware of the internals of the compiler, but would an explicit flag (i.e. #500 and its appropriate keywords on functions) help?

Which cases value is not accepted today? constant, private, internal ?

[axic]

@chriseth with payable, is this now easier to fix?

[chriseth]

Not really, the thing is that the argumentTypes AST annotation has to be pulled through several AST nodes, and this will not even solve cases like var x = f.value; x(2 ether)(1,2);

[3esmit]

This affects me in 0.5.1.

browser/PaymentChannel.sol:35:9: TypeError: Member "newChannel" not unique after argument-dependent lookup in contract ChannelFactory.
        _factory.newChannel.value(_amount)(_recipient, _duration);   
        ^-----------------^

contract ChannelFactory {
    function newChannel(address payable _recipient, uint256 _duration) external payable returns(PaymentChannel _channel){
        _channel = (new PaymentChannelETH).value(msg.value)(_recipient, _duration);
    }
    function newChannel(Token _token, address _recipient, uint256 _duration) external returns(PaymentChannel _channel) {
        _channel = new PaymentChannelERC20(_token, _recipient, _duration);
        _token.transferFrom(msg.sender, address(_channel), _token.allowance(msg.sender, address(this)));
    }
}

contract Account {
    function openChannel(ChannelFactory _factory, Token _token, address payable _recipient, uint256 _duration, uint256 _amount) external {
        _token.approve(address(_factory), _amount);
        _factory.newChannel(_token, _recipient, _duration);   
    }
    function openChannel(ChannelFactory _factory,  address payable _recipient, uint256 _duration, uint256 _amount) external {
        _factory.newChannel.value(_amount)(_recipient, _duration);   
    }
}

See example failing to compile built in remix.ethereum.org: https://gist.github.com/3esmit/fce237c187d1f54eacb2c434553b2dd2

While this is not fixed, compilation should fail when one function being overloaded have payable modifier, otherwise would not be possible to use the payable modifier.

[3esmit]

As this also affects .gas, overloading of functions should not even be supported.

I see that is a hard problem to solve.
I see some possible solutions that don't look nice, but might do it:

• For payable with gas setting: _factory.newChannel(_recipient, _duration)(_amount, _gas);
• For payable with only value: _factory.newChannel(_recipient, _duration)(_amount);
• For non payable: _factory.newChannel(_token, _recipient, _duration)(_gas);

I think the best way would be rework how one contract call/send to other, and using the interface call to generate the abi that would be used in the call/send.

I imagine:
Non payable auto-gas: this.call(_factory.newChannel(_token, _recipient, _duration))
Non payable manual-gas: this.call(_factory.newChannel(_token, _recipient, _duration), _gas)
Payable with auto-gas: this.send(_factory.newChannel(_recipient, _duration), _value)
Payable with manual-gas: this.send(_factory.newChannel(_recipient, _duration), _value, _gas)

[derekchiang]

Is there currently no way around this issue? So basically there's no way to call an overloaded payable function with some value?

[3esmit]

You can use address(otherContract).call.value(amount)(abi.encodeWithSignature("method(uint256,bytes)", param1, param2));

[chriseth]

Or you perform a type conversion to a contract that only has one of the two overloads.

[JonahGroendal]

Could the syntax be changed? IMO .value()() seems out of place in Solidity anyway since functions aren't first-class objects.

[chriseth]

If you have a good idea about how to change the syntax, we are all ears! Note that functions actually are first-class objects in Solidity: https://solidity.readthedocs.io/en/v0.5.8/types.html#function-types

[Marenz]

This issue can be re-visited now that #8177 is almost merged

[chriseth]

It will be almost the same problem. But we should add a test, actually!