Skip to content

Commit

Permalink
requested changes
Browse files Browse the repository at this point in the history
  • Loading branch information
ethanoroshiba committed Sep 27, 2024
1 parent e267d1c commit dc7508e
Show file tree
Hide file tree
Showing 15 changed files with 56 additions and 39 deletions.
5 changes: 0 additions & 5 deletions crates/astria-bridge-withdrawer/tests/blackbox/helpers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
#![expect(
clippy::missing_panics_doc,
reason = "These are tests; failing with panics is ok."
)]

mod ethereum;
mod mock_cometbft;
mod mock_sequencer;
Expand Down
5 changes: 5 additions & 0 deletions crates/astria-bridge-withdrawer/tests/blackbox/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
#![expect(
clippy::missing_panics_doc,
reason = "These are tests; failing with panics is ok."
)]

use astria_core::protocol::transaction::v1alpha1::Action;
use helpers::{
assert_actions_eq,
Expand Down
5 changes: 3 additions & 2 deletions crates/astria-conductor/tests/blackbox/main.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
pub mod firm_only;
#[expect(
#![expect(
clippy::missing_panics_doc,
reason = "clippy lints that are not ok in production code but acceptable or wanted in tests"
)]

pub mod firm_only;
pub mod helpers;
pub mod shutdown;
pub mod soft_and_firm;
Expand Down
6 changes: 5 additions & 1 deletion crates/astria-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ compile_error!(
);

#[rustfmt::skip]
#[allow(clippy::allow_attributes, clippy::allow_attributes_without_reason, reason = "cannot prevent allow attributes in generated files")]
#[allow(
clippy::allow_attributes,
clippy::allow_attributes_without_reason,
reason = "cannot prevent allow attributes in generated files"
)]
pub mod generated;

pub mod crypto;
Expand Down
10 changes: 8 additions & 2 deletions crates/astria-core/src/primitive/v1/asset/denom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,10 @@ mod tests {
TooManySegments,
};
#[track_caller]
#[expect(clippy::needless_pass_by_value, reason = "silly lint")]
#[expect(
clippy::needless_pass_by_value,
reason = "asserting on owned variants is less noisy then passing them by reference"
)]
fn assert_error(input: &str, kind: ParseIbcPrefixedErrorKind) {
let error = input
.parse::<IbcPrefixed>()
Expand Down Expand Up @@ -672,7 +675,10 @@ mod tests {
Whitespace,
};
#[track_caller]
#[expect(clippy::needless_pass_by_value, reason = "silly lint")]
#[expect(
clippy::needless_pass_by_value,
reason = "asserting on owned variants is less noisy then passing them by reference"
)]
fn assert_error(input: &str, kind: ParseTracePrefixedErrorKind) {
let error = input
.parse::<TracePrefixed>()
Expand Down
3 changes: 2 additions & 1 deletion crates/astria-core/src/protocol/genesis/v1alpha1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,11 @@ impl Protobuf for GenesisAppState {
type Error = GenesisAppStateError;
type Raw = raw::GenesisAppState;

// TODO (https://github.com/astriaorg/astria/issues/1580): remove this once Rust is upgraded to/past 1.83
#[expect(
clippy::allow_attributes,
clippy::allow_attributes_without_reason,
reason = "false positive"
reason = "false positive on `allowed_fee_assets` due to \"allow\" in the name"
)]
fn try_from_raw_ref(raw: &Self::Raw) -> Result<Self, Self::Error> {
let Self::Raw {
Expand Down
24 changes: 12 additions & 12 deletions crates/astria-core/src/protocol/transaction/v1alpha1/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl TryFrom<raw::Action> for Action {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, thiserror::Error)]
#[error(transparent)]
Expand Down Expand Up @@ -398,7 +398,7 @@ enum SequenceActionErrorKind {
#[derive(Clone, Debug)]
#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
pub struct SequenceAction {
pub rollup_id: RollupId,
Expand Down Expand Up @@ -453,7 +453,7 @@ impl Protobuf for SequenceAction {
#[derive(Clone, Debug)]
#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
pub struct TransferAction {
pub to: Address,
Expand Down Expand Up @@ -702,7 +702,7 @@ impl TryFrom<crate::generated::astria_vendored::tendermint::abci::ValidatorUpdat
#[derive(Clone, Debug)]
#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
pub struct SudoAddressChangeAction {
pub new_address: Address,
Expand Down Expand Up @@ -779,7 +779,7 @@ enum SudoAddressChangeActionErrorKind {
#[derive(Debug, Clone)]
#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
pub struct IbcSudoChangeAction {
pub new_address: Address,
Expand Down Expand Up @@ -1183,7 +1183,7 @@ enum Ics20WithdrawalErrorKind {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone)]
pub enum IbcRelayerChangeAction {
Expand Down Expand Up @@ -1267,7 +1267,7 @@ enum IbcRelayerChangeActionErrorKind {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone)]
pub enum FeeAssetChangeAction {
Expand Down Expand Up @@ -1351,7 +1351,7 @@ enum FeeAssetChangeActionErrorKind {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone)]
pub struct InitBridgeAccountAction {
Expand Down Expand Up @@ -1511,7 +1511,7 @@ enum InitBridgeAccountActionErrorKind {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone)]
pub struct BridgeLockAction {
Expand Down Expand Up @@ -1646,7 +1646,7 @@ enum BridgeLockActionErrorKind {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BridgeUnlockAction {
Expand Down Expand Up @@ -1797,7 +1797,7 @@ enum BridgeUnlockActionErrorKind {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistent and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone)]
pub struct BridgeSudoChangeAction {
Expand Down Expand Up @@ -1947,7 +1947,7 @@ pub enum FeeChange {

#[expect(
clippy::module_name_repetitions,
reason = "we want consistend and specific naming"
reason = "for parity with the Protobuf spec"
)]
#[derive(Debug, Clone)]
pub struct FeeChangeAction {
Expand Down
5 changes: 4 additions & 1 deletion crates/astria-merkle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ impl<'a> LeafBuilder<'a> {
/// Writes `bytes` into the builder, appending to the leaf.
///
/// See [`Tree::build_leaf`] for example usage.
#[expect(clippy::missing_panics_doc, reason = "invariant of the system")]
#[expect(
clippy::missing_panics_doc,
reason = "invariant of the system: the hasher must be set"
)]
pub fn write(&mut self, bytes: &[u8]) -> &mut Self {
let hasher = self
.hasher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ mod tests {
#[test]
fn should_fail_to_construct_from_missing_file() {
let error = CelestiaKeys::from_path("missing").unwrap_err();
// TODO (https://github.com/astriaorg/astria/issues/1581): create function for handling this and remove #[expect] (here and below)
#[expect(
clippy::manual_assert,
reason = "`assert!(matches!(..))` provides poor feedback on failure"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ fn new_msg_pay_for_blobs_should_fail_for_large_blob() {
commitment: Commitment([0; 32]),
};
let error = new_msg_pay_for_blobs(&[blob], Bech32Address("a".to_string())).unwrap_err();

// TODO (https://github.com/astriaorg/astria/issues/1581): create function for handling this and remove #[expect] (here and below)
#[expect(
clippy::manual_assert,
reason = "`assert!(matches!(..))` provides poor feedback on failure"
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-sequencer/src/bridge/bridge_lock_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ mod tests {
#[track_caller]
#[expect(
clippy::arithmetic_side_effects,
reason = "test will never overflow u128"
reason = "adding length of strings will never overflow u128 on currently existing machines"
)]
fn assert_correct_base_deposit_fee(deposit: &Deposit) {
let calculated_len = calculate_base_deposit_fee(deposit).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-sequencer/src/bridge/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn error_query_response(
}
}

// FIXME: there is a lot of code duplication due to `error_query_response`.
// FIXME (https://github.com/astriaorg/astria/issues/1582): there is a lot of code duplication due to `error_query_response`.
// this could be significantly shortened.
#[expect(clippy::too_many_lines, reason = "should be refactored")]
async fn get_bridge_account_info(
Expand Down
22 changes: 10 additions & 12 deletions crates/astria-sequencer/src/mempool/benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,11 @@ fn run_maintenance<T: MempoolSize>(bencher: divan::Bencher) {
.unwrap();
// Set the new nonce so that the entire `REMOVAL_CACHE_SIZE` entries in the
// `comet_bft_removal_cache` are filled (assuming this test case has enough txs).
#[expect(
clippy::arithmetic_side_effects,
clippy::cast_possible_truncation,
reason = "this is test-only code, using small values, and where the result is not critical"
)]
let new_nonce = (super::REMOVAL_CACHE_SIZE as u32 / u32::from(SIGNER_COUNT)) + 1;
let new_nonce = u32::try_from(super::REMOVAL_CACHE_SIZE)
.unwrap()
.checked_div(u32::from(SIGNER_COUNT))
.and_then(|res| res.checked_add(1))
.unwrap();
let mock_balances = mock_balances(0, 0);
let mut mock_state = runtime.block_on(mock_state_getter());

Expand Down Expand Up @@ -328,12 +327,11 @@ fn run_maintenance_tx_recosting<T: MempoolSize>(bencher: divan::Bencher) {
.unwrap();
// Set the new nonce so that the entire `REMOVAL_CACHE_SIZE` entries in the
// `comet_bft_removal_cache` are filled (assuming this test case has enough txs).
#[expect(
clippy::arithmetic_side_effects,
clippy::cast_possible_truncation,
reason = "this is test-only code, using small values, and where the result is not critical"
)]
let new_nonce = (super::REMOVAL_CACHE_SIZE as u32 / u32::from(SIGNER_COUNT)) + 1;
let new_nonce = u32::try_from(super::REMOVAL_CACHE_SIZE)
.unwrap()
.checked_div(u32::from(SIGNER_COUNT))
.and_then(|res| res.checked_add(1))
.unwrap();
let mock_balances = mock_balances(0, 0);
let mut mock_state = runtime.block_on(mock_state_getter());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@ mod test {

// From https://doc.rust-lang.org/std/cmp/trait.PartialOrd.html
#[test]
// TODO (https://github.com/astriaorg/astria/issues/1583): rework assertions and remove attribute
#[expect(
clippy::nonminimal_bool,
reason = "we want explicit assertions here to match the documented expected behavior"
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-sequencer/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl ActionHandler for SignedTransaction {
Ok(())
}

// FIXME: because most lines come from delegating (and error wrapping) to the
// FIXME (https://github.com/astriaorg/astria/issues/1584): because most lines come from delegating (and error wrapping) to the
// individual actions. This could be tidied up by implementing `ActionHandler for Action`
// and letting it delegate.
#[expect(clippy::too_many_lines, reason = "should be refactored")]
Expand Down

0 comments on commit dc7508e

Please sign in to comment.