EOSIO / eosio.cdt

EOSIO.CDT (Contract Development Toolkit) is a suite of tools used to build EOSIO contracts
http://eosio.github.io/eosio.cdt
MIT License
512 stars 289 forks source link

abi generation and de/serialization with nested maps #1042

Open dallasjohnson opened 3 years ago

dallasjohnson commented 3 years ago

std::map types with nested std::map types not working with abi-gen

I'm having trouble nesting std::maps in ACTION signatures and having them convert correctly through abigen.

A single level of type traversal works for std::map to extract primitive types or pre-defined structs but it doesn't seem to work for extracting types recursively when the value type of a std::map is also a std::map

For example a contract action like this:

ACTION adddetails(name actor, map<string, string> details, map<string, map<string, string>> nested_details) {...}

the name and details parameters are generated in the abi and behave as expected but the nested_details parameter does not work.

The generated abi contains the following:

...
{
    "structs": {
    {
        "name": "pair_string_string",
        "base": "",
        "fields": [
            {
                "name": "key",
                "type": "string"
            },
            {
                "name": "value",
                "type": "string"
            }
        ]
    },
    {
        "name": "adddetails",
        "base": "",
        "fields": [
            {
                "name": "actor",
                "type": "name"
            },
            {
                "name": "details",
                "type": "pair_string_string[]"
            },
            {
                "name": "nested_details",
                "type": "pair_string_pair_string_string[][]"
            },
        ]
    }

I would have expects another struct to be extracted out as pair_string_pair_string_string[] perhaps with a format like this:

 {
        "name": "pair_string_pair_string_string[]",
        "base": "",
        "fields": [
            {
                "name": "key",
                "type": "string"
            },
            {
                "name": "value",
                "type": "pair_string_string[]"
            }
        ]
    },

This would then refer to the same extracted type from the details paramater type pair_string_string with the vector wrapper ([]) but this doesn't get generated, suggesting the abi type generation for maps is not recursive. When I tried adding it manually I was able install the abi into the blockchain but then I would get runtime errors in the contract related to "uninitialized table element" on this action which I believe is related to this point in the contract code although it's a little hard to be sure with only caveman debugging available. From this error message the only reference I could find to in the cdt code was here: https://github.com/EOSIO/eosio.cdt/blob/a6b8d3fc289d46f4612588cdd7223a3d549238f6/tools/external/wabt/src/interp.cc#L1555 From there I got a bit lost to able able to pinpoint the cause any further.

Other details not directly related to this issue

I have seen comments on Telegram saying that std::maps are quite expensive and their use should be minimised but when I consider an alternative structure here could be to provide a parameter of vectors (or possibly nested vectors) of another struct which contains the key as a field on that struct but I feel that would severely impact the readability of the ACTION signature since a vector does not implicitly enforce uniqueness across any keys as is implied with a std::map. Another option could be to have a nested struct which only holds a map to wrap the nested map so the parameter would become map<string, nested_map_wrapper> nested_details where the nested_map_wrapper would only contain a map<string, string> nested_details field. but this feels like an unnecessary level when there is not other reason to have it.

It's worth noting the nested types are not required to exist in their own multi-index table row since they would never be queried or used without the context of the parent key.

nksanthosh commented 3 years ago

Can you please tell us which version of CDT you are using? eosio-cpp --version

There also exists a workaround for this as outlined in https://github.com/EOSIO/eosio.cdt/issues/658#issuecomment-571916106 - can you please let us know if this workaround is acceptable for your needs currently. We are also tracking the complete compatibility matrix as outlined https://github.com/EOSIO/eosio.cdt/issues/231 .

bogniq commented 3 years ago

I tested both develop and release\1.8.x branches, and the generated ABI doesn't contain definition for pair_string_pair_string_string mentioned above (see map_nest.cpp.txt and map_nest.abi.txt).

At the moment, typedef or using can be applied as a workaround (see map_nest2.cpp.txt and map_nest2.abi.txt as an example).

softprofe commented 2 years ago

After this PR https://github.com/EOSIO/eosio.cdt/pull/1213, the common used template can be nested use. After this PR, use don't need use typedef for common used template, such as vector, set, deque, map, pair, tuple..., and if you look into https://github.com/EOSIO/eosio.cdt/blob/develop/tests/unit/test_contracts/explicit_nested_tests.cpp, you will see a list of test for these nested types, now these types works well.