Support Templated dataSources

[dOrgJelli]

graphprotocol/graph-node#832

Here's my shot at a Checklist needed for this:

• Update generate-subgraph.js to support templates:
• Add templates to the mappings that will be creating them (DAORegistry, ?)
• Ensure existing tests still run and nothing's broken.
• Add a test that deploys a new DAO, registers it in the registry, and verifys the subgraph is updated accordingly.

[dOrgJelli]

This is looking like it'll require a large refactor to ensure it's done correct.

Currently the subgraph has all pre-deployed contracts (as you know), which are embedded in the subgraph.yml. This new templated datasources feature should be used for all instance based contracts (non-universal). Doing this is a large refactor, and would change how the whole project looks and acts (ex: test initialization).

@ben-kaufman has offered to chat with me tomorrow about this all, so I'll see if my analysis is correct. Will respond back soon next steps.

[ben-kaufman]

Hi, thanks for opening this issue!
I would actually recommend using the DAORegistry (which is already supported to some extent) instead of listening directly to the DaoCreator contract. This is in order to prevent spamming the subgraph with DAOs which should not be tracked.

I'm still reading the Graph Node PRs which implemented this dynamic feature to see how to write those templates.
We should probably start by moving only a single contract, for example, the Avatar to this dynamic structure and see if it works well, then continue to the rest.

We can further discuss all of that in our call later today.

[leviadam]

Hi @dOrgJelli :)

I agree with @ben-kaufman. We need to start simple.
I think we can maybe work first on a repo with a simple example. This will be the best way to understand this new feature.
Also, I agree we should use arc-hive.

[orenyodfat]

yes , on first phase we need a basic support for dynamically adding new daos (avatar, reputation,daotokens ,controller(if not using universal)).

@dOrgJelli please note that DaoRegistry already indexed by the subgraph.

[dOrgJelli]

I've done some initial R&D around this and realized the changes needed don't need to be a refactor, but instead an addition to what's existing. The pre-deployment of DAOs (in the daos folder) doesn't need to be changed to use this new feature IMO. I've updated the above checklist to reflect the new plan, and have worked on executing it here:
dOrgTech#2

In response to "only indexing DAOs that're registered in the registry", it looks like this will result in all events fired before the registration to be lost. I'm creating tests to verify this is correct, but the explanation of the feature states this:
"Whenever we we process a block and mappings request new data sources, these data sources are collected and, after having processed the block, are instantiated from templates. We then process the current block again but only with those new data sources."
graphprotocol/graph-node#832 (comment)

If this is in fact the case, I propose we'll need to start indexing these new DAOs when DAOCreator.forgeOrg is called to make sure we don't miss anything.

[dOrgJelli]

Also here is an example of what ops/generate-subgraph.js now supports as input & what it outputs:

input - mapping/DAORegistry/datasource.yaml

abis:
  - DAORegistry
entities:
  - DAORegistryContract
eventHandlers:
  - event: Propose(address)
    handler: handlePropose
  - event: Register(address,string)
    handler: handleRegister
  - event: UnRegister(address)
    handler: handleUnRegister
templates:
  - Avatar
  - Controller
  - Reputation
  - DAOToken

output - subgraph.yaml

- kind: ethereum/contract
    name: DAORegistry
    network: private
    source:
      address: '0x67B5656d60a809915323Bf2C40A8bEF15A152e3e'
      abi: DAORegistry
    mapping:
      kind: ethereum/events
      apiVersion: 0.0.1
      language: wasm/assemblyscript
      file: src/mappings/DAORegistry/mapping.ts
      entities:
        - DAORegistryContract
      abis:
        - name: DAORegistry
          file: ./abis/DAORegistry.json
      eventHandlers:
        - event: Propose(address)
          handler: handlePropose
        - event: 'Register(address,string)'
          handler: handleRegister
        - event: UnRegister(address)
          handler: handleUnRegister
    templates:
      - kind: ethereum/contract
        name: Avatar
        network: private
        source:
          abi: Avatar
        mapping:
          kind: ethereum/events
          apiVersion: 0.0.1
          language: wasm/assemblyscript
          file: src/mappings/Avatar/mapping.ts
          entities:
            - AvatarContract
          abis:
            - name: Avatar
              file: ./abis/Avatar.json
          eventHandlers:
            - event: 'SendEther(uint256,address)'
              handler: handleSendEth
            - event: 'ReceiveEther(address,uint256)'
              handler: handleReceiveEth
      - kind: ethereum/contract
        name: Controller
        network: private
        source:
          abi: Controller
        mapping: ...

[orenyodfat]

the dynamic data source feature enable to re scan the blockchain for new data source and index that ? does it ?
is there an issue to add data source upon daoregistery Propose or Registery event ?

[dOrgJelli]

@orenyodfat the feature allows the subgraph mapping to add new data sources (contract addresses) to the graph node. These newly added data sources (addresses) will then be watched for events.

AFAIK (need to test and verify), the re-scanning is limited to the current block (the block that caused the new data source to be created). This means that events prior to the registration of the DAO in the registry would never be processed.

If we can guarantee that the DAO will be Proposed in the DAORegistry in the same block it is created, then we can guarantee no data will be lost.

[orenyodfat]

it will be nice to have the subgraph re scanning from a specific block, even past one ,for incoming data source. we might ask the graph team for that.

[dOrgJelli]

Definitely agree. It would be nice if the re-scanning started at the block the newly added contract was created at.

I suspect they will have some push-back with this feature though, as partial updates to the graphQL data structures could occur from those past events, which would put the data in the cache in an undefined state.

For now, I propose we add the DaoCreator.sol to the subgraph, and index all newly created DAOs. If the size of the subgraph gets out of hand we can maybe add some pruning functionality if need be.

Thoughts @orenyodfat ?

[orenyodfat]

@dOrgJelli could you please open an issue on the graph-node for the re-scanning ?

I would like to avoid mapping the daocreator (and depend on that) as it is not mandatory to use for deploying a Dao . it is an helper contract. e.g dxdao does not use daocreator

[dOrgJelli]

@orenyodfat issue created here: graphprotocol/graph-node#902

[dOrgJelli]

After conversations with @orenyodfat and @leviadam, it looks like a workaround is emerging:

1. Precompute contract addresses before deployment.
2. Give addresses to the subgraph so it can create new data sources.
3. Deploy contracts.

To go into more detail:

1. address = sha3(rlp_encode(creator_account, creator_account_nonce))[12:] from this article.
2. I see two options here, one of which I prefer over the other.
   Option 1 is to expose an endpoint on the subgraph that allows an outside caller to create data sources. For example https://graph.alchemy.io/create/avatar?address=0x.... The potential problem I see here is that graph-node supports creating new data sources within a specific mapping module, and being able to invoke this from the outside seems sketchy.
   Option 2 is to propose the DAO be added to the DAORegistry before deploying the contracts, and having that event mapping create the new data sources. This to me seems ideal, although it'd require changing the Propose(address) event to not only include the address of the Avatar, but also the addresses of the Controller, Reputation, and DAOToken.
3. Deployment is straightforward, but we must ensure the deployer's account doesn't make other transactions before deployment or else the precomputed addresses would be incorrect (as the account nonce would change).

Thoughts on this?

[orenyodfat]

How about set the data source /or register the dao immediately after deploying the dao ? This will eliminate the need to precculate the address (solve section 3 concern ) . In this case there is very slight chance that between deployment and setting the data source there will be activity on the dao ..though it is very unlikely . The daoregistery is used by alchemy to fillet daos ,you can register dao on any time... not sure that tie that to indexing the dao is best option. There are many other ways to spam the subgraph as well .

…

On Sat, 4 May 2019 at 0:40 Jordan Ellis ***@***.***> wrote: After conversations with @orenyodfat <https://github.com/orenyodfat> and @leviadam <https://github.com/leviadam>, it looks like a workaround is emerging: 1. Precompute contract addresses before deployment. 2. Give addresses to the subgraph so it can create new data sources. 3. Deploy contracts. To go into more detail: 1. address = sha3(rlp_encode(creator_account, creator_account_nonce))[12:] from this article <https://swende.se/blog/Ethereum_quirks_and_vulns.html>. 2. I see two options here, one of which I prefer over the other. *Option 1* is to expose an endpoint on the subgraph that allows an outside caller to create data sources. For example https://graph.alchemy.io/create/avatar?address=0x.... The potential problem I see here is that graph-node supports creating new data sources within a specific mapping module, and being able to invoke this from the outside seems sketchy. *Option 2* is to propose the DAO be added to the DAORegistry before deploying the contracts, and having that event mapping <https://github.com/daostack/subgraph/blob/da5388863c385cfe35a3e455a6c3c8bddcc7e181/src/mappings/DAORegistry/mapping.ts#L10> create the new data sources. This to me seems ideal, although it'd require changing the Propose(address) <https://github.com/daostack/subgraph/blob/da5388863c385cfe35a3e455a6c3c8bddcc7e181/src/mappings/DAORegistry/datasource.yaml#L4> event to not only include the address of the Avatar, but also the addresses of the Controller, Reputation, and DAOToken. 3. Deployment is straightforward, but we must ensure the deployer's account doesn't make other transactions before deployment or else the precomputed addresses would be incorrect (as the account nonce would change). Thoughts on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#197 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABNXPHSOI3PW7MXM4GWRBYDPTSWOTANCNFSM4HHERYYA> .

[dOrgJelli]

Okay I agree with part 1 @orenyodfat, we just have to make sure there aren't any events that'd be missed.

For part 2, what is the alternative if we don't use the registry? Should there be another contract that's being watched, or should we try and go the route of exposing a endpoint on the graph-node server?

[dOrgJelli]

A solution which doesn't use the DAOregistry has been proposed here: daostack/arc#640

Would love to get your thoughts when you get a chance @orenyodfat @leviadam @ben-kaufman

[ben-kaufman]

I think this is fully implemented now so closing the issue. Thanks again @dOrgJelli for the work on that!