defi-wonderland / smock

The Solidity mocking library
MIT License
319 stars 40 forks source link

calling setVariable for booleans beside each other in storage fails to set the initial boolean #90

Open terencechow opened 2 years ago

terencechow commented 2 years ago

Describe the bug When a contract has 2 booleans beside each other, calling setVariable to each boolean will replace the mocking of the first.

Reproduction steps

  1. Deploy this contract:
contract Example {
  bool public _first;
  bool public _second;
}
  1. Call the below:
await contract.setVariable('_first', true);
await contract.setVariable('_second', true);
const _first = await contract._first()
assert(_first, true) # fails because _first returns false

Expected behavior We would expect _first to be true. However it is set to false.

System Specs:

Additional context I suspect this is a storage layout issue and the fact a bool is less than 1 word. For example putting a uint256 between the two booleans results in the above assertion to pass

contract Example {
  bool public _first;
  uint256 public intToSeparateWords;
  bool public _second;
}
smartcontracts commented 2 years ago

Whoops, I bet I know what's causing this. Thanks for the report! Will try to fix in the next few days :-)

smartcontracts commented 2 years ago

This is proving slightly more challenging to fix than I'd originally guessed. I need to do some refactoring to make this feasible (refactoring for the better, but still refactoring). It will likely take me until the end of the week to find the time to get this done and tested.

For some context, it is indeed a result of packed storage slots. Specifically it has to do with the fact that setVariable overwrites the entire storage slot instead of only overwriting the portion of the slot that should be modified when multiple variables are being packed. To fix this, I need to rewrite the storage logic so that each variable has a key/value/size/offset instead of just key/value, then I need to account for the size/offset when setting the storage slot such that I keep the portions of the old storage slot value.

wei3erHase commented 2 years ago

related to #117