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

ci: Bump repo to Solana 1.17 #5575

Merged
merged 26 commits into from
Oct 19, 2023
Merged

ci: Bump repo to Solana 1.17 #5575

merged 26 commits into from
Oct 19, 2023

Conversation

joncinque
Copy link
Contributor

@joncinque joncinque commented Oct 17, 2023

Problem

The 1.17 crates are out, but SPL is still on 1.16.

Solution

Bump everything and fix things up as they arise!

@joncinque
Copy link
Contributor Author

This is ready to go! Most of the commits are pretty simple in the end, but all required to avoid deprecated functions / fix new clippy lints.

@joncinque joncinque marked this pull request as ready for review October 18, 2023 22:01
Comment on lines 58 to 59
#[error("WrongStakeStateV2")]
WrongStakeStateV2,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#[error("WrongStakeStateV2")]
WrongStakeStateV2,
#[error("WrongStakeState")]
WrongStakeState,

overzealous search/replace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, big time, thanks for noticing this and fixing it 🙏

SinglePoolError::WrongStakeState => msg!("Error: Stake account is not in the state expected by the program."),
SinglePoolError::WrongStakeStateV2 => msg!("Error: Stake account is not in the state expected by the program."),
Copy link
Member

Choose a reason for hiding this comment

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

also here (github suggestions doesnt let me fix this...)

Copy link
Member

Choose a reason for hiding this comment

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

actually ill just push my own commit, need to handle merge conflicts anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for fixing this!

Comment on lines 51 to 60
class StakeStateType(IntEnum):
class StakeStateV2Type(IntEnum):
"""Stake State Types."""
UNINITIALIZED = 0
INITIALIZED = 1
STAKE = 2
REWARDS_POOL = 3


class StakeState(NamedTuple):
state_type: StakeStateType
class StakeStateV2(NamedTuple):
state_type: StakeStateV2Type
Copy link
Member

Choose a reason for hiding this comment

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

assuming this is also accidental because nothing about the definitions has changed at all...

Copy link
Member

Choose a reason for hiding this comment

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

ok yes im sure about this because flags are not added to STAKE_STATE_LAYOUT/STAKE_AND_META_LAYOUT nor were they renamed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this is also incorrect

@2501babe
Copy link
Member

2501babe commented Oct 19, 2023

i rebased on master, the diff on token/cli/src/main.rs is spurious (because it shows the full commit that was in master), the only change to fix merge was this went one way and had to go back the other way (or something):

-                format!("  auditor encryption pubkey set to {}", new_auditor_pubkey,),
+                format!(
+                    "  auditor encryption pubkey set to {}",
+                    new_auditor_pubkey,
+                ),

to wit:

hana@laptop:cli$ git diff master..HEAD src/main.rs 
diff --git a/token/cli/src/main.rs b/token/cli/src/main.rs
index cbd89e16..58dbefdb 100644
--- a/token/cli/src/main.rs
+++ b/token/cli/src/main.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::integer_arithmetic)]
+#![allow(clippy::arithmetic_side_effects)]
 use clap::{
     crate_description, crate_name, crate_version, value_t, value_t_or_exit, App, AppSettings, Arg,
     ArgGroup, ArgMatches, SubCommand,
@@ -63,7 +63,9 @@ use spl_token_client::{
     token::{ExtensionInitializationParams, Token},
 };
 use spl_token_metadata_interface::state::{Field, TokenMetadata};
-use std::{collections::HashMap, fmt, fmt::Display, process::exit, str::FromStr, sync::Arc};
+use std::{
+    collections::HashMap, fmt, fmt::Display, process::exit, rc::Rc, str::FromStr, sync::Arc,
+};
 use strum::IntoEnumIterator;
 use strum_macros::{EnumIter, EnumString, IntoStaticStr};
 
@@ -410,7 +412,7 @@ fn new_throwaway_signer() -> (Arc<dyn Signer>, Pubkey) {
 fn get_signer(
     matches: &ArgMatches<'_>,
     keypair_name: &str,
-    wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
+    wallet_manager: &mut Option<Rc<RemoteWalletManager>>,
 ) -> Option<(Arc<dyn Signer>, Pubkey)> {
     matches.value_of(keypair_name).map(|path| {
         let signer = signer_from_path(matches, path, keypair_name, wallet_manager)
@@ -464,7 +466,7 @@ type SignersOf = Vec<(Arc<dyn Signer>, Pubkey)>;
 pub fn signers_of(
     matches: &ArgMatches<'_>,
     name: &str,
-    wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
+    wallet_manager: &mut Option<Rc<RemoteWalletManager>>,
 ) -> Result<Option<SignersOf>, Box<dyn std::error::Error>> {
     if let Some(values) = matches.values_of(name) {
         let mut results = Vec::new();
@@ -4852,7 +4854,7 @@ async fn process_command<'a>(
     sub_command: &CommandName,
     sub_matches: &ArgMatches<'_>,
     config: &Config<'a>,
-    mut wallet_manager: Option<Arc<RemoteWalletManager>>,
+    mut wallet_manager: Option<Rc<RemoteWalletManager>>,
     mut bulk_signers: Vec<Arc<dyn Signer>>,
 ) -> CommandResult {
     match (sub_command, sub_matches) {

@joncinque joncinque merged commit c32015e into solana-labs:master Oct 19, 2023
43 checks passed
@joncinque joncinque deleted the bump1.17 branch October 19, 2023 16:04
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.

2 participants