Validate contract method return type against ABI

[ixje]

Summary or problem description Take this contract

def test_func(data: bytes) -> bool:
   if data == b'\x01\x02':
      return True
   return False

which has this manifest

{
    "name": "example",
    "groups": [],
    "abi": {
        "methods": [
            {
                "name": "test_func",
                "offset": 0,
                "parameters": [
                    {
                        "name": "data",
                        "type": "ByteArray"
                    }
                ],
                "returntype": "Boolean",
                "safe": false
            }
        ],
        "events": []
    },
    "permissions": [
        {
            "contract": "*",
            "methods": "*"
        }
    ],
    "trusts": [],
    "supportedstandards": [],
    "extra": null

As a contract consumer I expect that I can rely on the interface described by the ABI. Therefore, calling test_func is expected to return a boolean, but in this particular case it returns an integer. (The root cause can be attributed to an incorrect NEF pushing an Integer stackitem instead of a boolean stackitem, but that's not the discussion here.)

{
  script: 'DAIBAhHAHwwJdGVzdF9mdW5jDBTMtlvb3USvKenjdrXR8KI2DTJwLkFifVtS',
  state: 'HALT',
  gasconsumed: '1002450',
  exception: null,
  stack: [ { type: 'Integer', value: '1' } ]
}

In this case as a contract consumer I would thus expect a FAULT engine state instead of a HALT with an incorrect result.

Historically these kind of issues have occurred and caused trackers and wallets to stop functioning until updated. iirc one such case was an updated Switcheo nep5 token (NEO2) that started returning Integer types instead of the expected bytearray. The ABI is there to help prevent this, but currently it doesn't do so.

Do you have any solution you want to propose? Validate the return type against the returnType described in the ABI for the method and raise an exception if it deviates.

Strongly related to https://github.com/neo-project/neo/issues/1891 Neo Version

• Neo 3

Where in the software does this update applies to?

• VM (maybe?)

[ixje]

Note that for #1891 (which I still think is an important issue) incorrect input types can arguably be blamed on the contract consumer as he has control over it. The return type however is controlled by the contract itself, thus I'd argue more important to fix on the chain side.

[erikzhang]

In fact, ContractParameterType and StackItemType are not one-to-one correspondence. For example, for ContractParameterType.String you would expect StackItemType.ByteString or StackItemType.Buffer. For ContractParameterType.Array, you will expect StackItemType.Array or StackItemType.Struct. Similarly, for ContractParameterType.Boolean, you should expect to get any supported types, such as StackItemType.Boolean or StackItemType.Integer or StackItemType.ByteString and so on.

[ixje]

Is this described somewhere? Because if not, we better do so as the far majority of ABI consumers will not know this mapping.

[erikzhang]

This has not been described. But yes, it should have a document.

[chenzhitong]

This has not been described. But yes, it should have a document.

Then the developer must know which NeoVM type corresponds to the C# type, it's so unfriendly. For example, the UInt160 type in c# is a byte array type in NeoVM. Even if the parameter received in the smart contract is UInt160, you can actually pass in a non-20-byte bytearray as a parameter. However, a "normal" developer would not try to determine if a UInt160 parameter is in UInt160 format or not

[melanke]

These types doesn't match 1x1, correct? I mean, when I got a response from the RPC server with the "ByteString" type it could be many different C# types such as a String, UInt160 and others.

Isn't this a problem? For instance, to parse an Array that contains multiple "ByteString" values I would not be able to know which of the values are UInt160 and which are Strings.