Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

account info type missing typed fields and config values #3569

Open
nickfrosty opened this issue Nov 14, 2024 · 12 comments
Open

account info type missing typed fields and config values #3569

nickfrosty opened this issue Nov 14, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@nickfrosty
Copy link

nickfrosty commented Nov 14, 2024

Overview

the rpc.getAccountInfo() function is:

  • not showing the encoding field in the config (see image 1)
  • missing the space field in the returned value (see image 2)

image 1
image 2

Steps to reproduce

use the getAccountInfo rpc method:

const rpc = createSolanaRpc("https://api.devnet.solana.com");
let { value: accountInfo } = await rpc
  .getAccountInfo(address("nick6zJc6HpW3kfBm4xS2dmbuVRyb5F3AnUvj5ymzR5"), {
    // encoding does not appear
  })
  .send();
// shows a type error due to not finding `space`
console.log("accountInfo:", accountInfo.space);

Description of bug

the rpc.getAccountInfo() function should accept all the config settings for the getAccountInfo rpc method

and the response value should be typed to all response fields

@nickfrosty nickfrosty added the bug Something isn't working label Nov 14, 2024
@steveluscher
Copy link
Collaborator

Interesting.

This gives encoding in the autocomplete:

.getAccountInfo("nick6zJc6HpW3kfBm4xS2dmbuVRyb5F3AnUvj5ymzR5", {})

This does not:

.getAccountInfo(address("nick6zJc6HpW3kfBm4xS2dmbuVRyb5F3AnUvj5ymzR5"), {})

@steveluscher
Copy link
Collaborator

space is missing from AccountInfoBase

export type AccountInfoBase = Readonly<{
/** indicates if the account contains a program (and is strictly read-only) */
executable: boolean;
/** number of lamports assigned to this account */
lamports: Lamports;
/** pubkey of the program this account has been assigned to */
owner: Address;
/** the epoch at which this account will next owe rent */
rentEpoch: bigint;
}>;
cc/ @lorisleiva who last touched this.

@steveluscher
Copy link
Collaborator

These docs say size when presumably they mean space, @nickfrosty?

@lorisleiva
Copy link
Collaborator

@steveluscher Oh weird, I've checked everywhere and I can't find record of us ever using the size or space attribute on base account types. Doing a quick call to mainnet and I can see the attribute as space. Should I add it?

@steveluscher
Copy link
Collaborator

Yeah, so long as it’s actually there, everywhere we use AccountInfoBase, I guess it’s time, @lorisleiva!

@lorisleiva
Copy link
Collaborator

Okay I've just checked everywhere we use this AccountInfoBase in the code and compared each one with the docs. The conclusion is: they all return a space attribute of type u64 and some of them are mislabelled in the docs (not in the code example but in the written documentation on the left column). Here's the exhaustive list:

  • getAccountInfo method (mislabelled as size in the docs).
  • getMultipleAccounts method
  • getProgramAccounts method (mislabelled as size in the docs).
  • getTokenAccountsByDelegate method (mislabelled as size in the docs).
  • getTokenAccountsByOwner method (mislabelled as size in the docs).
  • accountSubscribe notifications
  • programSubscribe notifications

I'll now work on a PR that adds the space attribute to AccountInfoBase. Thank you for raising this!

@nickfrosty
Copy link
Author

These docs say size when presumably they mean space, @nickfrosty?

looks like it, I will get those docs updated today!

@nickfrosty
Copy link
Author

@lorisleiva I will get these updated in the docs. I have had it on the todo list to comb through all the rpc docs and validate the inputs and responses for too long now.
#3569 (comment) - I will get all these specific ones updated now :)

@steveluscher
Copy link
Collaborator

I spent some time tonight fighting with the TypeScript Intellisense problem (ie. that it won't cough up the encoding field). I think it comes down to the fact that encoding doesn't participate in every function overload so TypeScript doesn't consider it part of the minimum set of possible inputs. This makes no sense to me and I'm sure there's a TypeScript issue already filed about it.

Wouldn't it be weird if this bug from 2014 was back. microsoft/TypeScript#646

All in all, sounds like this: microsoft/TypeScript#44183

@nickfrosty
Copy link
Author

Why not just add encoding into the AccountInfoBase? since it is a default param for the rpc method, then the existing type narrowing should still work

@steveluscher
Copy link
Collaborator

AccountInfoBase is the return type, but yeah I tried something similar to what you suggest and added it to GetAccountInfoCommonConfig.

  type GetAccountInfoApiCommonConfig = Readonly<{
      // Defaults to `finalized`
      commitment?: Commitment;
+     encoding?: 'base58' | 'base64' | 'base64+zstd' | 'jsonParsed',
      // The minimum slot that the request can be evaluated at
      minContextSlot?: Slot;
  }>;

It didn't help.

@nickfrosty
Copy link
Author

hmm darn :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants