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

feature: add new token program feature #18780

Merged
merged 7 commits into from Jul 21, 2021

Conversation

joncinque
Copy link
Contributor

Problem

We need to deploy a new token program.

Summary of Changes

Add a feature for it! Since we already have a new token program id in the code, and that one was enabled at slot 66528004, and we're at 87877768 as of this writing, it felt ok to change this up.

On the other hand, I left the old feature in the feature set so it can still be queried. Let me know if you prefer to remove that too.

After, this will reuse the hack to the account db to force in the new token program for one of our validators: #15212 (comment) <--- this is more for me to remember

Fixes #

mvines
mvines previously approved these changes Jul 20, 2021
Copy link
Member

@mvines mvines left a comment

Choose a reason for hiding this comment

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

Removing/reusing the existing token upgrade infra makes sense to me

@mergify mergify bot dismissed mvines’s stale review July 20, 2021 19:45

Pull request has been modified.

@mvines
Copy link
Member

mvines commented Jul 20, 2021

@joncinque - when we have the final SPL Token binary, let's make a SPL Token release over in SPL-land as well.

Then replace program-test/src/programs/spl_token-3.1.0.so and update fetch-spl.sh. These changes probably belong in this PR as well actually

@CriesofCarrots
Copy link
Contributor

let's make a SPL Token release over in SPL-land as well

Yes, please! We need that for some ledger-app-solana changes I have cued. I'll probably also bump spl-token in solana-transaction-status to parse the new instruction.

@joncinque
Copy link
Contributor Author

Ah yes, great call! Should this next release be version 3.2.0 then?

@codecov
Copy link

codecov bot commented Jul 20, 2021

Codecov Report

Merging #18780 (60b133d) into master (b13caa1) will decrease coverage by 0.0%.
The diff coverage is 90.0%.

@@            Coverage Diff            @@
##           master   #18780     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         441      441             
  Lines      124988   125017     +29     
=========================================
+ Hits       103578   103586      +8     
- Misses      21410    21431     +21     

@joncinque
Copy link
Contributor Author

@CriesofCarrots I added the transaction parser with the upgrade, could you take a look at it?

Otherwise, just double-checking, is there a potential problem with backporting this to 1.6? spl-token v3.2.0 uses solana-program v1.7.4

@CriesofCarrots
Copy link
Contributor

CriesofCarrots commented Jul 20, 2021

@CriesofCarrots I added the transaction parser with the upgrade, could you take a look at it?

Thank you! 😍 Looks great.

Otherwise, just double-checking, is there a potential problem with backporting this to 1.6? spl-token v3.2.0 uses solana-program v1.7.4

Oof. Correct, this likely won't be backportable to v1.6 for the reason you noted. I guess this would need to head to MNB on the v1.7 train. (Rolling back SPL to v1.6 doesn't seem very feasible to me.)

@joncinque joncinque removed the v1.6 label Jul 20, 2021
@joncinque
Copy link
Contributor Author

Oof. Correct, this won't be backportable to v1.6 for the reason you noted.

Ah, beans. The program will soak on mainnet for a bit, so let's hope lines up with the timeline for 1.7.

@CriesofCarrots
Copy link
Contributor

@joncinque , actually... while I believe I've had troubles in the past with spl dependencies bringing in newer minor-version solana-program crates, in this case it looks like if we release a new version of spl-associated-token-account and bump that as well, the backport will work. v1.6 builds with that setup, anyway. Will have to check to ensure there are no crates.io packaging issues.

@CriesofCarrots
Copy link
Contributor

No harm in releasing a new solana-associated-token-account on spl-token v3.2 anyway, so let's give it a try!

@joncinque
Copy link
Contributor Author

All right, let's roll the dice!

@joncinque joncinque added the v1.6 label Jul 20, 2021
@joncinque
Copy link
Contributor Author

That should be everything now 🤞

@CriesofCarrots
Copy link
Contributor

CriesofCarrots commented Jul 20, 2021

@joncinque , alas, it does build fine with the new atoken, but doesn't please cargo package:

$ cargo package
   Packaging solana-account-decoder v1.6.19 (/solana/account-decoder)
   Verifying solana-account-decoder v1.6.19 (/solana/account-decoder)
    Updating crates.io index
    
error: failed to verify package tarball

Caused by:
  failed to select a version for `solana-program`.
      ... required by package `spl-token v3.2.0`
      ... which is depended on by `solana-account-decoder v1.6.19 (/solana/target/package/solana-account-decoder-1.6.19)`
  versions that meet the requirements `^1.7.4` are: 1.7.6, 1.7.5, 1.7.4

  all possible versions conflict with previously selected packages.

    previously selected package `solana-program v1.6.19`
      ... which is depended on by `solana-sdk v1.6.19`
      ... which is depended on by `solana-account-decoder v1.6.19 (/solana/target/package/solana-account-decoder-1.6.19)`

  failed to select a version for `solana-program` which could resolve this conflict

Sorry for the hassle

@joncinque joncinque removed the v1.6 label Jul 20, 2021
@joncinque
Copy link
Contributor Author

Ah bummer, oh well, it was worth a shot!

@joncinque joncinque merged commit 51f3b9a into solana-labs:master Jul 21, 2021
@joncinque
Copy link
Contributor Author

Merging on not-really-red, the removal of the v1.6 label causes this to look failed, when it isn't

mergify bot pushed a commit that referenced this pull request Jul 21, 2021
* feature: add new token program feature

* Fixup test

* Update to spl-token v3.2.0

* Update Cargo.lock + fmt

* Update token program version in fetch-spl.sh

* Bump associated token program to 1.0.3

* Add aToken so

(cherry picked from commit 51f3b9a)

# Conflicts:
#	Cargo.lock
#	account-decoder/Cargo.toml
#	accounts-cluster-bench/Cargo.toml
#	programs/bpf/Cargo.lock
#	rpc/Cargo.toml
#	sdk/src/feature_set.rs
#	tokens/Cargo.toml
#	transaction-status/Cargo.toml
@joncinque joncinque deleted the token-vx-feature branch July 21, 2021 12:55
mergify bot added a commit that referenced this pull request Jul 21, 2021
* feature: add new token program feature (#18780)

* feature: add new token program feature

* Fixup test

* Update to spl-token v3.2.0

* Update Cargo.lock + fmt

* Update token program version in fetch-spl.sh

* Bump associated token program to 1.0.3

* Add aToken so

(cherry picked from commit 51f3b9a)

# Conflicts:
#	Cargo.lock
#	account-decoder/Cargo.toml
#	accounts-cluster-bench/Cargo.toml
#	programs/bpf/Cargo.lock
#	rpc/Cargo.toml
#	sdk/src/feature_set.rs
#	tokens/Cargo.toml
#	transaction-status/Cargo.toml

* Fix merge issues

* Re-generate lock files

Co-authored-by: Jon Cinque <jon.cinque@gmail.com>
joncinque added a commit that referenced this pull request Jul 21, 2021
)

* feature: add new token program feature

* Fixup test

* Cargo fmt

* Add back whitespace for cargo fmt

* Revert file totally
This was referenced Aug 23, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants