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

Deny clippy::arithmetic_side_effects #725

Merged
merged 16 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .npm/packages/fuel-tx/index.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('fuel-tx [cjs]', () => {
})

it('should serialize and deserialize UtxoId correctly', () => {
let utxo_id = new tx.UtxoId("0x0c0000000000000000000000000000000000000000000000000000000000000b00001a");
let utxo_id = new tx.UtxoId("0x0c0000000000000000000000000000000000000000000000000000000000000b001a");
let bytes = utxo_id.to_bytes();
let utxo_id2 = tx.UtxoId.from_bytes(bytes);
expect(utxo_id.toString()).to.equal(utxo_id2.toString())
Expand Down
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Breaking
### Changed

- [#725](https://github.com/FuelLabs/fuel-vm/pull/725): Adds more clippy lints to catch possible integer overflow and casting bugs on compile time.

### Added

#### Breaking

- [#725](https://github.com/FuelLabs/fuel-vm/pull/725): `UtxoId::from_str` now rejects inputs with multiple `0x` prefixes. Many `::from_str` implementations also reject extra data in the end of the input, instead of silently ignoring it. `UtxoId::from_str` allows a single `:` between the fields. Unused `GasUnit` struct removed.
- [#726](https://github.com/FuelLabs/fuel-vm/pull/726): Removed code related to Binary Merkle Sum Trees (BMSTs). The BMST is deprecated and not used in production environments.

## [Version 0.49.0]
Expand Down
9 changes: 7 additions & 2 deletions fuel-asm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "std", doc = include_str!("../README.md"))]
#![deny(clippy::cast_possible_truncation)]
#![deny(clippy::string_slice)]
#![deny(
clippy::arithmetic_side_effects,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::string_slice
)]
#![deny(missing_docs)]
#![deny(unsafe_code)]
#![deny(unused_crate_dependencies)]
Expand Down
9 changes: 7 additions & 2 deletions fuel-crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@
#![cfg_attr(not(feature = "std"), no_std)]
// Wrong clippy convention; check
// https://rust-lang.github.io/api-guidelines/naming.html
#![deny(clippy::string_slice)]
#![warn(missing_docs)]
#![deny(unsafe_code)]
#![deny(unused_crate_dependencies)]
#![deny(clippy::cast_possible_truncation)]
#![deny(
clippy::arithmetic_side_effects,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::string_slice
)]

// Satisfy unused_crate_dependencies lint for self-dependency enabling test features
#[cfg(test)]
Expand Down
7 changes: 7 additions & 0 deletions fuel-derive/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
//! Derive macros for canonical type serialization and deserialization.

#![deny(unused_must_use, missing_docs)]
#![deny(
clippy::arithmetic_side_effects,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::string_slice
)]

extern crate proc_macro;
mod attribute;
Expand Down
18 changes: 9 additions & 9 deletions fuel-derive/src/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,24 @@ fn serialize_struct(s: &synstructure::Structure) -> TokenStream2 {

let size_static_code = variant.each(|binding| {
quote! {
size = ::fuel_types::canonical::add_sizes(size, #binding.size_static());
size = size.saturating_add(#binding.size_static());
}
});

let initial_size = if attrs.prefix.is_some() {
quote! { let mut size = 8; }
quote! { let mut size = 8usize; }
} else {
quote! { let mut size = 0; }
quote! { let mut size = 0usize; }
};
let size_static_code = quote! { #initial_size match self { #size_static_code}; size };

let size_dynamic_code = variant.each(|binding| {
quote! {
size = ::fuel_types::canonical::add_sizes(size, #binding.size_dynamic());
size = size.saturating_add(#binding.size_dynamic());
}
});
let size_dynamic_code =
quote! { let mut size = 0; match self { #size_dynamic_code}; size };
quote! { let mut size = 0usize; match self { #size_dynamic_code}; size };

let prefix = if let Some(prefix_type) = attrs.prefix.as_ref() {
quote! {
Expand Down Expand Up @@ -143,14 +143,14 @@ fn serialize_enum(s: &synstructure::Structure) -> TokenStream2 {
.map(|variant| {
variant.each(|binding| {
quote! {
size = ::fuel_types::canonical::add_sizes(size, #binding.size_static());
size = size.saturating_add(#binding.size_static());
}
})
})
.collect();
let match_size_static = quote! {{
// `repr(128)` is unstable, so because of that we can use 8 bytes.
let mut size = 8;
let mut size = 8usize;
match self { #match_size_static } size }
};

Expand All @@ -160,13 +160,13 @@ fn serialize_enum(s: &synstructure::Structure) -> TokenStream2 {
.map(|variant| {
variant.each(|binding| {
quote! {
size = ::fuel_types::canonical::add_sizes(size, #binding.size_dynamic());
size = size.saturating_add(#binding.size_dynamic());
}
})
})
.collect();
let match_size_dynamic =
quote! {{ let mut size = 0; match self { #match_size_dynamic } size }};
quote! {{ let mut size = 0usize; match self { #match_size_dynamic } size }};

let impl_code = s.gen_impl(quote! {
gen impl ::fuel_types::canonical::Serialize for @Self {
Expand Down
2 changes: 1 addition & 1 deletion fuel-merkle/test-helpers/src/binary/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::binary::{
pub fn verify<T: AsRef<[u8]>>(
root: &Data,
data: &T,
proof_set: &Vec<Data>,
proof_set: &[Data],
proof_index: u64,
num_leaves: u64,
) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion fuel-merkle/test-helpers/src/suites/binary_proofs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn generate_test(
name: String,
function_name: String,
description: String,
sample_data: &Vec<Bytes32>,
sample_data: &[Bytes32],
proof_index: u64,
) -> ProofTest {
let (root, proof_set) = {
Expand Down
8 changes: 7 additions & 1 deletion fuel-storage/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
#![no_std]
#![deny(clippy::cast_possible_truncation)]
#![deny(
clippy::arithmetic_side_effects,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::string_slice
)]
#![deny(unsafe_code)]
#![deny(unused_crate_dependencies)]

Expand Down
4 changes: 4 additions & 0 deletions fuel-tx/src/consts.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use crate::TxPointer;
use fuel_types::{
bytes::WORD_SIZE,
AssetId,
Bytes32,
Salt,
};

/// Size of balance entry, i.e. asset id and associated balance.
pub const BALANCE_ENTRY_SIZE: usize = AssetId::LEN + WORD_SIZE;

pub const TRANSACTION_SCRIPT_FIXED_SIZE: usize = WORD_SIZE // Identifier
+ WORD_SIZE // Gas price
+ WORD_SIZE // Gas limit
Expand Down
12 changes: 4 additions & 8 deletions fuel-tx/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ const LEAF_SIZE: usize = 16 * 1024;
const PADDING_BYTE: u8 = 0u8;
const MULTIPLE: usize = 8;

/// See https://stackoverflow.com/a/9194117
fn next_multiple<const N: usize>(x: usize) -> usize {
x + (N - x % N) % N
}

#[derive(Default, Derivative, Clone, PartialEq, Eq, Hash)]
#[derivative(Debug)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down Expand Up @@ -87,7 +82,7 @@ impl Contract {
if len == LEAF_SIZE || len % MULTIPLE == 0 {
tree.push(leaf);
} else {
let padding_size = next_multiple::<MULTIPLE>(len);
let padding_size = len.next_multiple_of(MULTIPLE);
let mut padded_leaf = [PADDING_BYTE; LEAF_SIZE];
padded_leaf[0..len].clone_from_slice(leaf);
tree.push(padded_leaf[..padding_size].as_ref());
Expand Down Expand Up @@ -178,6 +173,7 @@ impl TryFrom<&Transaction> for Contract {
}
}

#[allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)]
#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -302,7 +298,7 @@ mod tests {
// length rounded to the nearest multiple of 8, and each byte is the
// PADDING_BYTE by default. The leaf is generated by copying the
// remaining data bytes into the start of this array.
let sz = next_multiple::<8>(partial_leaf_size);
let sz = partial_leaf_size.next_multiple_of(8);
if sz > 0 {
let mut padded_leaf = vec![PADDING_BYTE; sz];
padded_leaf[0..code_len].clone_from_slice(&code);
Expand Down Expand Up @@ -346,7 +342,7 @@ mod tests {
// length rounded to the nearest multiple of 8, and each byte is the
// PADDING_BYTE by default. The leaf is generated by copying the
// remaining data bytes into the start of this array.
let sz = next_multiple::<8>(partial_leaf_size);
let sz = partial_leaf_size.next_multiple_of(8);
if sz > 0 {
let mut padded_leaf = vec![PADDING_BYTE; sz];
padded_leaf[0..partial_leaf_size].clone_from_slice(leaves[3]);
Expand Down
10 changes: 7 additions & 3 deletions fuel-tx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
// Wrong clippy convention; check
// https://rust-lang.github.io/api-guidelines/naming.html
#![allow(clippy::wrong_self_convention)]
#![deny(clippy::cast_possible_truncation)]
#![deny(clippy::string_slice)]
#![deny(
clippy::arithmetic_side_effects,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::string_slice
)]
#![deny(unused_crate_dependencies)]
#![deny(unsafe_code)]

Expand Down Expand Up @@ -91,7 +96,6 @@ pub use transaction::{
FormatValidityChecks,
GasCosts,
GasCostsValues,
GasUnit,
Mint,
PredicateParameters,
Script,
Expand Down
2 changes: 2 additions & 0 deletions fuel-tx/src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)]

mod offset;
mod valid_cases;

Expand Down
1 change: 0 additions & 1 deletion fuel-tx/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ pub use consensus_parameters::{
FeeParameters,
GasCosts,
GasCostsValues,
GasUnit,
PredicateParameters,
ScriptParameters,
TxParameters,
Expand Down
19 changes: 14 additions & 5 deletions fuel-tx/src/transaction/consensus_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ pub use gas::{
DependentCost,
GasCosts,
GasCostsValues,
GasUnit,
};

use crate::consts::BALANCE_ENTRY_SIZE;

#[cfg(feature = "test-helpers")]
const MAX_GAS: u64 = 100_000_000;
#[cfg(feature = "test-helpers")]
Expand Down Expand Up @@ -512,11 +513,19 @@ impl TxParameters {

/// Transaction memory offset in VM runtime
pub const fn tx_offset(&self) -> usize {
Bytes32::LEN // Tx ID
+ AssetId::LEN // Base asset ID
// Asset ID/Balance coin input pairs
+ self.max_inputs() as usize * (AssetId::LEN + WORD_SIZE)
let Some(balances_size) =
(self.max_inputs() as usize).checked_mul(BALANCE_ENTRY_SIZE)
else {
panic!(
"Consensus parameters shouldn't allow max_inputs to cause overflow here"
);
};

balances_size.saturating_add(
Bytes32::LEN // Tx ID
+ WORD_SIZE // Tx size
+ AssetId::LEN, // Base asset ID
)
}

/// Replace the max inputs with the given argument
Expand Down
Loading
Loading