-
Notifications
You must be signed in to change notification settings - Fork 347
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: l2-native-token, bridge fixes #811
Conversation
…contracts into kl/l2-native-token
Changes to gas cost
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
…led deposits for bridged tokens, add forwarding function to L1AR for SDK
…-contracts into ra/l2-native
fix: Ra/l2 native
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.
Also, since you have a PR into zksync-era, maybe it makes sense to target sync-layer-stable
?
@@ -153,6 +150,11 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { | |||
} | |||
|
|||
function withdrawToken(address _l2NativeToken, bytes memory _assetData) public returns (bytes32) { | |||
bytes32 recordedAssetId = INativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR).assetId(_l2NativeToken); | |||
bytes32 recordedOriginChainId = INativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR).originChainId(recordedAssetId); | |||
if (recordedOriginChainId == L1_CHAIN_ID) { |
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 am not sure this is correct, the more correct check is to have != block.chainid
check
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.
No, because we will probably want to withdraw tokens that are native to other L2s, and those will be encoded using the new format
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.
We could use the simple withdraw()
for them, but this is also ok I think
What ❔
Why ❔
Checklist