Skip to content

feat(graph-gateway): add internal query client to network subgraph service#856

Merged
LNSD merged 1 commit intomainfrom
lnsd/feat-gg-network-subgraph-indexer-client
Jul 12, 2024
Merged

feat(graph-gateway): add internal query client to network subgraph service#856
LNSD merged 1 commit intomainfrom
lnsd/feat-gg-network-subgraph-indexer-client

Conversation

@LNSD
Copy link
Contributor

@LNSD LNSD commented Jun 20, 2024

This PR removes the Gateway's dependency on the hosted service by adding a graph network subgraph client capable of directly querying an indexer from a configurable list of trusted indexer candidates.

This PR resolves #685

@LNSD LNSD requested a review from Theodus June 20, 2024 14:55
@LNSD LNSD self-assigned this Jun 20, 2024
@LNSD LNSD force-pushed the lnsd/feat-gg-network-subgraph-indexer-client branch 6 times, most recently from 9fc5d77 to 3222176 Compare June 24, 2024 15:15
@LNSD LNSD marked this pull request as ready for review June 24, 2024 15:19
@LNSD LNSD force-pushed the lnsd/feat-gg-network-subgraph-indexer-client branch from 3222176 to e8df44e Compare June 24, 2024 16:07
@LNSD LNSD marked this pull request as draft June 24, 2024 16:55
@LNSD LNSD force-pushed the lnsd/feat-gg-network-subgraph-indexer-client branch from e8df44e to 2bff1a0 Compare June 25, 2024 09:53
@LNSD LNSD marked this pull request as ready for review June 25, 2024 10:41
Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm testing this out on the local network before merging

Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't compatible with the indexer-service API. It's incorrectly expecting the response that the gateway returns. See the indexer_client for what the indexer-service returns:

pub struct IndexerResponsePayload {
#[serde(rename = "graphQLResponse")]
pub graphql_response: Option<String>,
pub attestation: Option<Attestation>,
pub error: Option<String>,
}

@LNSD
Copy link
Contributor Author

LNSD commented Jun 26, 2024

This isn't compatible with the indexer-service API. It's incorrectly expecting the response that the gateway returns. See the indexer_client for what the indexer-service returns:

I find this "non-standard" GraphQL-over-HTTP disturbing (to say something). I believe that we should follow the GraphQL specs to reduce the amount of work we need to do 😞

@Theodus Theodus self-requested a review June 26, 2024 16:06
@Theodus
Copy link
Member

Theodus commented Jun 26, 2024

Still getting errors on the local network:

Failed to fetch network subgraph from indexer indexer=0xf4EF6650E48d099a4972ea5B414daB86e1998Bd3 error=bootstrap meta query failed: Error sending subgraph meta query: Error deserializing GraphQL response. Unexpected response: {"graphQLResponse":"{\"data\":{\"meta\":{\"block\":{\"number\":93,\"hash\":\"0x303c4d5a90f8f8effdb80e57d262e0c22e70f72db2207662a6ef6d6a295f3e7a\"}}}}"}. Error: invalid type: string "{\"data\":{\"meta\":{\"block\":{\"number\":93,\"hash\":\"0x303c4d5a90f8f8effdb80e57d262e0c22e70f72db2207662a6ef6d6a295f3e7a\"}}}}", expected struct SubgraphMetaQueryResponse at line 1 column 150

#[derive(Debug, Deserialize)]
pub struct IndexerResponsePayload<T> {
#[serde(rename = "graphQLResponse")]
pub data: Option<T>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be data: Option<String> which then gets parsed into T after receiving it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this what serde does?

  • If the field is not present, it sets the field to None.
  • If it is present, it deserializes it into the T type.

Copy link
Contributor Author

@LNSD LNSD Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I just saw the error message.

It seems that we are escaping the JSON object as a string in the indexer service... Another deviation from the GraphQL-over-HTTP spec 😓

@LNSD LNSD force-pushed the lnsd/feat-gg-network-subgraph-indexer-client branch from 9aad162 to 5848c6b Compare July 8, 2024 13:26
@LNSD LNSD requested a review from Theodus July 8, 2024 13:28
Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still failing on the local network:

Failed to fetch network subgraph from indexer indexer=0xf4EF6650E48d099a4972ea5B414daB86e1998Bd3 error=bootstrap meta query failed: Error sending subgraph meta query: Error deserializing GraphQL response. Unexpected response: {"graphQLResponse":"{\"data\":{\"meta\":{\"block\":{\"number\":92,\"hash\":\"0x39693f38371b63f04710bdd480c3e20ab3f6693f01772417db63b7db6a8ed8df\"}}}}"}. Error: invalid type: string "{\"data\":{\"meta\":{\"block\":{\"number\":92,\"hash\":\"0x39693f38371b63f04710bdd480c3e20ab3f6693f01772417db63b7db6a8ed8df\"}}}}", expected a borrowed string at line 1 column 150

Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more errors:

  • deserializing the response body still fails (works if &'data str is replaced with String)
  • the request fails with "Missing attestation". Attestations aren't expected since we're using the free query route.

@LNSD LNSD force-pushed the lnsd/feat-gg-network-subgraph-indexer-client branch from 374e709 to 6d73e38 Compare July 9, 2024 13:45
@LNSD LNSD requested a review from Theodus July 10, 2024 08:36
…rvice

Signed-off-by: Lorenzo Delgado <lorenzo@edgeandnode.com>
@LNSD LNSD force-pushed the lnsd/feat-gg-network-subgraph-indexer-client branch from 6d73e38 to e5b9129 Compare July 10, 2024 09:54
Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LNSD LNSD merged commit 75113e8 into main Jul 12, 2024
@LNSD LNSD deleted the lnsd/feat-gg-network-subgraph-indexer-client branch July 12, 2024 13:59
@Theodus Theodus mentioned this pull request Jul 15, 2024
Theodus added a commit that referenced this pull request Jul 15, 2024
# Release Notes
- fix: skip POI fetch until cache TTL expires (#860)
- feat: set gateway ID to signer address (#868)
- feat: add internal query client to network subgraph service (#856,
#880)
- feat: direct indexer query API (#867)
- fix: remove deprecated data science logs (#882)

# Configuration Changes
- `gateway_id` removed
- `network_subgraph` replaced by `trusted_indexers`
Theodus added a commit that referenced this pull request Jul 15, 2024
- fix: skip POI fetch until cache TTL expires (#860)
- feat: set gateway ID to signer address (#868)
- feat: add internal query client to network subgraph service (#856,
- feat: direct indexer query API (#867)
- fix: remove deprecated data science logs (#882)

- `gateway_id` removed
- `network_subgraph` replaced by `trusted_indexers`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

facilitate indexer queries independent of client queries

2 participants