From d5844633b194bed5f53f4f1d1ec650370cd0e279 Mon Sep 17 00:00:00 2001 From: Jon C Date: Fri, 25 Oct 2024 22:53:33 +0200 Subject: [PATCH] CI: Update clippy to use `--all-features`, fix issues (#7395) * CI: Update clippy to use `--all-features`, fix issues #### Problem Clippy isn't run with `--all-features`, and there are some pieces that are broken with all features enabled. #### Summary of changes Add `--all-features` to clippy steps, and fix existing issues. * Fixup tests --- .github/workflows/publish-rust.yml | 2 +- .github/workflows/pull-request.yml | 2 +- libraries/discriminator/src/lib.rs | 6 ++--- libraries/pod/src/optional_keys.rs | 2 +- token-swap/program/src/constraints.rs | 8 +++---- token-swap/program/src/processor.rs | 29 ++++++++++++----------- token/program-2022/src/serialization.rs | 9 +------ token/program-2022/tests/serialization.rs | 25 +++++++------------ 8 files changed, 35 insertions(+), 48 deletions(-) diff --git a/.github/workflows/publish-rust.yml b/.github/workflows/publish-rust.yml index c95fc5911f0..ead8767e9bd 100644 --- a/.github/workflows/publish-rust.yml +++ b/.github/workflows/publish-rust.yml @@ -84,7 +84,7 @@ jobs: run: ./ci/install-build-deps.sh - name: Run clippy - run: ./cargo-nightly clippy -Zunstable-options --workspace --all-targets --features test-sbf -- --deny=warnings --deny=clippy::arithmetic_side_effects + run: ./cargo-nightly clippy -Zunstable-options --workspace --all-targets --all-features -- --deny=warnings --deny=clippy::arithmetic_side_effects cargo-build-test: runs-on: ubuntu-latest diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 0bf72e36d8a..4fa246c9820 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -71,7 +71,7 @@ jobs: run: ./ci/install-build-deps.sh - name: Run clippy - run: ./cargo-nightly clippy -Zunstable-options --workspace --all-targets --features test-sbf -- --deny=warnings --deny=clippy::arithmetic_side_effects + run: ./cargo-nightly clippy -Zunstable-options --workspace --all-targets --all-features -- --deny=warnings --deny=clippy::arithmetic_side_effects audit: runs-on: ubuntu-latest diff --git a/libraries/discriminator/src/lib.rs b/libraries/discriminator/src/lib.rs index c3ebbdaba43..5ec7b8ae322 100644 --- a/libraries/discriminator/src/lib.rs +++ b/libraries/discriminator/src/lib.rs @@ -130,15 +130,15 @@ mod tests { #[cfg(all(test, feature = "borsh"))] mod borsh_test { - use super::*; + use {super::*, borsh::BorshDeserialize}; #[test] fn borsh_test() { let my_discrim = ArrayDiscriminator::new_with_hash_input("my_discrim"); let mut buffer = [0u8; 8]; - my_discrim.serialize(&mut buffer[..]).unwrap(); + borsh::to_writer(&mut buffer[..], &my_discrim).unwrap(); let my_discrim_again = ArrayDiscriminator::try_from_slice(&buffer).unwrap(); assert_eq!(my_discrim, my_discrim_again); - assert_eq!(buf, my_discrim.into()); + assert_eq!(buffer, <[u8; 8]>::from(my_discrim)); } } diff --git a/libraries/pod/src/optional_keys.rs b/libraries/pod/src/optional_keys.rs index b31494d4ed7..1bfc28fd113 100644 --- a/libraries/pod/src/optional_keys.rs +++ b/libraries/pod/src/optional_keys.rs @@ -101,7 +101,7 @@ impl<'de> Visitor<'de> for OptionalNonZeroPubkeyVisitor { where E: Error, { - let pkey = Pubkey::from_str(&v) + let pkey = Pubkey::from_str(v) .map_err(|_| Error::invalid_value(Unexpected::Str(v), &"value string"))?; OptionalNonZeroPubkey::try_from(Some(pkey)) diff --git a/token-swap/program/src/constraints.rs b/token-swap/program/src/constraints.rs index 99e3d1b4564..66e9c3ec9b1 100644 --- a/token-swap/program/src/constraints.rs +++ b/token-swap/program/src/constraints.rs @@ -1,7 +1,7 @@ //! Various constraints as required for production environments #[cfg(feature = "production")] -use std::env; +use std::option_env; use { crate::{ curve::{ @@ -21,7 +21,7 @@ use { /// cannot be used, so we have to split the curves based on their types. pub struct SwapConstraints<'a> { /// Owner of the program - pub owner_key: &'a str, + pub owner_key: Option<&'a str>, /// Valid curve types pub valid_curve_types: &'a [CurveType], /// Valid fees @@ -61,7 +61,7 @@ impl<'a> SwapConstraints<'a> { } #[cfg(feature = "production")] -const OWNER_KEY: &str = env!("SWAP_PROGRAM_OWNER_FEE_ADDRESS"); +const OWNER_KEY: Option<&str> = option_env!("SWAP_PROGRAM_OWNER_FEE_ADDRESS"); #[cfg(feature = "production")] const FEES: &Fees = &Fees { trade_fee_numerator: 0, @@ -115,7 +115,7 @@ mod tests { let owner_withdraw_fee_denominator = 10; let host_fee_numerator = 10; let host_fee_denominator = 100; - let owner_key = ""; + let owner_key = Some(""); let curve_type = CurveType::ConstantProduct; let valid_fees = Fees { trade_fee_numerator, diff --git a/token-swap/program/src/processor.rs b/token-swap/program/src/processor.rs index 7fc63cb5a56..a596bd21884 100644 --- a/token-swap/program/src/processor.rs +++ b/token-swap/program/src/processor.rs @@ -338,6 +338,7 @@ impl Processor { if let Some(swap_constraints) = swap_constraints { let owner_key = swap_constraints .owner_key + .unwrap() .parse::() .map_err(|_| SwapError::InvalidOwner)?; if fee_account.owner != owner_key { @@ -3109,10 +3110,10 @@ mod tests { curve_type: CurveType::ConstantProduct, calculator: Arc::new(curve), }; - let owner_key = &new_key.to_string(); + let owner_key = new_key.to_string(); let valid_curve_types = &[CurveType::ConstantProduct]; let constraints = Some(SwapConstraints { - owner_key, + owner_key: Some(owner_key.as_ref()), valid_curve_types, fees: &fees, }); @@ -3182,10 +3183,10 @@ mod tests { curve_type: CurveType::ConstantProduct, calculator: Arc::new(curve), }; - let owner_key = &user_key.to_string(); + let owner_key = user_key.to_string(); let valid_curve_types = &[CurveType::ConstantProduct]; let constraints = Some(SwapConstraints { - owner_key, + owner_key: Some(owner_key.as_ref()), valid_curve_types, fees: &fees, }); @@ -3257,10 +3258,10 @@ mod tests { curve_type: CurveType::ConstantProduct, calculator: Arc::new(curve), }; - let owner_key = &user_key.to_string(); + let owner_key = user_key.to_string(); let valid_curve_types = &[CurveType::ConstantProduct]; let constraints = Some(SwapConstraints { - owner_key, + owner_key: Some(owner_key.as_ref()), valid_curve_types, fees: &fees, }); @@ -6481,10 +6482,10 @@ mod tests { calculator: Arc::new(curve), }; - let owner_key_str = &owner_key.to_string(); + let owner_key_str = owner_key.to_string(); let valid_curve_types = &[CurveType::ConstantProduct]; let constraints = Some(SwapConstraints { - owner_key: owner_key_str, + owner_key: Some(owner_key_str.as_ref()), valid_curve_types, fees: &fees, }); @@ -7184,7 +7185,7 @@ mod tests { _pool_key, _pool_account, ) = accounts.setup_token_accounts(&user_key, &authority_key, initial_a, initial_b, 0); - let owner_key = &swapper_key.to_string(); + let owner_key = swapper_key.to_string(); let fees = Fees { trade_fee_numerator, trade_fee_denominator, @@ -7196,7 +7197,7 @@ mod tests { host_fee_denominator, }; let constraints = Some(SwapConstraints { - owner_key, + owner_key: Some(owner_key.as_ref()), valid_curve_types: &[], fees: &fees, }); @@ -7264,7 +7265,7 @@ mod tests { _pool_key, _pool_account, ) = accounts.setup_token_accounts(&user_key, &authority_key, initial_a, initial_b, 0); - let owner_key = &swapper_key.to_string(); + let owner_key = swapper_key.to_string(); let fees = Fees { trade_fee_numerator, trade_fee_denominator, @@ -7276,7 +7277,7 @@ mod tests { host_fee_denominator, }; let constraints = Some(SwapConstraints { - owner_key, + owner_key: Some(owner_key.as_ref()), valid_curve_types: &[], fees: &fees, }); @@ -8188,9 +8189,9 @@ mod tests { calculator: Arc::new(ConstantProductCurve {}), }; - let owner_key_str = &owner_key.to_string(); + let owner_key_str = owner_key.to_string(); let constraints = Some(SwapConstraints { - owner_key: owner_key_str, + owner_key: Some(owner_key_str.as_ref()), valid_curve_types: &[CurveType::ConstantProduct], fees: &fees, }); diff --git a/token/program-2022/src/serialization.rs b/token/program-2022/src/serialization.rs index b246f1abc80..74d4f642adb 100644 --- a/token/program-2022/src/serialization.rs +++ b/token/program-2022/src/serialization.rs @@ -1,11 +1,6 @@ //! serialization module - contains helpers for serde types from other crates, //! deserialization visitors -use { - base64::{prelude::BASE64_STANDARD, Engine}, - serde::de::Error, -}; - /// helper function to ser/deser COption wrapped values pub mod coption_fromstr { use { @@ -77,9 +72,7 @@ pub mod coption_fromstr { D: Deserializer<'de>, T: FromStr, { - d.deserialize_option(COptionVisitor { - s: PhantomData::default(), - }) + d.deserialize_option(COptionVisitor { s: PhantomData }) } } diff --git a/token/program-2022/tests/serialization.rs b/token/program-2022/tests/serialization.rs index 1d8d3b5021f..66ad530810d 100644 --- a/token/program-2022/tests/serialization.rs +++ b/token/program-2022/tests/serialization.rs @@ -5,13 +5,7 @@ use { solana_program::program_option::COption, solana_sdk::pubkey::Pubkey, spl_pod::optional_keys::{OptionalNonZeroElGamalPubkey, OptionalNonZeroPubkey}, - spl_token_2022::{ - extension::confidential_transfer, - instruction, - solana_zk_sdk::encryption::pod::{ - auth_encryption::PodAeCiphertext, elgamal::PodElGamalPubkey, - }, - }, + spl_token_2022::{extension::confidential_transfer, instruction}, std::str::FromStr, }; @@ -70,7 +64,7 @@ fn serde_instruction_optional_nonzero_pubkeys_podbool() { }; let serialized = serde_json::to_string(&inst).unwrap(); - let serialized_expected = &format!("{{\"authority\":\"4uQeVj5tqViQh7yWWGStvkEG1Zmhx6uasJtWCJziofM\",\"autoApproveNewAccounts\":false,\"auditorElgamalPubkey\":\"ohdsJIKPEtvEhvKRszHlwUpAA55E63xY95Ck/uQMrVU=\"}}"); + let serialized_expected = &"{\"authority\":\"4uQeVj5tqViQh7yWWGStvkEG1Zmhx6uasJtWCJziofM\",\"autoApproveNewAccounts\":false,\"auditorElgamalPubkey\":\"ohdsJIKPEtvEhvKRszHlwUpAA55E63xY95Ck/uQMrVU=\"}"; assert_eq!(&serialized, serialized_expected); let deserialized = @@ -95,14 +89,13 @@ fn serde_instruction_optional_nonzero_pubkeys_podbool_with_none() { }; let serialized = serde_json::to_string(&inst).unwrap(); - let serialized_expected = &format!( - "{{\"authority\":null,\"autoApproveNewAccounts\":false,\"auditorElgamalPubkey\":null}}" - ); + let serialized_expected = + &"{\"authority\":null,\"autoApproveNewAccounts\":false,\"auditorElgamalPubkey\":null}"; assert_eq!(&serialized, serialized_expected); let deserialized = serde_json::from_str::( - &serialized_expected, + serialized_expected, ) .unwrap(); assert_eq!(inst, deserialized); @@ -123,12 +116,12 @@ fn serde_instruction_decryptable_balance_podu64() { }; let serialized = serde_json::to_string(&inst).unwrap(); - let serialized_expected = &format!("{{\"decryptableZeroBalance\":\"OBZmMHBqOhkZ9MLZSYlJJhgaJBnr6kS1C1Kqo1nNcaA3ECOX\",\"maximumPendingBalanceCreditCounter\":1099,\"proofInstructionOffset\":100}}"); + let serialized_expected = &"{\"decryptableZeroBalance\":\"OBZmMHBqOhkZ9MLZSYlJJhgaJBnr6kS1C1Kqo1nNcaA3ECOX\",\"maximumPendingBalanceCreditCounter\":1099,\"proofInstructionOffset\":100}"; assert_eq!(&serialized, serialized_expected); let deserialized = serde_json::from_str::< confidential_transfer::instruction::ConfigureAccountInstructionData, - >(&serialized_expected) + >(serialized_expected) .unwrap(); assert_eq!(inst, deserialized); } @@ -153,7 +146,7 @@ fn serde_instruction_elgamal_pubkey() { assert_eq!(&serialized, serialized_expected); let deserialized = - serde_json::from_str::(&serialized_expected) + serde_json::from_str::(serialized_expected) .unwrap(); assert_eq!(inst, deserialized); } @@ -171,5 +164,5 @@ fn serde_instruction_basis_points() { let serialized_expected = "{\"rateAuthority\":null,\"rate\":127}"; assert_eq!(&serialized, serialized_expected); - serde_json::from_str::(&serialized_expected).unwrap(); + serde_json::from_str::(serialized_expected).unwrap(); }