forge-std icon indicating copy to clipboard operation
forge-std copied to clipboard

Fix bug in StdStorage.sol where stdstore variable is not reset

Open kaihiroi opened this issue 3 years ago • 1 comments

Hello,

This pull request fixes a bug in the StdStorage.sol contract where the stdstore variable was not being reset after find() function was executed. This could cause problems if the find() function was called multiple times, as it may not find the correct slot number.

To fix this issue, I have added an expression to delete the finds mapping after the find() function has been called. This ensures that the stdstore variable is reset and ready for the next call to find().

I have also added some unit tests to verify that the stdstore variable is being reset correctly. These tests can be found in the test/StdStorage.t.sol file.

Please review and let me know if there are any changes needed. Thank you!

kaihiroi avatar Dec 14 '22 11:12 kaihiroi

hi there! Thanks for the PR. Can you go into a little more detail why you want find to be removed after each use?

The whole reason the mapping is there is so that we don't add a bunch of unnecessary calls in the trace for subsequent reading of a particular slot.

A more concrete example of when you need the storage cleared would be helpful to evaluate if this is the correct solution :)

There are very few reasons a slot should change for a particular address and function signature, so any clarity on the use case here would be appreciated

brockelmore avatar Dec 16 '22 04:12 brockelmore

Closing as stale

mds1 avatar Apr 25 '24 12:04 mds1