Use reserved discriminant segments for IndexForAscTypeId
#3446
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lutter
approved these changes
Apr 12, 2022
| TransactionReceipt = 1000, | ||
| Log = 1001, | ||
| ArrayH256 = 1002, | ||
| ArrayLog = 1003, |
There was a problem hiding this comment.
I assume these are all not in real use yet?
There was a problem hiding this comment.
Positive.
Those types were created for #3373, which haven't gone live yet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the organization of
IndexForAscTypeIdvariants for future blockchain implementations.Motivation:
The incentive for this change is the expected (and desired) growth of supported blockchains in Graph-Node. Each new blockchain implementation also comes with a myriad of new AssemblyScript types defined in Graph-TS that need to be represented in the runtime. We have been appending new type IDs to the end of the
IndexForAscTypeIdenum, but the addition of new types proved to be convoluted and hard to reason about. We aim to improve the developer experience by removing this kind of friction.Rationale:
We propose that each blockchain implementation have a fixed and contiguous discriminant space of 1,000 items, starting to be enforced after our latest released implementations: Ethereum and Near.
Our goal is that every new supported blockchain, for instance, Tendermint, will have room to include new AssemblyScript class IDs without interweaving its discriminants with other blockchains.
Merging and Releasing instructions:
Although this PR interferes with memory allocation in the runtime, it should not disrupt previously compiled subgraphs because the currently released AssemblyScript type IDs were not touched.
However, any existing Tendermint prototype subgraphs will face errors due to this changes and needs to be recompiled and re-synced.
TODO: We need to reorganize the variants in graph-ts'
TypeIdenum to exactly match the variants and their kid rearranged by this PR.