Add a Mirror Node GraphQL API #668

steven-sheehy · 2023-02-01T04:50:46Z

Description:

Related issue(s):

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

netlify · 2023-02-01T04:50:56Z

✅ Deploy Preview for hedera-hips ready!

Name	Link
🔨 Latest commit	`c97d3db`
🔍 Latest deploy log	https://app.netlify.com/sites/hedera-hips/deploys/64232ff8ba00500008ddaeb3
😎 Deploy Preview	https://deploy-preview-668--hedera-hips.netlify.app/hip/hip-668
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

steven-sheehy · 2023-02-01T05:01:58Z

HIP/HIP-668.md

+  The admin key associated with this entity whose signing requirements must be met in order to modify the entity on
+  the network.
+  """
+  key: Object


This dynamic structure is a bit unique. Need to document it separately somewhere.

shemnon · 2023-02-01T22:46:53Z

HIP/HIP-668.md

+"An ISO 8601 compatible duration with support for nanoseconds granularity in the format P[n]Y[n]M[n]DT[n]H[n]M[n]S."
+scalar Duration
+
+"A 64-bit numeric type."


Signed or unsigned? I think it should be explicit.

True, will add a note that it's signed. Built-in types like Int are signed and this one will be as well, especially since it's backed by Java which only has signed.

steven-sheehy · 2023-02-02T04:49:15Z

HIP/HIP-668.md

+type EntityId {
+  "The unique incrementing identifier associated with this entity."
+  num: Long!
+
+  "The realm number. Currently always zero."
+  realm: Long!
+
+  "The shard number to allow for horizontal scaling of networks. Currently always zero."
+  shard: Long!
+}
+
+"The unique identifier to find an entity on the Hedera network."
+input EntityIdInput {
+  "The unique incrementing identifier associated with this entity."
+  num: Long! @Min(value: 0)
+
+  "The realm number. Defaults to zero."
+  realm: Long! = 0 @Min(value: 0)
+
+  "The shard number to allow for horizontal scaling of networks. Defaults to zero."
+  shard: Long! = 0 @Min(value: 0)
+}


Do people prefer the EntityId as a complex type or due to its wide use in the API prefer it as a 0.0.x scalar string like the REST API? It can get quite verbose to query for all these entity ids as accountId {shard, realm, num}

I like the scalar string since its all 0 shard 0 realm for now.
Just my two cents

I'd go with the complex type. I appreciate the 0 defaults and the verbosity but I think that's something that can be managed at the client level

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

Signed-off-by: Michael Garber <michael.garber@swirldslabs.com>

AlfredoG87 · 2023-02-07T16:33:24Z

This is cosmetic but by looking at the codebase, noticed that all the HIP Files are in lower case except this one:

Should we rename the file to "hip-668.md" just for consistency?

MarcKriguerAtHedera

I had thought I had submitted this last week, but I was just prompted to finish this review. Sorry for the delay.

Looks very good, just a few (3) nits.

MarcKriguerAtHedera · 2023-02-02T00:00:03Z

HIP/HIP-668.md

+GraphQL. Data should be nested and allow for pagination at different levels. Strongly typed objects should be preferred
+over complex string representations of multi-field objects similar to the protobuf-based HAPI. For example, the
+transaction ID `0.0.289304-1673028763-068736176` in the current API could be a GraphQL type `TransactionId` with
+`nonce`, `payerAccountId`, `scheduled`, and `validStart` fields where even those could be complex types.


This example isn't very clear. 0.0.289304 is probably the payerAccountId; I'm guessing 1673028763 is the validStart, so then 068736176 is either the nonce or scheduled -- but isn't one of those fields missing? Maybe make sure the fields appear in the same order in the TransactionId as they do, spelled out, and the example would be much clearer.

I removed the example string and just use the generic term string so as to not cause confusion and focus on the details of the old approach. The idea was to show that request/response is more strongly typed objects now, not to show how things map from the old to the new.

MarcKriguerAtHedera · 2023-02-02T00:04:22Z

HIP/HIP-668.md

+
+The new GraphQL API will be written as a new module in the open source Hedera Mirror Node repository. It will be Java
+based to allow for code reuse with its other modules. It will interact directly with the SQL database like the other
+modules. Creating it as separate module will allow it to be a microservice that can be scaled separately from other


(grammar) suggestion: add the word "a" between "as" and "separate module"

Thanks, added a.

MarcKriguerAtHedera · 2023-02-10T00:58:56Z

HIP/HIP-668.md

+"A rational number, used to provide an accurate number without any rounding issues and to provide consistency with HAPI."
+type Fraction {
+  "The rational's denominator. Will be a value of one when the numerator should be treated as a whole number."
+  denominator: Long!


Should this have a minimum value of 1?

No, it's not an input, only a response type.

ericleponner · 2023-02-22T07:29:48Z

Here are the various search forms enabled by HashScan:

Entity	Explorer Search Input	HIP-668
Account	Entity ID	ok
	EVM Address	ok
	Alias	ok
	Admin/Public Key	?
Contract	Entity ID	ok
Contract	EVM Address	ok
Token	Entity ID	ok
Topic	n/a
Transaction	Transaction ID	ok
Transaction	Transaction Hash (48bytes)	?

If we want to implement HashScan search with GraphQL, we would need something like this:

type Query {
        …
	accounts(keyInput: KeyInput!): [Account!]!
	transaction(hash: TransactionHashInput!): Transaction
        …
}

ericleponner · 2023-02-22T07:34:22Z

Also I see evmAddress fields in xxxInput only.
Shouldn't we have evmAddress fields in Contract and/or Account types ?

ericleponner · 2023-02-22T08:12:19Z

Call below returns one or zero transaction:

type Query {
    transaction(id: TransactionInput!): Transaction
}

Multiple transactions may have the same transaction id.
What is the expected result in that case ?

Shouldn't we have an additional call like this ?

type Query {
    transactions(id: TransactionId!): [Transaction]
}

ericleponner · 2023-02-22T10:36:57Z

To implement EIP 3091, HashScan needs to retrieve a transaction matching an Ethereum hash.
It does this by running two REST calls (api/v1/contracts/results/{eth} and api/v1/transactions?timestamp={t}).

Shouldn't we add this capability in GraphQL interface ?

input TransactionInput {
  …
  ethereumHash: XXX
  …
}

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

steven-sheehy · 2023-03-22T22:50:01Z

Should we rename the file to "hip-668.md" just for consistency?

Thanks, renamed.

steven-sheehy · 2023-03-22T22:56:55Z

type Query {
        …
	accounts(keyInput: KeyInput!): [Account!]!
	transaction(hash: TransactionHashInput!): Transaction
        …
}

This is good feedback. Unfortunately, we do not want to expose transaction hash until we have a scalable solution and it's implemented in all environments and supports more than just recent data. Definitely something that we want to add in the future, though.

For public key, the unfortunate problem is that users are free to create an unbounded number of accounts with the same public key. So if we want to support search by key then it would have to be a pageable Connection and not a list. Or we could put some hardcoded bounds on it? Will think on it.

steven-sheehy · 2023-03-22T22:59:09Z

To implement EIP 3091, HashScan needs to retrieve a transaction matching an Ethereum hash.

I've added the ability to search for a contract result by its ethereum hash, but it still requires a contract ID first. I think once we have the search for transaction by hash we can just utilize that for EIP 3091. Until then, you can keep using the REST API approach.

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

steven-sheehy · 2023-03-23T00:01:08Z

Also I see evmAddress fields in xxxInput only.
Shouldn't we have evmAddress fields in Contract and/or Account types ?

Yes, I was holding off on it until the dust settled on HIP-631 since it was adding multiple such virtual addresses. Since that has been delayed until Q3 and they're using the alias terminology, I went ahead and added the evmAddress field to contract and account.

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

steven-sheehy · 2023-03-23T00:56:24Z

Multiple transactions may have the same transaction id.
What is the expected result in that case ?

I was thinking just to return the successful one. Or to return one of the errored ones if only errors. But that's probably not ideal for all scenarios. So I changed it to return a list. Note I named it as transaction(id: TransactionInput!): [Transaction!]! since the method name has to be unique and I wanted to reserve transactions for a potential search API that returns an unbounded Connection.

Signed-off-by: Michael Garber <michael.garber@swirldslabs.com>

Nana-EC · 2023-07-06T00:32:15Z

HIP/HIP-668.md

+## Motivation
+
+The current mirror node REST API provides a functional API that returns most of the data users need. However, due to the
+nature of REST APIs, it suffers from problems inherit in REST itself including both under-fetching and over-fetching.


nit: I think you meant

Suggested change

nature of REST APIs, it suffers from problems inherit in REST itself including both under-fetching and over-fetching.

nature of REST APIs, it suffers from problems inherent to REST itself including both under-fetching and over-fetching.

Nana-EC · 2023-07-06T00:36:29Z

HIP/HIP-668.md

+the transaction ID being a string as in the REST API, it could be a GraphQL type `TransactionId` with
+`nonce`, `payerAccountId`, `scheduled`, and `validStart` fields where even those could be complex types.
+
+An existing GraphQL API for the Hedera network was created by community members under the name


Is this better located in the rejected ideas section?

Nana-EC · 2023-07-06T00:38:20Z

HIP/HIP-668.md

+graphs of data. The open source mirror node does not provide any guarantees of database schema compatibility between
+releases so this schema will become harder to maintain over time without the use of a view model abstraction layer.
+
+[The Graph](https://thegraph.com/) provides an indexing protocol for networks and allows anyone to publish sub-graphs


Not sure it's necessary to mention The Graph. As you noted it's main purpose is the indexing of data and GraphQL is used to expose the data. The mixup only occurs in the API choice and not the main problem being solved.
Maybe another section to move to rejected ideas.

Nana-EC · 2023-07-06T00:47:39Z

HIP/HIP-668.md

+type EntityId {
+  "The unique incrementing identifier associated with this entity."
+  num: Long!
+
+  "The realm number. Currently always zero."
+  realm: Long!
+
+  "The shard number to allow for horizontal scaling of networks. Currently always zero."
+  shard: Long!
+}
+
+"The unique identifier to find an entity on the Hedera network."
+input EntityIdInput {
+  "The unique incrementing identifier associated with this entity."
+  num: Long! @Min(value: 0)
+
+  "The realm number. Defaults to zero."
+  realm: Long! = 0 @Min(value: 0)
+
+  "The shard number to allow for horizontal scaling of networks. Defaults to zero."
+  shard: Long! = 0 @Min(value: 0)
+}


I'd go with the complex type. I appreciate the 0 defaults and the verbosity but I think that's something that can be managed at the client level

Nana-EC · 2023-07-06T00:59:23Z

HIP/HIP-668.md

+}
+
+"An identifier that represents a well-known or always changing block in the network."
+enum BlockType {


Block types also have the following labels

pending - Not applicable to Hedera due to our consensus but it's often used by devs and maps to the latest

safe - ... more details in link below

finalized - ... more details in link below

More info here https://github.com/ethereum/execution-apis/blob/main/src/schemas/block.yaml#L100
I think Safe and Finalized may also be treated like

Nana-EC · 2023-07-06T01:08:19Z

HIP/HIP-668.md

+  key: Object
+
+  "The memo associated with the entity."
+  memo: String!


With HIP 729 we should add nonce to this.

Add a Mirror Node GraphQL API

b17d884

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

steven-sheehy requested review from jnels124, xin-hedera, MarcKriguerAtHedera and edwin-greene February 1, 2023 04:50

steven-sheehy self-assigned this Feb 1, 2023

steven-sheehy commented Feb 1, 2023

View reviewed changes

shemnon reviewed Feb 1, 2023

View reviewed changes

This was referenced Feb 2, 2023

Create a GraphQL HIP hashgraph/hedera-mirror-node#4795

Closed

GraphQL Search for account by alias or address hashgraph/hedera-mirror-node#5318

Closed

GraphQL Account nested objects hashgraph/hedera-mirror-node#5319

Open

steven-sheehy commented Feb 2, 2023

View reviewed changes

steven-sheehy and others added 2 commits February 1, 2023 22:52

Fix for HIP validation

cdd6781

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

Add discussion link to hip-668

b57df86

Signed-off-by: Michael Garber <michael.garber@swirldslabs.com>

MarcKriguerAtHedera previously approved these changes Feb 10, 2023

View reviewed changes

Address some feedback

658b4c8

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

steven-sheehy dismissed MarcKriguerAtHedera’s stale review via 658b4c8 March 22, 2023 22:49

Add EVM address

12d7093

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

Change transaction query to return a list

0ed79d7

Signed-off-by: Steven Sheehy <steven.sheehy@swirldslabs.com>

Update HIP-668.md updated date

c97d3db

Signed-off-by: Michael Garber <michael.garber@swirldslabs.com>

Nana-EC reviewed Jul 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Mirror Node GraphQL API #668

Add a Mirror Node GraphQL API #668

steven-sheehy commented Feb 1, 2023

netlify bot commented Feb 1, 2023 •

edited

Loading

steven-sheehy Feb 1, 2023

shemnon Feb 1, 2023

steven-sheehy Feb 1, 2023

steven-sheehy Feb 2, 2023

mgarbs Feb 3, 2023

Nana-EC Jul 6, 2023

AlfredoG87 commented Feb 7, 2023 •

edited

Loading

MarcKriguerAtHedera left a comment

MarcKriguerAtHedera Feb 2, 2023

steven-sheehy Mar 22, 2023 •

edited

Loading

MarcKriguerAtHedera Feb 2, 2023

steven-sheehy Mar 22, 2023

MarcKriguerAtHedera Feb 10, 2023

steven-sheehy Feb 10, 2023

ericleponner commented Feb 22, 2023

ericleponner commented Feb 22, 2023 •

edited

Loading

ericleponner commented Feb 22, 2023 •

edited

Loading

ericleponner commented Feb 22, 2023

steven-sheehy commented Mar 22, 2023

steven-sheehy commented Mar 22, 2023

steven-sheehy commented Mar 22, 2023

steven-sheehy commented Mar 23, 2023

steven-sheehy commented Mar 23, 2023

Nana-EC Jul 6, 2023

Nana-EC Jul 6, 2023

Nana-EC Jul 6, 2023

Nana-EC Jul 6, 2023

Nana-EC Jul 6, 2023

Nana-EC Jul 6, 2023

	nature of REST APIs, it suffers from problems inherit in REST itself including both under-fetching and over-fetching.
	nature of REST APIs, it suffers from problems inherent to REST itself including both under-fetching and over-fetching.

Add a Mirror Node GraphQL API #668

Are you sure you want to change the base?

Add a Mirror Node GraphQL API #668

Conversation

steven-sheehy commented Feb 1, 2023

netlify bot commented Feb 1, 2023 • edited Loading

✅ Deploy Preview for hedera-hips ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlfredoG87 commented Feb 7, 2023 • edited Loading

MarcKriguerAtHedera left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steven-sheehy Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericleponner commented Feb 22, 2023

ericleponner commented Feb 22, 2023 • edited Loading

ericleponner commented Feb 22, 2023 • edited Loading

ericleponner commented Feb 22, 2023

steven-sheehy commented Mar 22, 2023

steven-sheehy commented Mar 22, 2023

steven-sheehy commented Mar 22, 2023

steven-sheehy commented Mar 23, 2023

steven-sheehy commented Mar 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Feb 1, 2023 •

edited

Loading

AlfredoG87 commented Feb 7, 2023 •

edited

Loading

steven-sheehy Mar 22, 2023 •

edited

Loading

ericleponner commented Feb 22, 2023 •

edited

Loading

ericleponner commented Feb 22, 2023 •

edited

Loading