-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Processor design RFCs #125
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout for the manifest file
-
What part of this has to be created by hand vs. created via tooling? In particular all the information about the handlers.
-
Why does the signature information need to be specified in the handlers? As long as you have a fully specified name (module + module level name), then there can be only one event or extrinsic, as there is no overloading. The full signature should then be available from some external resource (the chain metadata file?) This part seems like it can get really rough to keep in synch with chain if 100% manual.
-
The notion of a
mapping script
,Mappings API
andvirtual events
are not really defined anywhere, despite being referenced quite a bit. I think some sort of glossary or defintion secton would be a benefit. -
If
EventHandler.virtual
is true, then it appears the described format forEventHandler.event
. Perhaps there needs to be a distinctVirtualEventHandler
? -
If I care about what extrinsic is causing my event, by using the
EventHandler.extrinsic
, then don't I almost certainly need to look at its parameter values as well in my event mapper? In which case isn't this manifest level filtering going to be redundant pre-filtering? -
My assumption is that if I want to get a Hydra instance going for a given runtime, where the mappings and manifest has been prepared by someone else, then I should not really edit that file. The manifest, as I have understood it, be about coordinating the inherent components of a Subgraph, not about a particular deployment of it. However, this format does not follow that principle, as I need to edit it to change the
Hydra Indexer Source
. If my premise is correct, then not only should the endpoint not be in the manifest, but even the blocks possibly should not, as a particular manifest is really just locked to a runtime or runtime module, which itself could exist in different chains in different block intervals. Ultimately, it depends on what we see the role of the manifest being. -
dataSources
, along with its description, indicates multiple sources, but the type does not, should the type perhaps be[DataSource]
or something, signalling a vector? -
Why is
entities
actually needed inMapping
, is there some benefit to respecifying it here, as the dev will need to keep it in synch with the input file. -
The
filter
property seems to me to be a risky proposition with unclear upside. By splitting the rule for what extrinsic invocations actually impact processor state across the mapping code and the manifest file, it seems its easy to lose track of what the effective rule is over time. -
What is the purpose of
ExtrinsicHandler.exports
? -
BlockHandler
probably needs to have an option for whether it runs before or after all other mappings, as this would correspond toon_initialize
andon_finalize
in the runtime. Thehandler
property description referenes an event, I assume this is a copy&paste error.
Proposal for generating type-safe mappings for events
-
I seem to recall that some feature called
typegen
needed to be part of this to makethings properly automatic, this is what Leszek said, is that still the case here? -
This appears to not allow extrinsic only type safe mappings? Why is that? I suspect this to be used 99% of the time, as our current event based approach degenerates into working back into the extrinsic payload. The only reason to process an event is if the side-effect is triggered by an inherent (on_finalized, on_initialized) or it's triggered by multiple extrinsics, and you would like to avoid dealing with them separately, AND (for both cases) the event has sufficient information to compute the full side-effect. This latter constraint is almost never satisfied in our runtime.
-
Once we have transactional handlers, I cannot think of a single use case off the top of my head where we want to lock in pairs of extrinsics and events. I also think the example would get really messy with you had lots of pairs like this, you would have
Mod1Extrinsc2BalancesBalancesSetEvent
or something like that to capture combos of different extrinsics causing balance setting. If we do not have this, the example just boils down to what The Graph would have, which would be a single typeBalances.BalanceSetEvent
which would have the layout of currentBalanceSet__Params
. -
I did not understand when we would end up in a situation like
BalanceSet__Params
, where there are no names for params, doesn't the metadata file have names for all extrinsics and events? Seems so https://whisperd.tech/post/substrate_metadata/. -
In the example, the module name for an event is prefixing the name of the type extending
SubstrateEvent
, e.g.BalancesBalanceSetEvent
. Could we use proper typescript scoping/namespacing to increase readability here, so it would beBalances.BalanceSetEvent
?
Virtual events
-
I suggest the "Goals and motivations" section only discussed the problem/limitations being addressed in an approach without virtual events. Right now its simultanously describing the problem & solution. A high level description introducing the virtual events high levle "solution" to the stated problems deserves to live in it's own section perhaps.
-
Isn't just having transaction handlers an equally suitable solution to the first point in the motivations?
-
In the context of EVM support, I sort of interpret these virtual events as our current pre-handlers, with the main difference being that pre-handles directly call what method, while this is lifted out into the manifest file here. Is that correct? I presume the main value-add of this is that you can just grab someone elses virtual event generator off-the shelf, without needing to mess with changing what they call. If this is correct, then it would seem that perhaps virtual event generators should be a distinct kind of extrinsicshandler which is pure, i.e. it does not access the database. Because you really dont want to have to audit exactly what some off-the shelf pre-handler does or does not do.
-
I don't actually entirely see a smooth way of getting virtual event generators from the outside world and integrating them into my setup. It would seem to require quite a lot of manual stitching, copying, editing of manifest files, etc.
If the virtual event generators change, then you will probably need to do a bit of brittle housekeeping? I assume this can be made smoother. -
If the prior point is correct, then the main new value unlocked is presumably the EVM handling, but for this specific problem I think it seems we are some ways off from the full experience. Lets say I want to write a query node for some new smart contracts. With the virtual events approach it seems I would need to sit down and dig into how to handle extrinsics to the EVM pallet, which is very complex, and I would have to hand-craft types that mirror the different parameters and extrinsics I care about. Once I had all this, I could write my actual handlers for these. At this point, if someone wanted to write another query node for the same smart contract, they could get my event emitters, and map events differently depending on what queries they wanted to support. Given that its often going to be one canonical query set of interest to a given system, the value of the reusability unlocked is relatively low. What I really want to be able to do is what we are thinking I can do in the type safe Substrate case. I want to provide the ABI of my contracts to the codegen tool, and I want to tell it that I care about methods x,y,z, and it gen generate all the required types I want so that I can just jump into writing handlers. In principle the codegen tool could generate virtual event generators, but that is only a technicality at that point.
I just mentioned Comments to Strongly typed mappings proposalIt looks really interesting and useful, I don't think I can add much to what @bedeho already mentioned, but I'll just mention a couple of things that came to my mind:
So I imagine this to be a file like the By default those files do not re-export any interfaces/classes from
Is the metadata file just the metadata queried from the chain via There is a Joystream chain currently uses
So if I understand correctly we will have a separate file like I was just worrying about possible In such case:
We currently sidestep those issues along with some monorepo-setup related ones by having a
Is it possible for substrate events to have named parameters? (I don't think I've seen a case like that) I think that for unnamed parameters something like
Here I was thinking about events that can be emitted by multiple different extrinsics and therefore having different As already mentioned by @bedeho in that case in order to make If we do want to handle events that can be possibly emitted by multiple different extrinsics that would probably require more consideration. |
c769c56
to
54509f2
Compare
* fix: json list codegen issues affects: @subsquid/hydra-cli, hydra-e2e-tests * style: fix lint errors affects: @subsquid/hydra-cli, hydra-e2e-tests
This PR adds three RFCs for the new Hydra processor:
/docs/rfc/manifest-file.md
)type-safe-mappings.md
)virtual-events.md
)