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

2.0.0 solana-program release causing issues due to <=2.0.0 on patch upgrades. #6897

Closed
jacobcreech opened this issue Jun 24, 2024 · 26 comments
Closed

Comments

@jacobcreech
Copy link
Contributor

Problem

solana-program upgraded to 2.0.0 on Friday, leading to a number of builds across the ecosystem because of the <=2.0.0 in a patch version upgrade on #6182.

The problematic dependencies are as follows:

spl-pod version 0.1.1 -> Previous version 0.1.0 did not have <=2.0.0. Upgrade on patch broke some builds
spl-token-metadata-interface version 0.2.1 -> Previous version 0.2.0 did not have <=2.0.0. Upgrade on patch broke some builds
spl-associated-token-account version 2.3.1 -> Previous version 2.3.0 did not have <=2.0.0.
spl-token version 4.0.1 -> Previous version 4.0.0 did not have <=2.0.0.
spl-token-group-interface version 0.1.1 -> Previous version 0.1.0 did not have <=2.0.0.

These are the ones I can find at this time. This affects anyone using anchor-spl right now, as well as some projects that don't use Anchor but pull in spl-token 4.0.1 while using solana-program 1.X.

Relevant Anchor issue: coral-xyz/anchor#3044

Suggested Solution

Purge the versions causing problems, re-release them without the <=2.0.0, upgrade and release with <=2.0.0 as a "major" version if not already available. As mentioned here

@joncinque
Copy link
Contributor

I ran into a lot of these issues while publishing the last spl-token-cli, so there are compatible major versions that already exist in spl:

  • spl-associated-token-account -- v3 exists, so we can yank 2.3.1
  • spl-token -- might need v5, then we can yank 4.0.1
  • spl-token-group-interface -- v0.2 exists, so we can yank 0.1.1
  • spl-token-metdata-interface -- v0.3 exists, so we can yank 0.2.1
  • spl-pod -- v0.2 exists, so we can yank 0.1.1
  • spl-token-2022 -- is ok

There are some more:

  • spl-program-error -- yank 0.3.1
  • spl-discriminator -- yank 0.1.1
  • spl-type-length-value -- yank v0.3.1
  • spl-tlv-account-resolution -- yank v0.5.2
  • spl-transfer-hook-interface -- yank v0.5.1

With that, I think everything will be ok -- if people use the newest versions, they'll still get the version that allows for both, so they'll need to be careful with dependencies, but at least the patch version won't automatically pick it up

@joncinque
Copy link
Contributor

But first, we'll need to update the monorepo to the correct versions

@joncinque
Copy link
Contributor

Thankfully, it looks like just spl-token needs an update 🥳

@joncinque
Copy link
Contributor

joncinque commented Jun 24, 2024

Bah, never mind, it unfortunately creates more downstream issues due to the version mismatches for the new spl-token major versions.

The real solution will be a lot like the proposed solution:

  • upgrade SPL to v2
  • publish all new major versions for SPL crates
  • get agave to use these new crates + backport
  • yank the old ones

@jacobcreech
Copy link
Contributor Author

Don't you want to republish the old versions as well without the <=2.0.0 to avoid breaking any build that uses them? Otherwise you'll break more than just the monorepo

@joncinque
Copy link
Contributor

Sorry, I don't think I understand -- shouldn't the existing pre-patch versions without the <= 2.0.0 be fine? Here's the proposal:

  • yank the patch versions with <= 2.0.0, people can use the old ones
  • keep new major version with <= 2.0.0 (all spl crates have one of these currently)
  • publish a new major version with only 2.0.0 support

@willhickey
Copy link
Contributor

willhickey commented Jun 24, 2024

Using spl-token as an example. The relevant published versions are:
v4.0.0 -> ^1.16.1
v4.0.1 -> >=1.17.17, <=2
v5.0.0 -> >=1.18.11, <=2

I think the correct remediation is:

  • create a new token-v4.0 branch just before the v5.0 version bump
    • on the v4.0 branch remove <=2 monorepo dependencies
    • publish v4.0.2
  • On master bump the version to 6.0.0 and remove >=1.18.11 from monorepo dependencies
  • Yank v5.0.0
  • Publish v6.0.0 from the updated master
  • Update monorepo master and v2.0 with spl-token = "6"
  • Publish new v2.0.x crates from the monorepo
  • yank v4.0.1

Which would give us this:
v4.0.0 -> ^1.16.1
v4.0.1 -> >=1.17.17, <=2 yanked
v4.0.2 -> >=1.17.17
v5.0.0 -> >=1.18.11, <=2
v6.0.0 -> 2

Hopefully we won't have to do much with the new branches (for all the old major versions) but without them we don't have a way to publish new patches to replace the yanked versions.

I think this is consistent with Jacob & Jon's proposals above. Any thoughts on this plan before I start yanking crates?

@joncinque
Copy link
Contributor

joncinque commented Jun 24, 2024

I'm almost onboard, but my main question: why do we need 4.0.2?

Also, we can't yank 4.0.1 until the monorepo is updated and backported since it requires =4.0.1. https://github.com/anza-xyz/agave/blob/69c0b0edb6fa5ac17c2e15ef19c9b0487d105352/Cargo.toml#L418

My plan is to leave a major version that allows both, like v5.0.0, but then have v6.0.0 which only allows 2

Edit: oh, and we won't be able to update the monorepo to 4.0.0, 4.0.2, or 5.0.0 😅 which is why I think we need to bite the bullet and upgrade all of the spl crates to v2 only and then update the monorepo to those

@willhickey
Copy link
Contributor

Oh yeah, good point about needing a new major to remove the monorepo dependency.

Maybe we don't need 4.0.2. My thinking was we don't really support v1.16 or v1.17 on public clusters anymore, so I don't want people using 4.0.0. But hopefully projects that use 4.0.0 don't have monorepo dependencies pinned and will pick up v1.18 by default. I guess if anyone is pinned to v1.16 that can be their problem.

I'll update my comment above shortly

@jacobcreech
Copy link
Contributor Author

Sorry, I don't think I understand -- shouldn't the existing pre-patch versions without the <= 2.0.0 be fine?

I was worried someone was pinning =4.0.1 or one of the other offending patch versions and then their build will start failing. Looking at crates.io dependents though, looks like we should be fine. :shipit:

@jacobcreech
Copy link
Contributor Author

Also, we can't yank 4.0.1 until the monorepo is updated and backported since it requires =4.0.1. https://github.com/anza-xyz/agave/blob/69c0b0edb6fa5ac17c2e15ef19c9b0487d105352/Cargo.toml#L418

If you were to republish without the <=2.0.0 after yanking, you wouldn't run into this issue. I'm worried some dependent of 4.0.1 will start to fail post our yanking other than the monorepo.

@willhickey
Copy link
Contributor

If you were to republish without the <=2.0.0 after yanking, you wouldn't run into this issue. I'm worried some dependent of 4.0.1 will start to fail post our yanking other than the monorepo.

We can publish new patch versions (eg 4.0.2) but I don't want to re-publish yanked versions with changes. Too much opportunity for confusion

@joncinque
Copy link
Contributor

Everything has been yanked except for spl-type-length-value v0.3.1 (waiting on cavemanloverboy/sol#11) and spl-token v4.0.1 (waiting on new release of Solana v2 crates)

@joncinque
Copy link
Contributor

I yanked spl-type-length-value v0.3.1, which has resolved some of the issues. For example, cargo install spl-stake-pool-cli now works for v1

@jacobcreech
Copy link
Contributor Author

What is left on this?

@joncinque
Copy link
Contributor

joncinque commented Jul 2, 2024

Once solana-program 2.0.2 is cut, I'll yank spl-token 4.0.1, then this will be done

@ifiokjr
Copy link

ifiokjr commented Jul 4, 2024

@joncinque I'm questioning why we aren't creating a patched [email protected] with the offending <=2 removed.

I'm almost onboard, but my main question: why do we need 4.0.2?

We need it because the versions of some dependencies in [email protected] are severely outdated (num-derive, num-traits and num_enum are all >3 years old).

This is probably the case for many of the other libraries which used <=2. Ideally, developers shouldn't be required to downgrade their dependency tree (or introduce duplication) in their stack as a result of this fix.

@jacobcreech
Copy link
Contributor Author

@joncinque if we do create a [email protected] patch with <=2 removed, this should fix spl-token issues within anchor projects + anyone using [email protected]. Could this be done before [email protected] is cut?

@joncinque
Copy link
Contributor

Oh that's a good call -- I'll republish 4.0.0 as 4.0.2 to be totally safe.

@joncinque
Copy link
Contributor

Just to make sure -- how does that sound @jacobcreech ?

@acheroncrypto
Copy link
Contributor

We need patch releases with only Solana v1 dependence for all SPL crates that #6182 changed to fully fix this issue.

@joncinque
Copy link
Contributor

We need patch releases with only Solana v1 dependence for all SPL crates that #6182 changed to fully fix this issue.

@acheroncrypto all of the ones allowing v2 have been yanked except for spl-token -- have you seen any other still causing issues?

@jacobcreech
Copy link
Contributor Author

Oh that's a good call -- I'll republish 4.0.0 as 4.0.2 to be totally safe.

I don't believe that will completely work. Looking at the diff between 4.0.1 and 4.0.0, there's a decent amount of versions in 4.0.0 that'll put us back a bit. The above comment states some of them.

Correct me if I'm wrong, but I believe the quickest path forward would be to revert #6182 on top of 4.0.1 and do a patch release of 4.0.2. That will fix the dependency issues we're seeing in the ecosystem while allowing you to still cut the required major upgrades for 2.X

@acheroncrypto
Copy link
Contributor

We need patch releases with only Solana v1 dependence for all SPL crates that #6182 changed to fully fix this issue.

@acheroncrypto all of the ones allowing v2 have been yanked except for spl-token -- have you seen any other still causing issues?

How so? When I look at the crates, pretty much all I see is that SPL allows both v1 and v2 in the latest versions before the v2 exclusive breaking bumps.

Here are some examples other than spl-token:

These versions were the latest versions before v2 got released. They are also the versions that are used by the latest anchor-spl because the latest Anchor was released before the v2 bump.

Yanking is also worse than making a patch release with #6182 reverted in this case, not only because of the outdated dependencies mentioned in #6897 (comment), but also because it won't even work for most of the SPL crates. For example, let's take spl-token-2022, it only has one version (3.0.2) in the v3 release line, so if it were to get yanked, there wouldn't be a SemVer compatible version that cargo can fallback to.

@acheroncrypto
Copy link
Contributor

This seems to have been fixed with the new patch releases, as Anchor CI succeeds now. Thanks!

@jacobcreech
Copy link
Contributor Author

Looks like with the patch releases done, this issue is now completed. Thanks all for all the work involved to get this over the line!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants