-
Notifications
You must be signed in to change notification settings - Fork 989
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
Add compilation restrictions #921
base: main
Are you sure you want to change the base?
Conversation
c9ccf1d
to
85f1e9e
Compare
Does this help with #918? |
no, this PR focuses on the reduction of compilation times |
@ reviewers please double check the bytecode! |
If we merge this, we can't update v4-periphery to point to the new main branch because I'm pretty sure |
This seems like a problem to me? |
manager = new PoolManager(address(this)); | ||
function deployFreshManager() internal virtual returns (IPoolManager manager_) { | ||
bytes memory bytecode = | ||
abi.encodePacked(vm.getCode("out/PoolManager.sol/PoolManager.default.json"), abi.encode(address(this))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why cant you do it as it was before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it's somewhat of a problem. But we can probably also migrate periphery to use briefcase instead of v4 directly?
If we deploy the pool manager using the new
keyword instead of assembly, then the tests run against a version of the pool that is compiled without IR, so the bytecode of the pool manager will be different.
No description provided.