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

Improved state layout checks and add minor clarifications #105

Merged
merged 11 commits into from
Jul 3, 2023

Conversation

zerosnacks
Copy link
Contributor

@zerosnacks zerosnacks commented Jun 30, 2023

  • restores and improves state layout tests according to the updated diamond storage layout (works as expected!)

With the new tests we don't need to rely on a storage report generation feature because we explicitly test the locations of each storage slot with its expected behavior.

  • minor clarifications and safety assumptions

Comment on lines +251 to +260
vm.record();
dispatcher.getToken(tokenA_, address(dispatcher));
(reads, ) = vm.accesses(address(dispatcher));
assertEq((reads[0]), keccak256(abi.encode(tokenA_, uint256(IMPLEMENTATION_STORAGE_SLOT) + 5)));

vm.record();
dispatcher.getToken(tokenB_, address(dispatcher));
(reads, ) = vm.accesses(address(dispatcher));
assertEq((reads[0]), keccak256(abi.encode(tokenB_, uint256(IMPLEMENTATION_STORAGE_SLOT) + 5)));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently doesn't include a full 1:1 check for the struct, I don't think its worth the time or effort.

The base case of a struct in storage is handled above in ReflexState.relations

@zerosnacks zerosnacks merged commit 8453e8c into development Jul 3, 2023
5 checks passed
@zerosnacks zerosnacks deleted the feat/optimisations-and-improvements branch July 3, 2023 10:15
@zerosnacks zerosnacks restored the feat/optimisations-and-improvements branch July 3, 2023 11:07
@zerosnacks zerosnacks deleted the feat/optimisations-and-improvements branch July 3, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants