Skip to content

Commit

Permalink
CI: Update clippy to use --all-features, fix issues (#7395)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
joncinque authored Oct 25, 2024
1 parent bd51bfc commit d584463
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 48 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/publish-rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions libraries/discriminator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
2 changes: 1 addition & 1 deletion libraries/pod/src/optional_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions token-swap/program/src/constraints.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Various constraints as required for production environments

#[cfg(feature = "production")]
use std::env;
use std::option_env;
use {
crate::{
curve::{
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
29 changes: 15 additions & 14 deletions token-swap/program/src/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ impl Processor {
if let Some(swap_constraints) = swap_constraints {
let owner_key = swap_constraints
.owner_key
.unwrap()
.parse::<Pubkey>()
.map_err(|_| SwapError::InvalidOwner)?;
if fee_account.owner != owner_key {
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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,
Expand All @@ -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,
});
Expand Down Expand Up @@ -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,
Expand All @@ -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,
});
Expand Down Expand Up @@ -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,
});
Expand Down
9 changes: 1 addition & 8 deletions token/program-2022/src/serialization.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 })
}
}

Expand Down
25 changes: 9 additions & 16 deletions token/program-2022/tests/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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 =
Expand All @@ -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::<confidential_transfer::instruction::InitializeMintData>(
&serialized_expected,
serialized_expected,
)
.unwrap();
assert_eq!(inst, deserialized);
Expand All @@ -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);
}
Expand All @@ -153,7 +146,7 @@ fn serde_instruction_elgamal_pubkey() {
assert_eq!(&serialized, serialized_expected);

let deserialized =
serde_json::from_str::<InitializeConfidentialTransferFeeConfigData>(&serialized_expected)
serde_json::from_str::<InitializeConfidentialTransferFeeConfigData>(serialized_expected)
.unwrap();
assert_eq!(inst, deserialized);
}
Expand All @@ -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::<InitializeInstructionData>(&serialized_expected).unwrap();
serde_json::from_str::<InitializeInstructionData>(serialized_expected).unwrap();
}

0 comments on commit d584463

Please sign in to comment.