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

ts: Remove SPL coders from Anchor package #2155

Merged
merged 10 commits into from Nov 17, 2022

Conversation

acheroncrypto
Copy link
Collaborator

Follow up on #2143, coders in Anchor package is no longer necessary. This is a breaking change, codebases that rely on SPL from Anchor package would need to migrate to the new packages and only import as needed.

@vercel
Copy link

vercel bot commented Aug 27, 2022

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

A member of the Team first needs to authorize it.

if (accountInfo === null) {
throw new Error(`invalid account info for ${address}`);
}
const data = coder().accounts.decode("token", accountInfo.data);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could just decode token accounts without any coder, not sure if it's needed though.

Copy link
Member

Choose a reason for hiding this comment

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

if this is going to be removed, would be worth also investigating where and why TokenAccounts are being added to the account store cache and managing that too.

Copy link
Member

Choose a reason for hiding this comment

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

maybe @armaniferrante has insight there

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Completely removing this caused a problem that I don't remember so I added a coder for only what we need and nothing more in 57f0f7e

@acheroncrypto acheroncrypto marked this pull request as ready for review August 31, 2022 18:55
@Henry-E
Copy link
Contributor

Henry-E commented Nov 16, 2022

Not if sure @tomlinton or @callensm is best to check this out

@callensm
Copy link
Member

it's my understanding that the spl coders here are mildly broken anyway, so i wouldn't care if they were removed in favor of the official spl packages that have their own encoding/decoding, but would almost certain be a breaking change for many.

@Henry-E
Copy link
Contributor

Henry-E commented Nov 16, 2022

Cool, thanks for that. A breaking change doesn't sound great for sure. Might be a close then unfortunately.

@callensm
Copy link
Member

Cool, thanks for that. A breaking change doesn't sound great for sure. Might be a close then unfortunately.

i'd still consider merging once we get more input of the usability of these coders. if the impact is minimal might be worth getting rid of the bloat /shrug

@acheroncrypto
Copy link
Collaborator Author

Cool, thanks for that. A breaking change doesn't sound great for sure. Might be a close then unfortunately.

We had a talk about this with @armaniferrante and his input was to completely remove them and allow people to import as needed.

@callensm
Copy link
Member

callensm commented Nov 16, 2022

ok, just verified. we're good to merge this once conflicts are resolved.

I was misunderstanding what was actually being proposed for removal.

@Henry-E
Copy link
Contributor

Henry-E commented Nov 16, 2022

Great, @acheroncrypto if you wouldn't mind to fix the merge conflicts when you get a chance and will try merge ASAP!

@acheroncrypto
Copy link
Collaborator Author

Fixed the conflicts, only zero-copy test seems to fail and it's not related to this PR.

@Henry-E
Copy link
Contributor

Henry-E commented Nov 17, 2022

@acheroncrypto are you happy with merging or any last bits you want to add?

Ha, you posted literal seconds before I did. Will merge so

@Henry-E Henry-E merged commit af11599 into coral-xyz:master Nov 17, 2022
Henry-E pushed a commit to Henry-E/anchor that referenced this pull request Dec 6, 2022
* ts: Remove SPL coders from Anchor package

* Replace old SPL custom coder tests

* Build and link new SPL packages before tests and decode token account for AccountStore

* Fix Github actions ts setup

* Fix u64 buffer length

* Update submodules

* Update CHANGELOG

* Checkout correct submodule commits

* Add filler program to generate idls in order for anchor test to pass
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.

None yet

3 participants