Osaka hardfork #381
Osaka hardfork #381
Conversation
- Update 122 src/ files from =0.8.25 to =0.8.33 - Update test/unit/deployer/AltItoA.sol pragma - Update src/proxy/ERC1967UUPSUpgradeable.sol pragma This allows the full test suite to run with the Osaka EVM target. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AllowanceHolder.sol and the Deployer contracts (Deployer.sol, BlastDeployer.sol, ModeDeployer.sol) need to remain on 0.8.25 for deployment consistency while the rest of the codebase uses 0.8.33 for Osaka compatibility. Changes: - Use flexible pragma (^0.8.25) for these contracts so they compile with both 0.8.25 (for deployment) and 0.8.33 (for tests) - Update ERC1967UUPSUpgradeable.sol to ^0.8.25 since Deployer imports it - CI explicitly builds these with FOUNDRY_SOLC_VERSION=0.8.25 - Tests use vm.getDeployedCode() to load pre-compiled artifacts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… in deployment scripts
🛡️ Immunefi PR ReviewsWe noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below: Once submitted, we'll take care of assigning a reviewer and follow up here. |
There was a problem hiding this comment.
what are the sqrtPriceLimitX96 changes here for? 🤔
There was a problem hiding this comment.
The integration tests OOG with the Osaka-hardfork gas limit on the various tick-based AMMs we test against. Adding a meaningful sqrtPriceLimitX96 fixes this so that the tests can pass. It's infuriating.
| // Preserve the settlerMetaTxn address for the hardcoded signing hash. | ||
| new NonceBump(); |
There was a problem hiding this comment.
This is because of the removal of new AllowanceHolder() if I followed properly, is it too complex to just change the harcoded values? I would do that or modify the comment a bit, the "Preserve" makes sense now that you are doing the change but later on I think it is easy to forget why it needed to be preserved, I suggest just explaining that it needs to be bumped and maybe mention what is the expected nonce
There was a problem hiding this comment.
Correct.
We don't want to change the hardcoded value in test/integration/SettlerMetaTxnPairTest.t.sol (look for the giant warning) because it corresponds to an integration test elsewhere in 0x's repos. If we change the nonce of the test contract, we have to change the hardcoded hash. It's laborious to coordinate that update, so I would rather just bump the nonce here as a kludge. Do you have a suggestion for what the comment should say?
If we forget to preserve the nonce, there's a CI test that will fail. And if you paper that over while ignoring the giant warning, then whomever does that is beyond help 😓
There was a problem hiding this comment.
Got it. For me the comment should be something like a warning too
// Account nonce is relevant for upcoming settlerMetaTxn deployment to preserve
// the address used in hardcoded signing hash. Expected nonce is X.
// Dummy deployment to bump nonce to X
I fixed the CI. It should be good now. |
No description provided.