Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

False positive with gas-struct-packing #597

Open
RyanRHall opened this issue Aug 27, 2024 · 3 comments
Open

False positive with gas-struct-packing #597

RyanRHall opened this issue Aug 27, 2024 · 3 comments

Comments

@RyanRHall
Copy link

RyanRHall commented Aug 27, 2024

I think there is a false positive with the gas-struct-packing rule when using contract types. Here is the MRE:

pragma solidity 0.8.24;

contract Bar {}

contract Foo {
  // this triggers gas-struct-packing
  struct MyStruct1 {
    uint96 a; // ────────╮ 1 slot full
    Bar bar; // ─────────╯
    address myAddr; // ──  20/32 bytes used in 2nd slot
  }

  // this is fine
  struct MyStruct2 {
    uint96 a; // ────────╮ 1 slot full
    address myAddr; // ──╯
    Bar bar; // ─────────  20/32 bytes used in 2nd slot
  }
}

MyStruct1 and MyStruct2 are both "tightly packed", but for some reason the linter doesn't like the first struct. I tested this against solhint v5.0.3

@jhweintraub
Copy link

jhweintraub commented Sep 17, 2024

I found another false positive also when using multiple addresses, since you can't pack 2 addresses into the same slot

struct ExampleStruct {
  address addr1; // 1 slot
  address addr2; // 1 slot
  address addr3; // 1 slot
}
./ExampleContract.sol
  39:3  warning  GC: For [ ExampleStruct ] struct, packing seems inefficient. Try rearranging to achieve 32bytes slots  gas-struct-packing

@dbale-altoros
Copy link
Collaborator

@RyanRHall regarding your issue
As Bar is a contract , solhint cannot know slot space... If you look at the ast tree... Bar is a UserDefinedTypeName
Which by default is considered to take a whole new slot
That is why the first one fails and the second one, which is at the end, does not

@dbale-altoros
Copy link
Collaborator

@jhweintraub this is a good one... i need to check the code... thanks a lot for reporting!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants