Fake return values from stack.

[Dexaran]

In Solidity, if you call a function that does not return a value, assuming that it returns something, you will get fake return values from the stack. Not unassigned value or error.

I think that it is a serious issue because we can't rely on return values at all. It would be better to throw an error or return unassigned values.

I have described this in solidity issue #2630.

pragma solidity ^0.4.11;

contract test_receive
{ 
// this contract is needed only for the other contract to compile - it's never actually called
    function receive(address, uint256, bytes) returns (bool, uint256)
   { 
        return (false, 222);
    }
} 

contract test_cast
{
    uint public test_uint;
    bool public test_bool;

    function test(address addr, uint256 n) returns (uint256)
    {
        bytes memory _data;
        var (_success, _back) = test_receive(addr).receive(addr, n, _data);
        require(_success);
        test_uint = _back;
        test_bool = _success;
        return _back;
    }
}

contract dummy
  {
    function() payable {  }
}

Is here any possibility to avoid the same problem in Viper?

[DavidKnott]

@Dexaran You're on it! This is a huge problem, I think this problem will be easier to deal with once we have the functionality to make calls to other contracts in place. Then I'll start by trying to come up with a failing test case similar to the one you made in Solidity.

[fubuloubu]

As a fancier solution, implementing a basic verification stage type checking all return values in the function's control flow path would be useful to provide programmer feedback to a bad contract.

[jacqueswww]

Adding this to Fix up the beta project, so some time can be taken to investigate and see if we can avoid this.

[haydenadams]

@jacqueswww @DavidKnott You should be careful here: argotorg/solidity#4116

Many ERC20 contracts including OMG do not return booleans as specified in the ERC20 standard. If you have a contract with a common interface for all ERC20s that expects boolean return values for transfer(), approve(), and transferFrom() and enforce that any call using that interface has to return either True or False, all calls to the OMG token contract (and several other tokens) will revert. This is problematic for many DEX designs.

EDIT: did some digging. Apparently its at least 130 tokens affected.

[fubuloubu]

Interfaces should be implemented exactly as designed, 💯 agree on that. I am curious if the random stack value return influences this at all because sometimes the value will be non-zero, and that would be difficult to debug. Your suggestion to enforce call return values as the interface specifies should absolutely be enforced.

I don't know if this solves the problem here though, as the OP notes it will return without throwing random values from the stack.

[haydenadams]

@fubuloubu I wasn't necessarily suggesting it. I was saying if it is enforced over 130 tokens currently on EtherDelta will no longer be able to be transferred using the standard ERC20 interface.

[jacqueswww]

I investigated this some more, and because we use CALL and don't have a in memory stack like solidity. In it's current iteration vyper can not have this issue.
However this is something to consider when we make a decision with regards to #901 .