From 4d7c72aa4559a7845fb841211d5ef9f9fe71109b Mon Sep 17 00:00:00 2001 From: Luca Georges Francois Date: Wed, 2 Aug 2023 00:24:37 +0200 Subject: [PATCH 1/4] tests(contracts): add new test contract for struct repacker Signed-off-by: Luca Georges Francois --- tests/contracts/StructsToRepack.sol | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/contracts/StructsToRepack.sol diff --git a/tests/contracts/StructsToRepack.sol b/tests/contracts/StructsToRepack.sol new file mode 100644 index 0000000..2dae1ba --- /dev/null +++ b/tests/contracts/StructsToRepack.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: UNLICENSED + +pragma solidity ^0.8.15; + +struct SimpleStruct { + uint128 a; + uint256 b; + uint128 c; +} From a9f1d54baf2dda6f34bba9c77885d4753738eb7c Mon Sep 17 00:00:00 2001 From: Luca Georges Francois Date: Wed, 2 Aug 2023 00:25:05 +0200 Subject: [PATCH 2/4] feat(scan): implement ffd strategy for struct repacking Signed-off-by: Luca Georges Francois --- .../implementations/struct_repacker.rs | 130 +++++++++++++++++- 1 file changed, 124 insertions(+), 6 deletions(-) diff --git a/src/scanners/implementations/struct_repacker.rs b/src/scanners/implementations/struct_repacker.rs index f69db63..b9d945f 100644 --- a/src/scanners/implementations/struct_repacker.rs +++ b/src/scanners/implementations/struct_repacker.rs @@ -1,14 +1,132 @@ -use syn_solidity::Item; +use syn_solidity::{Item, ItemContract, ItemStruct, VariableDeclaration}; -use crate::scanners::{result::Reporter, Scanner}; +use crate::scanners::{ + result::{Reporter, Severity}, + Scanner, +}; + +const BIN_SIZE: u16 = 256; + +#[allow(dead_code)] +#[derive(Clone, Debug)] +struct BinItem { + name: String, + size: u16, +} + +impl From<&VariableDeclaration> for BinItem { + fn from(field: &VariableDeclaration) -> Self { + BinItem { + name: field.name.as_ref().unwrap().as_string(), + size: match field.ty { + syn_solidity::Type::Address(_, _) => 20, + syn_solidity::Type::Bool(_) => 1, + syn_solidity::Type::String(_) => 32, + syn_solidity::Type::Bytes(_) => 32, + syn_solidity::Type::FixedBytes(_, s) => s.get(), + syn_solidity::Type::Int(_, s) => s.unwrap().get(), + syn_solidity::Type::Uint(_, s) => s.unwrap().get(), + syn_solidity::Type::Array(_) => 32, + syn_solidity::Type::Tuple(_) => todo!(), + syn_solidity::Type::Function(_) => todo!(), + syn_solidity::Type::Mapping(_) => 32, + syn_solidity::Type::Custom(_) => todo!(), + }, + } + } +} + +trait RepackStrategy { + fn repack(&self, initial_structure: &[BinItem]) -> Vec; +} + +#[derive(Default)] +struct FirstFitDecreasing {} + +impl RepackStrategy for FirstFitDecreasing { + fn repack(&self, initial_structure: &[BinItem]) -> Vec { + let mut cpy = initial_structure.to_vec(); + + // Reorder structure fields from biggest size to smallest. + cpy.sort_by(|a, b| b.size.cmp(&a.size)); + + cpy + } +} #[derive(Default)] pub struct StructRepacker {} +impl StructRepacker { + fn scan_contract(&self, contract: &ItemContract, reporter: &mut Reporter) { + for item in &contract.body { + if let Item::Struct(structure) = item { + self.scan_struct(structure, reporter) + } + } + } + + fn scan_struct(&self, structure: &ItemStruct, reporter: &mut Reporter) { + let initial_structure: Vec = structure.fields.iter().map(BinItem::from).collect(); + let initial_slot_consumption = self.compute_actual_slot_consumption(&initial_structure); + let optimum_slot_consumption = self.compute_optimum_slot_consumption(&initial_structure); + + if initial_slot_consumption != optimum_slot_consumption { + let strategy = FirstFitDecreasing::default(); + let _repacked_struct = strategy.repack(&initial_structure); + + let line = structure.span().start().line; + let column = structure.span().start().column; + + reporter.report( + line, + column, + Severity::Boost, + "Structure can be re-written to potentially consume less storage slots", + ) + } + } + + fn compute_optimum_slot_consumption(&self, structure: &[BinItem]) -> u16 { + (structure.iter().map(|f| f.size).sum::() as f64 / BIN_SIZE as f64) as u16 + } + + fn compute_actual_slot_consumption(&self, structure: &Vec) -> u16 { + let mut slot_consumption: u16 = 0; + let mut carry: u16 = 0; + + for field in structure { + match carry + field.size { + slot if slot < BIN_SIZE => { + carry += field.size; + } + slot if slot == BIN_SIZE => { + slot_consumption += 1; + carry = 0; + } + _ => { + slot_consumption += 1; + carry = field.size; + } + } + } + + if carry != 0 { + slot_consumption += 1 + } + + slot_consumption + } +} + impl Scanner for StructRepacker { - fn execute(&self, _ast: &[Item], _reporter: &mut Reporter) { - /* - println!("todo!") - */ + fn execute(&self, ast: &[Item], reporter: &mut Reporter) { + for item in ast { + match item { + Item::Contract(contract) => self.scan_contract(contract, reporter), + Item::Struct(structure) => self.scan_struct(structure, reporter), + _ => {} + } + } } } From d3c1a046473cc126eb848a5ff3fcd7247afd2be3 Mon Sep 17 00:00:00 2001 From: Luca Georges Francois Date: Fri, 13 Oct 2023 23:04:14 +0200 Subject: [PATCH 3/4] fix: update struct repacker scanner to reflect dependencies bump Signed-off-by: Luca Georges Francois --- src/scanners/implementations/struct_repacker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scanners/implementations/struct_repacker.rs b/src/scanners/implementations/struct_repacker.rs index b9d945f..5b71a67 100644 --- a/src/scanners/implementations/struct_repacker.rs +++ b/src/scanners/implementations/struct_repacker.rs @@ -1,4 +1,4 @@ -use syn_solidity::{Item, ItemContract, ItemStruct, VariableDeclaration}; +use syn_solidity::{Item, ItemContract, ItemStruct, Spanned, VariableDeclaration}; use crate::scanners::{ result::{Reporter, Severity}, From fc095a603a4da08a95fac0b7bbcd25d9febfce0c Mon Sep 17 00:00:00 2001 From: Luca Georges Francois Date: Fri, 13 Oct 2023 23:08:11 +0200 Subject: [PATCH 4/4] fix(scanners): remove todo making the program panic Signed-off-by: Luca Georges Francois --- src/scanners/implementations/struct_repacker.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/scanners/implementations/struct_repacker.rs b/src/scanners/implementations/struct_repacker.rs index 5b71a67..8b652db 100644 --- a/src/scanners/implementations/struct_repacker.rs +++ b/src/scanners/implementations/struct_repacker.rs @@ -27,10 +27,10 @@ impl From<&VariableDeclaration> for BinItem { syn_solidity::Type::Int(_, s) => s.unwrap().get(), syn_solidity::Type::Uint(_, s) => s.unwrap().get(), syn_solidity::Type::Array(_) => 32, - syn_solidity::Type::Tuple(_) => todo!(), - syn_solidity::Type::Function(_) => todo!(), + syn_solidity::Type::Tuple(_) => 0, // todo!() + syn_solidity::Type::Function(_) => 0, // todo!() syn_solidity::Type::Mapping(_) => 32, - syn_solidity::Type::Custom(_) => todo!(), + syn_solidity::Type::Custom(_) => 0, // todo!() }, } }