Skip to content

Commit

Permalink
Deny lint.cast_possible_wrap in the whole codebase (#2834)
Browse files Browse the repository at this point in the history
# Description
Cf. discussion in #2828

Casting uXX into their corresponding iXX is dangerous as it can lead to
silent overflow with very weird outcomes.
 
# Changes
- [x] Adds a workspace specific lint rule set and imports it into all
subspaces
- [x] Adds `cast_possible_wrap=deny` to catch all occurrences where we
do these unsafe conversions

Up for discussion if other `as` lints (e.g.
`cast_lossless/cast_possible_truncation/cast_precision_loss/cast_sign_loss`)
or even `clippy::as_conversions` as a whole should be denied.

## How to test
Clippy
  • Loading branch information
fleupold authored Jul 26, 2024
1 parent f45b6bd commit 74a1271
Show file tree
Hide file tree
Showing 38 changed files with 119 additions and 27 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,6 @@ tracing-subscriber = "0.3.18"
url = "2.5.0"
warp = { git = 'https://github.com/cowprotocol/warp.git', rev = "586244e", default-features = false }
web3 = { version = "0.19.0", default-features = false }

[workspace.lints]
clippy.cast_possible_wrap = "deny"
3 changes: 3 additions & 0 deletions crates/alerter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@ tracing = { workspace = true }
tracing-subscriber = { workspace = true }
url = { workspace = true }
warp = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/app-data-hash/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ license = "MIT OR Apache-2.0"
[dependencies]
hex-literal = { workspace = true }
tiny-keccak = { version = "2.0.2", features = ["keccak"] }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/app-data/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ ethcontract = { workspace = true }

[features]
test_helpers = []

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/autopilot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,6 @@ web3 = { workspace = true }
mockall = { workspace = true }
testlib = { path = "../testlib" }
tokio = { workspace = true, features = ["test-util"] }

[lints]
workspace = true
4 changes: 2 additions & 2 deletions crates/autopilot/src/database/ethflow_events/event_storing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ impl EventStoring<EthFlowEvent> for Postgres {
let mut ex = self.pool.begin().await?;
database::ethflow_orders::delete_refunds(
&mut ex,
*range.start() as i64,
*range.end() as i64,
i64::try_from(*range.start()).unwrap_or(i64::MAX),
i64::try_from(*range.end()).unwrap_or(i64::MAX),
)
.await?;
database::ethflow_orders::insert_refund_tx_hashes(&mut ex, &refunds).await?;
Expand Down
4 changes: 2 additions & 2 deletions crates/autopilot/src/database/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ pub async fn replace_events(

pub fn meta_to_event_index(meta: &EventMetadata) -> EventIndex {
EventIndex {
block_number: meta.block_number as i64,
log_index: meta.log_index as i64,
block_number: i64::try_from(meta.block_number).unwrap_or(i64::MAX),
log_index: i64::try_from(meta.log_index).unwrap_or(i64::MAX),
}
}

Expand Down
4 changes: 3 additions & 1 deletion crates/autopilot/src/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ impl RunLoop {
async fn single_run(&self, id: domain::auction::Id, auction: &domain::Auction) {
tracing::info!("solving");
Metrics::get().auction.set(id);
Metrics::get().orders.set(auction.orders.len() as _);
Metrics::get()
.orders
.set(i64::try_from(auction.orders.len()).unwrap_or(i64::MAX));

let mut participants = self.competition(id, auction).await;

Expand Down
6 changes: 3 additions & 3 deletions crates/autopilot/src/solvable_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ impl OrderFilterCounter {
metrics
.auction_candidate_orders
.with_label_values(&[class])
.set(count as _);
.set(i64::try_from(count).unwrap_or(i64::MAX));
}

Self {
Expand Down Expand Up @@ -724,14 +724,14 @@ impl OrderFilterCounter {
self.metrics
.auction_solvable_orders
.with_label_values(&[class])
.set(count as _);
.set(i64::try_from(count).unwrap_or(i64::MAX));
}

for (reason, count) in self.counts {
self.metrics
.auction_filtered_orders
.with_label_values(&[reason])
.set(count as _);
.set(i64::try_from(count).unwrap_or(i64::MAX));
}

removed
Expand Down
3 changes: 3 additions & 0 deletions crates/bytes-hex/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ serde = { workspace = true }
serde_json = { workspace = true }
serde_with = { workspace = true }
hex = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/contracts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,6 @@ tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"], optio
[build-dependencies]
ethcontract = { workspace = true }
ethcontract-generate = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/cow-amm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ tokio = { workspace = true, features = [] }
tracing = { workspace = true }
hex = { workspace = true }
hex-literal = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/database/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ strum = { workspace = true }
[dev-dependencies]
hex-literal = { workspace = true }
tokio = { workspace = true, features = ["macros"] }

[lints]
workspace = true
2 changes: 1 addition & 1 deletion crates/database/src/ethflow_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub async fn insert_refund_tx_hash(
ex.execute(
sqlx::query(QUERY)
.bind(refund.order_uid)
.bind(refund.block_number as i64)
.bind(i64::try_from(refund.block_number).unwrap_or(i64::MAX))
.bind(refund.tx_hash),
)
.await?;
Expand Down
9 changes: 5 additions & 4 deletions crates/database/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,21 @@ pub async fn delete(
ex: &mut PgTransaction<'_>,
delete_from_block_number: u64,
) -> Result<(), sqlx::Error> {
let delete_from_block_number = i64::try_from(delete_from_block_number).unwrap_or(i64::MAX);
const QUERY_INVALIDATION: &str = "DELETE FROM invalidations WHERE block_number >= $1;";
ex.execute(sqlx::query(QUERY_INVALIDATION).bind(delete_from_block_number as i64))
ex.execute(sqlx::query(QUERY_INVALIDATION).bind(delete_from_block_number))
.await?;

const QUERY_TRADE: &str = "DELETE FROM trades WHERE block_number >= $1;";
ex.execute(sqlx::query(QUERY_TRADE).bind(delete_from_block_number as i64))
ex.execute(sqlx::query(QUERY_TRADE).bind(delete_from_block_number))
.await?;

const QUERY_SETTLEMENTS: &str = "DELETE FROM settlements WHERE block_number >= $1;";
ex.execute(sqlx::query(QUERY_SETTLEMENTS).bind(delete_from_block_number as i64))
ex.execute(sqlx::query(QUERY_SETTLEMENTS).bind(delete_from_block_number))
.await?;

const QUERY_PRESIGNATURES: &str = "DELETE FROM presignature_events WHERE block_number >= $1;";
ex.execute(sqlx::query(QUERY_PRESIGNATURES).bind(delete_from_block_number as i64))
ex.execute(sqlx::query(QUERY_PRESIGNATURES).bind(delete_from_block_number))
.await?;

Ok(())
Expand Down
14 changes: 10 additions & 4 deletions crates/database/src/settlements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,18 @@ pub async fn delete(
) -> Result<(), sqlx::Error> {
const QUERY_OBSERVATIONS: &str =
"DELETE FROM settlement_observations WHERE block_number >= $1;";
ex.execute(sqlx::query(QUERY_OBSERVATIONS).bind(delete_from_block_number as i64))
.await?;
ex.execute(
sqlx::query(QUERY_OBSERVATIONS)
.bind(i64::try_from(delete_from_block_number).unwrap_or(i64::MAX)),
)
.await?;

const QUERY_ORDER_EXECUTIONS: &str = "DELETE FROM order_execution WHERE block_number >= $1;";
ex.execute(sqlx::query(QUERY_ORDER_EXECUTIONS).bind(delete_from_block_number as i64))
.await?;
ex.execute(
sqlx::query(QUERY_ORDER_EXECUTIONS)
.bind(i64::try_from(delete_from_block_number).unwrap_or(i64::MAX)),
)
.await?;

Ok(())
}
Expand Down
3 changes: 3 additions & 0 deletions crates/driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,6 @@ mockall = { workspace = true }
solvers-dto = { path = "../solvers-dto" }
tokio = { workspace = true, features = ["test-util", "process"] }
tempfile = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/e2e/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,6 @@ app-data-hash = { path = "../app-data-hash" }
futures = { workspace = true }
rand = { workspace = true }
refunder = { path = "../refunder" }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/ethrpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@ itertools = { workspace = true }
[dev-dependencies]
maplit = { workspace = true }
testlib = { path = "../testlib" }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/model/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ maplit = { workspace = true }

[features]
e2e = []

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/number/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ num = { workspace = true }
primitive-types = { workspace = true }
serde_with = { workspace = true }
serde = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/observe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ time = { workspace = true }
tokio = { workspace = true, features = [] }
tracing = { workspace = true }
tracing-subscriber = { workspace = true, features = ["env-filter", "fmt", "time"] }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/order-validation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ contracts = { path = "../contracts" }
ethcontract = { workspace = true }
thiserror = { workspace = true }
tracing = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/orderbook/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,6 @@ tokio = { workspace = true, features = ["test-util"] }
[build-dependencies]
anyhow = { workspace = true }
vergen = { version = "8", features = ["git", "gitcl"] }

[lints]
workspace = true
4 changes: 2 additions & 2 deletions crates/orderbook/src/database/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,8 @@ impl OrderStoring for Postgres {
database::orders::user_orders(
&mut ex,
&ByteArray(owner.0),
offset as i64,
limit.map(|l| l as i64),
i64::try_from(offset).unwrap_or(i64::MAX),
limit.map(|l| i64::try_from(l).unwrap_or(i64::MAX)),
)
.map(|result| match result {
Ok(order) => full_order_into_model_order(order),
Expand Down
3 changes: 3 additions & 0 deletions crates/rate-limit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ reqwest = { workspace = true, features = ["json"] }
thiserror = { workspace = true }
tokio = { workspace = true, features = [] }
tracing = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/refunder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ tokio = { workspace = true, features = ["macros", "time", "rt-multi-thread"] }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
url = { workspace = true }

[lints]
workspace = true
2 changes: 1 addition & 1 deletion crates/refunder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub async fn run(args: arguments::Arguments) {
pg_pool,
web3,
ethflow_contract,
args.min_validity_duration.as_secs() as i64,
i64::try_from(args.min_validity_duration.as_secs()).unwrap_or(i64::MAX),
args.min_slippage_bps,
refunder_account,
);
Expand Down
3 changes: 3 additions & 0 deletions crates/s3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ serde_json = { workspace = true }
[dev-dependencies]
chrono = { workspace = true, features = ["clock"] }
tokio = { workspace = true, features = ["test-util", "macros"] }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/shared/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,6 @@ regex = { workspace = true }
testlib = { path = "../testlib" }
app-data = { path = "../app-data", features = ["test_helpers"] }
tokio = { workspace = true, features = ["rt-multi-thread"] }

[lints]
workspace = true
8 changes: 6 additions & 2 deletions crates/shared/src/maintenance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ impl ServiceMaintenance {
"running maintenance",
);

self.metrics.last_seen_block.set(block.number as _);
self.metrics
.last_seen_block
.set(i64::try_from(block.number).unwrap_or(i64::MAX));

if let Err(err) = self
.run_maintenance()
Expand All @@ -115,7 +117,9 @@ impl ServiceMaintenance {
continue;
}

self.metrics.last_updated_block.set(block.number as _);
self.metrics
.last_updated_block
.set(i64::try_from(block.number).unwrap_or(i64::MAX));
self.metrics
.runs
.with_label_values(&["success", self.name()])
Expand Down
4 changes: 2 additions & 2 deletions crates/shared/src/price_estimation/native_price_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,14 @@ impl UpdateTask {
let metrics = Metrics::get();
metrics
.native_price_cache_size
.set(inner.cache.lock().unwrap().len() as i64);
.set(i64::try_from(inner.cache.lock().unwrap().len()).unwrap_or(i64::MAX));

let max_age = inner.max_age.saturating_sub(self.prefetch_time);
let mut outdated_entries = inner.sorted_tokens_to_update(max_age, Instant::now());

metrics
.native_price_cache_outdated_entries
.set(outdated_entries.len() as i64);
.set(i64::try_from(outdated_entries.len()).unwrap_or(i64::MAX));

outdated_entries.truncate(self.update_size.unwrap_or(usize::MAX));

Expand Down
6 changes: 4 additions & 2 deletions crates/shared/src/sources/balancer_v2/swap/stable_math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,10 @@ mod tests {
}
}
let b = (invariant / (amplification_parameter * num_tokens)) + sum - invariant;
let c = (invariant.powi(num_tokens_usize as i32 + 1) * -1.)
/ (amplification_parameter * num_tokens.powi(num_tokens_usize as i32 + 1) * prod);
let c = (invariant.powi(i32::try_from(num_tokens_usize).unwrap_or(i32::MAX) + 1) * -1.)
/ (amplification_parameter
* num_tokens.powi(i32::try_from(num_tokens_usize).unwrap_or(i32::MAX) + 1)
* prod);
let x = b.powi(2) - (c * 4.);
let root_x = x.powf(0.5);
let neg_b = b * -1.;
Expand Down
4 changes: 3 additions & 1 deletion crates/shared/src/zeroex_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,9 @@ fn retain_valid_orders(orders: &mut Vec<OrderRecord>) {
let mut included_orders = HashSet::new();
let now = chrono::offset::Utc::now();
orders.retain(|order| {
let expiry = Utc.timestamp_opt(order.order().expiry as i64, 0).unwrap();
let expiry = Utc
.timestamp_opt(i64::try_from(order.order().expiry).unwrap_or(i64::MAX), 0)
.unwrap();

// only keep orders which are still valid and unique
expiry > now && included_orders.insert(order.metadata().order_hash.clone())
Expand Down
3 changes: 3 additions & 0 deletions crates/solver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,6 @@ ethcontract-mock = { workspace = true }
tokio = { workspace = true, features = ["test-util"] }
tracing-subscriber = { workspace = true }
testlib = { path = "../testlib" }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/solvers-dto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ number = { path = "../number" }
serde = { workspace = true }
serde_with = { workspace = true }
web3 = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/solvers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,6 @@ tracing = { workspace = true }
tempfile = { workspace = true }
hex-literal = { workspace = true }
ethcontract = { workspace = true }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/testlib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ license = "MIT OR Apache-2.0"
ethcontract = { workspace = true }
ethcontract-mock = { workspace = true }
hex-literal = { workspace = true }

[lints]
workspace = true

0 comments on commit 74a1271

Please sign in to comment.