refactor(experimental-graphql): revise accounts schema (again)

[buffalojoec]

This PR attempts to revise the accounts schema, as per #1831.

[buffalojoec]

Current dependencies on/for this PR:

• master
  • PR refactor(experimental-graphql): roll some new type aliases #1848
    • PR refactor(experimental-graphql): revise accounts schema (again) #1837 👈
      • PR refactor(experimental-graphql): revise transaction instructions schema (again) #1841
        • PR refactor(experimental-graphql): add simulate transaction query #1852
          • PR refactor(experimental-graphql): introduce custom parsing extension #1858

This stack of pull requests is managed by Graphite.

[buffalojoec]

We're still going to want to sort out this comment here from the previous PR. I'm not sure if blocking the ability to specify encoding for these nested accounts is the way.

[steveluscher]

Tangent; this is hella deprecated right? Should we do something else here?

[buffalojoec]

Hah, I didn't check if it was deprecated 😅

Something different, like, take it out completely?

[steveluscher]

Take it out if there's no non-deprecated way to get at it, or flatten it down to the same level as blockhash and give it a good name.

[buffalojoec]

Actually I've been digging around and I don't see any evidence this is deprecated. Do you have any links? Also threw a flare up in Discord.

[steveluscher]

[steveluscher]

Returning this to your queue for some questions!

[buffalojoec]

Returning this to your queue for some questions!

OK, I pushed up a revised version! Lmk what you think.

[steveluscher]

I would really eliminate encoding from the account field entirely, and bury all that complexity in the resolvers. Either way, you could rip out encoding in a separate PR.

Let me know if you want to do a call to talk about how this would actually be implemented.

[steveluscher]

So, here's how I think about this.

People should just straight up not be bothered with these fetch modes. It should just never surface in the GraphQL API.

I should absolutely not give a shit about specifying an encoding, I should just do this.

account(address: "123") {
  ... on MintAccount {
    supply
  }
}

…and if that requires that we ask the RPC to parse the data using the jsonParsed mode, so be it.

To be pedantic, if we detect the presence of subselections in the query that we can't possibly fulfil except by fetching the thing in jsonParsed mode, then we should fetch the account in jsonParsed mode. Otherwise, we shouldn't.

If I really want the underlying buffer, then we can offer this:

account(address: "123") {
  data(encoding: BASE64)
}

Then we fetch it with the base64 encoding.

What if someone does this totally legal query?

account(address: "123") {
  compressedBase64Data: data(encoding: BASE64_ZSTD)
  base64Data: data(encoding: BASE64)
  ... on MintAccount {
    supply
  }
}

Yep. We have to make that underlying fetch three times with the current JSON RPC API.

Except when we rewrite the RPC, we can make this more efficient, and get all of the data in one shot.

[buffalojoec]

Nice, I like this. jsonParsed is already the default here, so you don't need to specify any encodings unless you want a buffer, but this is a much better way to handle the encoded stuff. Nice.

Will definitely roll this into a new PR.

[steveluscher]

Take it out if there's no non-deprecated way to get at it, or flatten it down to the same level as blockhash and give it a good name.

[steveluscher]

Much preferable to do data(encoding: AccountEncoding).

[buffalojoec]

Merge activity

• Nov 28, 8:18 AM: @buffalojoec started a stack merge that includes this pull request via Graphite.
• Nov 28, 8:18 AM: @buffalojoec merged this pull request with Graphite.

[github-actions]

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.