-
Notifications
You must be signed in to change notification settings - Fork 530
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
chore: upgrade spl-token package #299
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a994011
to
3567b41
Compare
- we had linked it so far to adapt API - core is using this library only for `TokenAccount` wrapper - TokenAccount wrapper is only used by fixed-price at this point and by token-metadata but only until the SDK is revamped (which is WIP)
3567b41
to
adc56c8
Compare
- removed one obsolete query method which used a `mint` property which isn't exposed by the `mpl-core` Account class anymore
aaf1c42
to
d4ca326
Compare
- executables need to be a dev dep for the package using it - picking up from a workspace root does not work, instead it was picking up the `tsc` executable that was installed globally on the developer machine
3b84211
to
12ece8c
Compare
austbot
approved these changes
Mar 11, 2022
thlorenz
added a commit
that referenced
this pull request
Mar 11, 2022
* master: Update Cargo.toml Take more bytes from slot for entropy (#300) chore: upgrade spl-token package (#299) Auction house docs (#296) Fix order in extra accounts comment to match (#298) chore(CODEOWNERS): remove aaron from code owners (#292) Adds another account for collection update authority when authuser is not minting (#290)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Upgraded all packages except
fixed-price-sale
go the latestspl-token
version.This latest version introduced breaking API changes and the code using it was adapted.
The only package using actual methods of this package in production SDK code is
token-vault
,all others are only using them as part of test setups or to reference the
TOKEN_ID
.The new API exposes more instructions instead of transactions which is essential if we want to
expose an SDK that allows users to pick and choose which they need to include and build cost
efficient transactions.
Working on token-metadata triggered me to pull this update forward as I realized that I need
not only the instruction methods there, but also lots of other helper methods regarding mints
and tokens that were included with that API.
As outlined in the issue referenced below I did attempt to upgrade this for
fixed-price-sale
as well but ran into too many issues with failing tests which point to timing issues which I
was unable to debug in the amount of time I wanted to spend on this update.
Core Update
The mpl core package also references spl-token in the code but was missing it as a dependency.
It is only using it for its
TokenAccount
wrapper.This wrapper is only used in
fixed-price
in thevalidateMembershipToken
which is exclusively used in its tests. Thus I expect this to be updated and the use become obsolete oncefixed-price-sale
also updates spl-token.The wrapper is also used inside
token-metadata
, but a revamp of the SDK of that package is already in the works and thus this use will also become obsolete.This core
TokenAccount
is based on an spl-coreAccountInfo
that as such doesn't exist in the new version (instead a similar classAccount
exists. Thus it is very difficult to easily update this use.For those reasons I decided to remove
TokenAccount
from core with the intent to publish a new core version once those mentioned packages don't use this functionality anymore. Still that update should be a minor version bump.NOTE: since core was published missing the
spl-token
dep in the package.json which didn't cause errors while all parent packages used the same version, I had to publish a patch@metaplex-foundation/mpl-core@0.0.5
and upgrade all packages to use this version in order to get CI to pass for this PR. This required to remove one obsolete query method intoken-metadata
since it required a now obsoleteAccount
property to work. The removed method wasn't used anywhere in our packages.As a side note
mpl-core
dep was missing fromfixed-price-sale
package as well which I fixed.Metaplex Fix
The metaplex package was depending on token-vault types + consts that were removed in the meantime. This caused CI to fail and was fixed by adding both to metaplex proper (referring to consts exposed by the core package).
TypeScript Dependency Fix
typescript
dev dependencytsc
command installed globally on the respective machine instead of the one we meant it to use (local to the package or at least the repo)References
fixed-price-sale
packageTesting
Where needed tests and supporting functions were updated to use the new API and I ensured that
they all pass after the change.
Next Steps
In order to have consistent dep versions across the repo the issue mentioned above should be
addressed ASAP.