MoralisWeb3 / Moralis-JS-SDK

Moralis Official Javascript SDK
https://docs.moralis.io
Other
366 stars 257 forks source link

Contract multi-value result and struct returns incorrect #154

Closed mloit closed 2 years ago

mloit commented 2 years ago

I've come across a couple of seemingly related cases where the result type goes against what would be expected.

The first case is where a contract is returning multiple anonymous values. One would expect an array of indexed values, but instead an object with index names is returned.

The second case is when a contract is returning a structure with multiple values. One would expect to receive an object with properties having the corresponding names as defined in the ABI, but instead an array is returned. This carries forward to when an array of structs is returned, the SDK is producing an array of arrays, instead of an array of objects.

Contract Returning Multiple Values

When a contract function is called that returns multiple values the SDK converts this into an object with numbered parameters. This should be more correctly returned as an array, as the parameters are anonymous and only defined by their order.

Example constructions

Below are some example constructions in solidity and the resultant ABI along with their returned outputs from within the Moralis SDK.

This is the call that was used to test


  echo: async function(selector) {

    const result = await contract.__call("echo", {_selected: selector});

        console.log("echo() returned: ", typeof result, result);
    },

 __call: async function(name, params = undefined) {

        const ctx = {
            chain: contract.network,
            address: contract.address[contract.network],
            abi: contract.abi,
            function_name: name
        };

        if(typeof params !== 'undefined') {
            ctx.params = params;
        }

        result = await Moralis.Web3API.native.runContractFunction(ctx).then(function(result) {
            return result;
        }).catch(function(error) {
            console.log('contract.' + name + '():' + error.error);
            result = undefined;
            return;
        });
        return result;
    }

Contract function:

This is the contract function that was created for testing purposes while debugging this and other issues

function echo(uint256 _selected) public view returns (address, uint256, bool) {
    return (msg.sender, _selected, active);
}

Resultant ABI:

This is the relevant ABI snippet passed in the abi option

{
    "inputs": [
        {
            "internalType": "uint256",
            "name": "_selected",
            "type": "uint256"
        }
    ],
    "name": "echo",
    "outputs": [
        {
            "internalType": "address",
            "name": "",
            "type": "address"
        },
        {
            "internalType": "uint256",
            "name": "",
            "type": "uint256"
        },
        {
            "internalType": "bool",
            "name": "",
            "type": "bool"
        }
    ],
    "stateMutability": "view",
    "type": "function"
}

Received Result

Result Type: *Object*
{
    0: "0x0000000000000000000000000000000000000000"
    1: "1"
    2: true
}

While this is technically okay, it would be more sensible if returned as an array, as access is via index, and not name. Couple this with the result from a when a Struct is returned, it's as if the relationships are reversed.

Expected/Preferred Result
Result Type: *Array* of Length 3
[
    0: "0x0000000000000000000000000000000000000000"
    1: "1"
    2: true
]

This way we can test for the number of returned elements with result.length, as well as iterate through with foreach

Contract Returning Stuct

When a contract function is called that returns multiple values the SDK converts this into an object with numbered parameters. This should be more correctly returned as an array, as the parameters are anonymous and only defined by their order.

Example constructions

Contract function:

struct MyStruct {
    address add;
    uint256 selected;
    bool active;
}

function echo(uint256 _selected) public view returns (MyStruct memory) {
    MyStruct memory _toSend = MyStruct(msg.sender, _selected, active);
    return _toSend;
}

Resultant ABI:

{
    "inputs": [
        {
            "internalType": "uint256",
            "name": "_selected",
            "type": "uint256"
        }
    ],
    "name": "echo",
    "outputs": [
    {
    "components": [
        {
            "internalType": "address",
            "name": "add",
            "type": "address"
        },
        {
            "internalType": "uint256",
            "name": "selected",
            "type": "uint256"
        },
        {
            "internalType": "bool",
            "name": "active",
            "type": "bool"
        }
    ],
    "internalType": "struct IntroStake.MyStruct",
    "name": "",
    "type": "tuple"
}

Received Result

Result Type: *Array* of Length 3
[
    0: "0x0000000000000000000000000000000000000000"
    1: "1"
    2: true
]

This is a larger problem than the first case, as the names of the elements of the results have effectively been stripped away, making any code using it prone to breaking if the Contract/ABI was updated re-ordering the parameters.

Expected/Preferred Result
Result Type: *Object*
{
    "add": "0x0000000000000000000000000000000000000000"
    "selected": "1"
    "active": true
}

This would be the correct and expected result, and we can access the parameters based on their proper names as opposed to relying on their order within the struct. This is more sensible as an object with named parameters, and would be immune to problems associated with re-ordering.

This last case is compounded when an array of struct is returned. In that case an array of arrays is returned, having lost all contextual information.

Checklist

Environment

Hosted webpage. Following scripts are loaded on the page

  <!-- jQuery js -->
  <script src="https://code.jquery.com/jquery-3.6.0.min.js"
    integrity="sha256-/xUj+3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=" crossorigin="anonymous"></script>
  <!-- web3 js -->
  <script src="https://cdn.jsdelivr.net/npm/web3@latest/dist/web3.min.js"></script>
  <!-- WalletConnect js -->
  <script type="text/javascript"
    src="https://unpkg.com/@walletconnect/web3-provider@1.2.1/dist/umd/index.min.js"></script>
  <!-- moralis js -->
  <script src="https://unpkg.com/moralis/dist/moralis.js"></script>

Server

Client

ErnoW commented 2 years ago

Good feature request. It is not really related to the sdk codebase though, as it is an implementation of the moralis web3 api. Better is to add a feature request to our roadmap: https://roadmap.moralis.io/

Closing this issue, as it is not something we can fix in the sdk now.

mloit commented 2 years ago

Not sure I'd classify it as a feature, as the results are incorrect IMHO. Having said that I understand now that this is happening at a higher level, so I'll bubble up the request. Thanks