Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

refactor(experimental): revise accounts schema #1817

Merged

Conversation

buffalojoec
Copy link
Contributor

This PR introduces a revised version of the account schema for jsonParsed
account data.

Typically we see JSON-parsed account data like this:

{
  data: {
    parsed: {
      info: {
        /* data */
      },
      type: 'mint',
    },
    program: 'Tokenkegxxxxxxxx',
    space: 82n,
  },
}

However, when it comes to GraphQL, I've made an opinionated decision to move
away from the JSON-parsed format returned by the RPC, and instead opt for this
"optimized" format:

{
  data: {
    /* data */
  },
  meta: {
    program: 'Tokenkegxxxxxxxx',
    space: 82n,
    type: 'mint',
  },
}

Since we know we'll always get program, space, and type, we can extract
those to be "parsed metadata" about an account and introduce the meta field.

@mergify mergify bot added the community label Nov 9, 2023
@mergify mergify bot requested a review from a team November 9, 2023 18:01
@buffalojoec buffalojoec force-pushed the 11-09-refactor_experimental_revise_accounts_schema branch from 60a6178 to e36eb26 Compare November 9, 2023 19:01
@buffalojoec buffalojoec force-pushed the 11-09-refactor_experimental_revise_accounts_schema branch from e36eb26 to e443604 Compare November 10, 2023 13:33
Copy link
Contributor

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

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

Let's go further! Free yourself completely from the JSON-RPC responses, and imagine building a UI that displays a mint account. For sure you would want this:

query {
  account(address: "abc") {
    # Stuff that every account has...
    address
    owner
    space
    ... on MintAccount {
      decimals
      isInitialized
      mintAuthority
      supply
    }
  }
}

No need for data, meta, or type fields.

Then in JS:

const account = ...; // Fetch the account
if (account.__type === 'MintAccount') {
  // TypeScript has now refined `account` to have the `MintAccount` fields
  console.table({
    decimals: account.decimals,
    supply: account.supply,
  });
}

The only thing that has to happen to make this work is that the fields have to be unique among all the types. You can't have a MintAccount that returns a supply that's an integer and a FooAccount that returns supply that's an object. In practice I haven't found this to be very difficult to avoid.

tl;dr collapse the account schema as flat as is practical and see what we end up with.

Comment on lines +970 to +971
mint {
... on MintAccount {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this always of the MintAccount type? Shouldn't be any need to type the subselection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's always a mint account, and I could stick the MintAccount type in there (which would be pretty cool), but that eliminates the potential for someone to get the mint account as base58, etc. It would always be the parsed MintAccount.

I'm leaning towards it though, because having to specify the spread here stinks.

Copy link
Contributor

Choose a reason for hiding this comment

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

One way we can do that is to let fields take arguments:

source {
  mint(encoding: BASE_64) {
    data # Base64String
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

My idea here is better.

slot
}
}
meta {
program
Copy link
Contributor

Choose a reason for hiding this comment

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

What should we call this for real in this greenfield schema?

  • Assigned Program id
  • Owner
  • Owning program

https://docs.solana.com/developing/programming-model/accounts#ownership-and-assignment-to-programs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ownerProgram I think.

timestamp: expect.any(BigInt),
},
node: {
authorizedVoters: expect.arrayContaining([
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to talk about whether to make things like this plural fields or connections to prepare for a future where it's possible to paginate through them.

https://relay.dev/graphql/connections.htm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pagination has been discussed for RPC v2, yeah? Probably good to do.

@buffalojoec
Copy link
Contributor Author

Let's go further! Free yourself completely from the JSON-RPC responses, and imagine building a UI that displays a mint account.

Yeah, I like this. The primary reason I went with data and meta was to isolate the field names, so any parsed accounts could be named whatever, without worry of collision.

The only thing that has to happen to make this work is that the fields have to be unique among all the types. You can't have a MintAccount that returns a supply that's an integer and a FooAccount that returns supply that's an object. In practice I haven't found this to be very difficult to avoid.

Right, so we could do this with the JSON-parsed data no problem. It's all known parsed structures and we can rename certain fields however we want to make this work. However, I'm thinking about a future where I can plug my IDL or Kinobi AST into this API and get parsing of my program's stuff!

This might complicate that.

@buffalojoec
Copy link
Contributor Author

Merging this stack for now so we can get the new schema layout in #1819, which will allow us to much more easily discuss changes! I also won't have to make the changes twice.

Created #1831 for tracking, for now.

@buffalojoec buffalojoec merged commit e6fc977 into master Nov 11, 2023
7 checks passed
@buffalojoec buffalojoec deleted the 11-09-refactor_experimental_revise_accounts_schema branch November 11, 2023 20:22
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants