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

Identity com bugfix/robust fetch nullable #2301

Merged

Conversation

Henry-E
Copy link

@Henry-E Henry-E commented Dec 5, 2022

#2174

Had to create a new PR because the original came from a company account which couldn't accept changes from maintainers.

@vercel
Copy link

vercel bot commented Dec 5, 2022

Someone is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

henrye added 2 commits December 5, 2022 18:18
…:Henry-E/anchor into Henry-E-identity-com-bugfix/robust-fetchNullable
@Henry-E Henry-E force-pushed the identity-com-bugfix/robust-fetchNullable branch from 2d49524 to 8ab5236 Compare December 6, 2022 21:31
@stegaBOB
Copy link
Contributor

stegaBOB commented Dec 6, 2022

Are there any cases where this wouldn't work? Could this be improved by checking the owner to check if its owned by the system program?

@Henry-E
Copy link
Author

Henry-E commented Dec 6, 2022

To be honest I'm not even sure what fetchNullable is for. But it seems unlikely that this would just be useful for system program accounts maybe? There could be program owned accounts that just store solana and no data. Though again, I'm not really very familiar with this function.

@rado0x54 might be able to comment on what the use cases are that they've been using it for?

@Henry-E
Copy link
Author

Henry-E commented Dec 7, 2022

Ok, given that the definition of fetchNullable is Returns a deserialized account, returning null if it doesn't exist. it seems reasonable that it would also return null if the account has no data on it? Since the whole point of fetch is to return the deserialized account data. So I think this is probably an ok expansion or "fix" to the behaviour of fetchNullable, since I think right now the function will just outright error if it tries to decode the account's data when it has lamports but no data. So it's actually preventing the function from erroring and making it more robust. Anyone who disagrees is free to voice their opinion and we can look into undoing the change later.

@Henry-E Henry-E merged commit 09b829d into coral-xyz:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants