Closed dOrgJelli closed 4 years ago
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.
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.
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.
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.
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:
https://github.com/dOrgTech/subgraph/pull/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." https://github.com/graphprotocol/graph-node/pull/832#issue-262803861
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.
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: ...
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 ?
@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 Propose
d in the DAORegistry
in the same block it is created, then we can guarantee no data will be lost.
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.
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 ?
@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
@orenyodfat issue created here: https://github.com/graphprotocol/graph-node/issues/902
After conversations with @orenyodfat and @leviadam, it looks like a workaround is emerging:
To go into more detail:
address = sha3(rlp_encode(creator_account, creator_account_nonce))[12:]
from this article.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
.Thoughts on this?
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 notifications@github.com wrote:
After conversations with @orenyodfat https://github.com/orenyodfat and @leviadam https://github.com/leviadam, it looks like a workaround is emerging:
- Precompute contract addresses before deployment.
- Give addresses to the subgraph so it can create new data sources.
- Deploy contracts.
To go into more detail:
- address = sha3(rlp_encode(creator_account, creator_account_nonce))[12:] from this article https://swende.se/blog/Ethereum_quirks_and_vulns.html.
- 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.
- 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 https://github.com/daostack/subgraph/issues/197#issuecomment-489248494, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNXPHSOI3PW7MXM4GWRBYDPTSWOTANCNFSM4HHERYYA .
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?
A solution which doesn't use the DAOregistry has been proposed here: https://github.com/daostack/arc/pull/640
Would love to get your thoughts when you get a chance @orenyodfat @leviadam @ben-kaufman
I think this is fully implemented now so closing the issue. Thanks again @dOrgJelli for the work on that!
https://github.com/graphprotocol/graph-node/pull/832
Here's my shot at a Checklist needed for this:
generate-subgraph.js
to supporttemplates:
DAORegistry
, ?)