-
Notifications
You must be signed in to change notification settings - Fork 158
♻️ Make DN404 zksync / abstract compatible #145
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
Conversation
cygaar
left a comment
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.
lgtm outside of the permit2 address
| /// @dev The ZKsync Permit2 deployment. | ||
| /// If deploying on ZKsync or Abstract, override `_isPermit2(address)` to check against this too. | ||
| /// [Etherscan](https://era.zksync.network/address/0x0000000000225e31D15943971F47aD3022F714Fa) | ||
| address internal constant _ZKSYNC_PERMIT_2 = 0x0000000000225e31D15943971F47aD3022F714Fa; |
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.
The abstract permit2 address is actually: 0x6b4420f66De496D18A6c36367cf7f1440fd9289f
Do you need it specifically at the address above?
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.
Oh my. fragmentation. Best to get it at the same address!
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.
It’s not yet up, https://explorer.testnet.abs.xyz/address/0x6b4420f66De496D18A6c36367cf7f1440fd9289f
so better fix now b4 mainnet.
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'll try to get it deployed to the same address
| /// @dev The ZKsync Permit2 deployment. | ||
| /// If deploying on ZKsync or Abstract, override `_isPermit2(address)` to check against this too. | ||
| /// [Etherscan](https://era.zksync.network/address/0x0000000000225e31D15943971F47aD3022F714Fa) | ||
| address internal constant _ZKSYNC_PERMIT_2 = 0x0000000000225e31D15943971F47aD3022F714Fa; |
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.
Same comment as above
|
Much thanks @cygaar @coffeexcoin |
Description
♻️
Checklist
Ensure you completed all of the steps below before submitting your pull request:
forge fmt?forge snapshot?forge test?Pull requests with an incomplete checklist will be thrown out.