search

huff-language / huffmate

A library of modern, hyper-optimized, and extensible Huff contracts with extensive testing and documentation built by Huff maintainers.
https://github.com/pentagonxyz/huffmate
MIT License
430 stars 55 forks source link

bug: SafeTransferLib #119

Closed obatirou closed 1 year ago

obatirou commented 1 year ago

This PR uncover a bug in SafeTransferLib by fixing the fuzzing.

This needs investigation.

Capture d’écran 2023-03-24 à 20 01 38
Maddiaa0 commented 1 year ago

Its interesting that merging the above pr did not break ci, do you know your current fuzzer seed?

clabby commented 1 year ago

I don't believe this is a bug, but thank you for reporting 😄

If you notice, the to address is the Hardhat console address, and that contract is not rejected by the test because it has no code (foundry hooks into it when it receives a call, and reverts forcefully if no selectors match).

We should fix up this test to exclude the console and VM addresses.

obatirou commented 1 year ago

I don't believe this is a bug, but thank you for reporting 😄

If you notice, the to address is the Hardhat console address, and that contract is not rejected by the test because it has no code (foundry hooks into it when it receives a call, and reverts forcefully if no selectors match).

We should fix up this test to exclude the console and VM addresses.

Oh you’re right, thank you ! I should have not labeled this issue as a bug without looking into it a little bit. Unfortunately, did not have time when discovering this failing test. It should have been tagged as a failing test and not a bug until confirmation. I will make sure to pay more attention next time !

clabby commented 1 year ago

I don't believe this is a bug, but thank you for reporting smile If you notice, the to address is the Hardhat console address, and that contract is not rejected by the test because it has no code (foundry hooks into it when it receives a call, and reverts forcefully if no selectors match). We should fix up this test to exclude the console and VM addresses.

Oh you’re right, thank you ! I should have not labeled this issue as a bug without looking into it a little bit. Unfortunately, did not have time when discovering this failing test. It should have been tagged as a failing test and not a bug until confirmation. I will make sure to pay more attention next time !

No worries! Better safe than sorry, much appreciated.