Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Open
terencechow opened this issue Oct 9, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@terencechow
Copy link

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:

  • OS: mac os
  • Package Version (or commit hash): 2.0.7

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
Copy link
Contributor

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

@smartcontracts smartcontracts added the bug Something isn't working label Oct 9, 2021
@smartcontracts
Copy link
Contributor

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
Copy link
Member

related to #117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants