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

refactor: revert on l2factory #59

Merged
merged 3 commits into from
Jun 21, 2024
Merged

Conversation

0xDiscotech
Copy link
Contributor

🤖 Linear

Closes OPT-109

* chore: update errors

* test: update l2 factory unit tests based on changes
@0xDiscotech 0xDiscotech self-assigned this Jun 20, 2024
Copy link

linear bot commented Jun 20, 2024

OPT-109 Add back the reverts on the L2 Factory

Since we are going to deploy only a single L2 Factory per L2 deployments, we are safe by reverting and not tracking any other nonces.
If the deployment fail, then a re-deployment is needed.

Copy link
Contributor

@excaliborr excaliborr left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -174,14 +126,12 @@ contract L2OpUSDCFactory is IL2OpUSDCFactory {
* @param _initCode The creation bytecode.
* @return _newContract The 20-byte address where the contract was deployed.
*/
function _deployCreate(bytes memory _initCode) internal returns (address _newContract, bool _success) {
function _deployCreate(bytes memory _initCode) internal returns (address _newContract) {
assembly ("memory-safe") {
_newContract := create(0x0, add(_initCode, 0x20), mload(_initCode))
}
if (_newContract == address(0) || _newContract.code.length == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (_newContract == address(0) || _newContract.code.length == 0) {
if (uint160(_newContract) * _newContract.code.length == 0) {

Thanks Doc 🫶

Copy link
Member

Choose a reason for hiding this comment

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

Doesn´t save gas in this case XD

Copy link
Member

@hexshire hexshire left a comment

Choose a reason for hiding this comment

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

🚀

@0xDiscotech 0xDiscotech merged commit ad06052 into dev Jun 21, 2024
4 checks passed
@0xDiscotech 0xDiscotech deleted the refactor/revert-on-l2factory branch June 21, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants