hypha-dao / document-graph

EOSIO contracts for modeling a graph data structure with flexible data storage of each document
MIT License
10 stars 3 forks source link

RFC: Include contract in Document and Edge classes #12

Closed mgravitt closed 3 years ago

mgravitt commented 3 years ago

We are refactoring Document and Edge classes/structs significantly for improved design, which will also enable the structure to be used in many contracts as plug-and-play. Do the below recommendations make the structure the most re-usable?

Here's the current view of what Edge would look like:

       struct [[eosio::table, eosio::contract("docs")]] Edge
       {
        Edge();
        Edge(eosio::name contract, const eosio::name &creator, const eosio::checksum256 &fromNode,
             const eosio::checksum256 &toNode, const eosio::name &edgeName);
        ~Edge();

        void emplace ();

        uint64_t id;

        // these three additional indexes allow isolating/querying edges more precisely (less iteration)
        uint64_t from_node_edge_name_index;
        uint64_t from_node_to_node_index;
        uint64_t to_node_edge_name_index;

        eosio::checksum256 from_node;
        eosio::checksum256 to_node;
        eosio::name edge_name;
        eosio::time_point created_date;
        eosio::name creator;

        uint64_t primary_key() const;
        uint64_t by_from_node_edge_name_index() const;
        uint64_t by_from_node_to_node_index() const;
        uint64_t by_to_node_edge_name_index() const;
        uint64_t by_edge_name() const;
        uint64_t by_created() const;
        uint64_t by_creator() const;
        eosio::checksum256 by_from() const;
        eosio::checksum256 by_to() const;

        typedef eosio::multi_index<eosio::name("edges"), Edge,
                                   eosio::indexed_by<eosio::name("fromnode"), eosio::const_mem_fun<Edge, eosio::checksum256, &Edge::by_from>>,
                                   eosio::indexed_by<eosio::name("tonode"), eosio::const_mem_fun<Edge, eosio::checksum256, &Edge::by_to>>,
                                   eosio::indexed_by<eosio::name("edgename"), eosio::const_mem_fun<Edge, uint64_t, &Edge::by_edge_name>>,
                                   eosio::indexed_by<eosio::name("byfromname"), eosio::const_mem_fun<Edge, uint64_t, &Edge::by_from_node_edge_name_index>>,
                                   eosio::indexed_by<eosio::name("byfromto"), eosio::const_mem_fun<Edge, uint64_t, &Edge::by_from_node_to_node_index>>,
                                   eosio::indexed_by<eosio::name("bytoname"), eosio::const_mem_fun<Edge, uint64_t, &Edge::by_to_node_edge_name_index>>,
                                   eosio::indexed_by<eosio::name("bycreated"), eosio::const_mem_fun<Edge, uint64_t, &Edge::by_created>>,
                                   eosio::indexed_by<eosio::name("bycreator"), eosio::const_mem_fun<Edge, uint64_t, &Edge::by_creator>>>
            edge_table;

    private:
        eosio::name contract;
        uint64_t createID(const eosio::checksum256 &from_node, const eosio::checksum256 &to_node, const eosio::name &edge_name);

        EOSLIB_SERIALIZE(Edge, (id)(from_node_edge_name_index)(from_node_to_node_index)(to_node_edge_name_index)(from_node)(to_node)(edge_name)(created_date)(creator))
    };

Since reading and writing to the edge_table will occur within the Edge class, we add the contract to the constructor and also as a private member. Even though we don't need contract to be saved within the table, the ABI generator automatically generates it and puts it into the table definition.

If we do this, we will need migrate existing data as part of the deployment. Not a big deal, but definitely work.

Cross-contract edges

Not related to the above, but also impacts data structure. It's possible that we will need to support cross-contract edges in the future. For example, redemptions held in bank.hypha will likely become documents and will need to be linked to the members in dao.hypha.

Due to the manner that we are indexing edges (all combinations of the columns), the indexes would grow significantly by adding two edges to each row. There are already 8 indexes, and adding two additional rows would require 16 more if we held with the same pattern. This may not be required, but would certainly add complexity.

Instead, I think we would use base ABI functionality described here: https://developers.eos.io/welcome/latest/getting-started/smart-contract-development/understanding-ABI-files/#struct-base

I have not used this before, but it seems that it allows support for inheritance. If and when needed, we would create a CrossContractEdge struct which would inherit from Edge and the two additional members and needed indexes.

ABI generator tag

@Gerard097 created a Macro used in the current version to alleviate the need to edit the DocumentGraph struct definitions for each contract. Is that still compatible with the above?

Recommendation

Gerard097 commented 3 years ago

I was thinking we might not need to store the contract in the edge, but It mostly dependes on how the edge and where is it going to be used. I.e we could simply send the contract as a parameter when calling the emplace/fetch function but this would require the caller to know the contract, and as said before depending on where we want to deal with the edge object this might not be posible.

About the Cross-contract edges I think it's a good idea to have them as a separated struct so we don't enforce the use of the extra slots if the contract doesn't really needs it (external usage).

The macro will need some modifications but I think we can still use that method to generate the ABI tags, the earlier we test this the better.

sebastianmontero commented 3 years ago

It looks good to me @dappdever, I didn't know that structs supported inheritance, it's a nice feature and it makes sense to use it for the cross contract edges, but would it create a new table?