sablier-labs / v2-subgraphs

Subgraphs for Sablier V2 token streaming protocol
6 stars 4 forks source link

Core and periphery entities are bundled together #13

Closed PaulRBerg closed 11 months ago

PaulRBerg commented 11 months ago

The core package includes non-V2 Core entities, such as Batch:

https://github.com/sablier-labs/v2-subgraphs/blob/b948ff927a4f1cecc4f23a4b37ff93bdb2f3f421/packages/core/schema.graphql#L204-L219

Such entities should either be moved to another package/ subgraph called periphery, or the package should be named differently to indicate that it contains logic for the Sablier V2 protocol as a whole (not just V2 Core).

Alternatively, we could remove the plural form of this repository name and keep the subgraph at the root since the PRBProxy subgraph is expected to be migrated over.

razgraf commented 11 months ago

non-V2 Core entities

There's some nuance to this. While it is true that the periphery has utilities revolving around grouping and batching, the Batch used in this subgraph is a core-helper system. It doesn't make use of the periphery contracts, it rebuilds the batches from scratch.

Such entities should either be moved

Hmm, not sure how this would work. These entities have to live together for the same subgraph to work and make use of them. The alternative would be having a different subgraph just for this batching element, which wouldn't be helpful - we need it in the same queries as the core data.

or the package should be named differently

Agree this might be helpful, seeing as the nuances can lead to interpretations. Another name would make it a bit clearer that one will see additional things, not just core-contracts concepts. Any suggestions? Maybe a synonym for core such as nucleus?

we could remove the plural form

I would wait a bit before doing this. The Airstreams system will most likely have a separate subgraph to track the factory. Waiting for that opportunity to (1) add another package to the directory and (2) figure out how to share code/entities between subgraphs, if possible (and if needed).

P.S. I think this would have made for a good "Discussion" rather than an "Issue".

PaulRBerg commented 11 months ago

While it is true that the periphery has utilities revolving around grouping and batching, the Batch used in this subgraph is a core-helper system

Fine but in this case it is misleading to call that package core. V2 Core means something very specific in the wider context of Sablier V2, and V2 Core does not have any knowledge of batch-like systems.

These entities have to live together for the same subgraph to work and make use of them

Very well

Agree this might be helpful ... Any suggestions?

I think this would have made for a good "Discussion" rather than an "Issue".

It's pretty clear that we need to rename the core folder in this repo. Discussing topics in the Issues channel that we know for certain we want to adjust is fine IMO, even if we don't have a solution from the get-go.