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

Fix Shift Amount in Proxy #868

Merged
merged 4 commits into from
Dec 14, 2024
Merged

Fix Shift Amount in Proxy #868

merged 4 commits into from
Dec 14, 2024

Conversation

nlordell
Copy link
Collaborator

This PR fixes the shift amounts in the SafeProxy implementation. It also slightly optimizes how we do the masking by 2 instructions (removes the need to PUSH 0 and SHR).

I added some additional tests to make sure the masking works as expected and a comment explaining why we only mask for handling the masterCopy() call.

@nlordell nlordell requested review from a team, akshay-ap, mmv08 and remedcu and removed request for a team December 13, 2024 08:38
@nlordell
Copy link
Collaborator Author

nlordell commented Dec 13, 2024

@mmv08 - for some reason, the test is failing on zkSync. It looks like its related to some JS library issue, as there is a stack overflow happening (and looks to be caught in some infinite recursion).

I disabled the tests for now, but if you figure out what is going on feel free to enable them back in.

Figured it out, it looks like ContractFactory.deploy(...) doesn't resolve addresses in constructor args in the same way when zkSync is enabled :/

@nlordell nlordell merged commit 834e798 into main Dec 14, 2024
25 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants