Custom USDC bridge #1

fedealconada · 2024-09-18T09:19:40Z

We want to use the canonical zkSync Bridge with a custom bridge implementation that works only for USDC (since we want to use the native USDC and not the ERC20 that the zksync bridge deploys by default).

The PR is a little bit large but the key contracts to review are:

Bridge/Withdrawal flow

The flow to bridge and withdraw using custom bridges is the same as when bridging with any token except for the fact that when Bridgehub is called, you need specify the address of the custom L1 bridge.

To bridge USDC from L1 (Ethereum) -> L2 (Sophon):

Call bridgehub.requestL2TransactionTwoBridges (same as normally) but you set the secondBridgeAddress with the custom shared bridge deployed on L1. This way, the bridgehub contracts knows which contract to ping to.
requestL2TransactionTwoBridges makes a call to the custom shared bridge customBridgeL1.bridgehubDeposit function and transfers USDC from the user to this contract and emits an event.
sequencers will pick this event and automatically make a call to customL2Bridge.finalizeDeposit on the custom bridge deployed on L2 (on Sophon).
finalizeDeposit is the one that calls usdc.mint() to mint USDC on L2 (note this custom bridge must have MINTER role on the USDC contract).

To withdraw USDC from L2 (Sophon) -> L1 (Etheruem):

User makes a call to customBridgeL2.withdraw (same as normally except for the fact that you're calling the custom bridge contract)
Once the batch is sealed, user needs to call customL1Bridge.finalizeWithdrawal to finalise the withdrawal

Fixes #SOP-53
Fixes #SOP-54
Fixes #SOP-88
Fixes #SOP-89

linear · 2024-09-19T08:39:16Z

SOP-54 Create L2SharedBridge.sol

Create a custom L2SharedBridge.sol that works with the native USDC

SOP-53 Create L1SharedBridge.sol

Create a custom L1SharedBridge.sol that works with the USDC implementation

SOP-88 Create unit tests

federava

This looks great! The implementation is clear, and the test coverage is thorough.

src/L1SharedBridge.sol

test/l2SharedBridge/L2SharedBridgeBase.t.sol

src/L1SharedBridge.sol

kelemeno · 2024-09-24T20:11:17Z

src/L1SharedBridge.sol

-        bytes32 _txHash
-    ) external override onlyBridgehub whenNotPaused {
-        require(depositHappened[_chainId][_txHash] == 0x00, "ShB tx hap");
+    function bridgehubConfirmL2Transaction(uint256 _chainId, bytes32 _txDataHash, bytes32 _txHash)


I think a different linter is used, it's fine but it would be a bit better even for auditors to minimise the diff.

Could be, I'm just using foundry's fmt

src/L1SharedBridge.sol

kelemeno · 2024-09-24T20:14:08Z

src/L2SharedBridge.sol

+    address public immutable L2_USDC_TOKEN;
+
+    constructor(address _l1UsdcToken, address _l2UsdcToken) {
+        // ERA_CHAIN_ID = _eraChainId; // TODO: checke if we need this!


I don't think you need it

kelemeno · 2024-09-24T20:15:02Z

src/L2SharedBridge.sol

-        address _aliasedOwner
-    ) external reinitializer(2) {
+    /// _l1Bridge The address of the legacy L1 Bridge contract.
+    function initialize(address _l1SharedBridge) external reinitializer(2) {


You don't need reinitalizes(2). But its probably ok to have it (I'm not sure)

Will double check! Ty

kelemeno · 2024-09-24T20:19:45Z

Btw, which commit did you branch off from in era-contracts? It is the audited one right? Could you include it somewhere please?

src/L1SharedBridge.sol

kelemeno · 2024-09-24T20:21:37Z

src/L1SharedBridge.sol

+        view
+        returns (bytes memory)
+    {
+        (, bytes memory data1) = _l1Token.staticcall(abi.encodeCall(IERC20Metadata.name, ()));


This could be hardcoded, it is also good like this.

kelemeno · 2024-09-24T20:23:13Z

src/L2SharedBridge.sol

    /// @param _amount The total amount of tokens to be withdrawn
-    function withdraw(address _l1Receiver, address _l2Token, uint256 _amount) external override {
+    function withdraw(address _l1Receiver, address, uint256 _amount) external override {


Nit: If the token is specified, then it should be validated, so that users don't screw up.

the _l2Token param is just there to comply with the interface, but it's not used at all, you can send whatever address but the function will always use the USDC one

kelemeno · 2024-09-24T20:26:59Z

test/l2SharedBridge/L2SharedBridgeBase.t.sol

+import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
+import {IL2SharedBridge} from "../../src/interfaces/IL2SharedBridge.sol";
+
+contract MintableToken is MockERC20 {


I don't know how similar this is to the real USDC, it might be good enough, but it might make sense to test it with the full thing

Also testing on real env, dockerized node probably supports L1<>L2 bridging, or even the local setup

but it might make sense to test it with the full thing

what do you mean by the full thing @kelemeno ? i think we only need mint and burn to test

kelemeno · 2024-09-24T20:27:40Z

test/l2SharedBridge/L2SharedBridgeBase.t.sol

+    function testFinalizeDeposit() public {
+        uint256 initialBalance = mockL2Token.balanceOf(bob);
+        address aliasedL1SharedBridge =
+            address(uint160(l1SharedBridge) + uint160(0x1111000000000000000000000000000000001111));


you can use the applyAlias function from one of the libraries

kelemeno · 2024-09-24T20:28:48Z

test/l1ShareBridge/L1SharedBridgeHyperEnabled.t.sol

+import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR} from "@era-contracts/l1-contracts/contracts/common/L2ContractAddresses.sol";
+
+// note, this should be the same as where hyper is disabled
+contract L1SharedBridgeHyperEnabledTest is L1SharedBridgeTest {


This HyperEnabled was a mistake even in our repo, it was the same as the Base case, does it add to coverage or test anything in a different way?

i don't think so, but since you had it there, i moved those tests here as well in case there was something i was not seeing

kelemeno · 2024-09-24T20:31:16Z

src/interfaces/IL1ERC20Bridge.sol

+/// @author Matter Labs
+// note we use the IL1ERC20Bridge only to send L1<>L2 messages,
+// and we use this interface so that when the switch happened the old messages could be processed
+interface IL1ERC20Bridge {


This can probably be removed

kelemeno · 2024-09-24T20:36:07Z

Look good overall I think, the main is a direct clone of one of era_contracts repo right?

I would get rid of even more of the legacy code, I don't think you need it, modifying the interfaces is ok.

But the approach/security/code looks good!

fedealconada · 2024-09-25T07:57:58Z

Look good overall I think, the main is a direct clone of one of era_contracts repo right?

I would get rid of even more of the legacy code, I don't think you need it, modifying the interfaces is ok.

But the approach/security/code looks good!

Thanks for the comments!

fedealconada · 2024-10-02T07:52:29Z

I would get rid of even more of the legacy code, I don't think you need it, modifying the interfaces is ok.

would you point me to which are the other legacy things that we are safe to remove?

fedealconada · 2024-10-02T07:54:16Z

src/L1SharedBridge.sol

-        }
-    }
-
+    // TODO: do we need?


@kelemeno do we need this? or it's safe to be removed?

yes you can remove.

fedealconada · 2024-10-02T08:12:42Z

Btw, which commit did you branch off from in era-contracts? It is the audited one right? Could you include it somewhere please?

this is the commit we've forked from https://github.com/matter-labs/era-contracts.git#bce4b2d0f34bd87f1aaadd291772935afb1c3bd6

Add utils to save deployed addresses

fedealconada added 15 commits September 18, 2024 09:55

chore: modify L1SharedBridge.sol and L2SharedBridge.sol contracts

cefdeb1

chore: adjust forked L1 tests

b4169aa

chore: add deployment scripts

bc05068

chore: add other scripts

5ed6c5f

chore: fix imports and remove Counter

42ecd88

chore: update gitignore

a2f2ae5

chore: improve scripts

0d38cd6

chore: add README

a5b035b

chore: fix burn mechanism on L2SharedBridge

a169202

chore: clean up and fixes

1f46975

chore: update README

0838cc2

chore: fix build

a3e68ba

chore: remove unused tests

a7919a4

chore: polishes

a9b771c

chore: polish contracts and tests

60854d8

fedealconada force-pushed the custom-usdc-bridge branch 2 times, most recently from ca62b13 to 3f38ad5 Compare September 18, 2024 21:50

fedealconada added 3 commits September 19, 2024 09:29

chore: update addresses

caeb069

chore: fix finalize withdrawal script

9840995

chore: fix finalize withdrawal script II

a6c88ec

fedealconada force-pushed the custom-usdc-bridge branch from 3f38ad5 to a6c88ec Compare September 19, 2024 08:29

fedealconada requested review from RomanHiden, federava, lutr0 and CryptoMan0x September 19, 2024 08:37

chore: add unit tests for l2 bridge

bb00ffc

CryptoMan0x approved these changes Sep 23, 2024

View reviewed changes

federava reviewed Sep 24, 2024

View reviewed changes

src/L1SharedBridge.sol Outdated Show resolved Hide resolved

test/l2SharedBridge/L2SharedBridgeBase.t.sol Show resolved Hide resolved

kelemeno reviewed Sep 24, 2024

View reviewed changes

src/L1SharedBridge.sol Outdated Show resolved Hide resolved

kelemeno reviewed Sep 24, 2024

View reviewed changes

src/L1SharedBridge.sol Outdated Show resolved Hide resolved

kelemeno reviewed Sep 24, 2024

View reviewed changes

src/L1SharedBridge.sol Show resolved Hide resolved

kelemeno reviewed Sep 24, 2024

View reviewed changes

fedealconada commented Oct 2, 2024

View reviewed changes

chore: add fixes from reviews: rmv legacy code, modify interfaces

dc65224

fedealconada force-pushed the custom-usdc-bridge branch from fcc50ac to dc65224 Compare October 2, 2024 08:04

chore: update README

76f6c9c

fedealconada added 5 commits October 2, 2024 09:14

chore: add utils to save deployed addresses

80c6669

Merge pull request #2 from sophon-org/save-addresses

6e68ca5

Add utils to save deployed addresses

chore: rmv receiveEth function

4e06bbb

chore: update DeployL1SharedBridge script and upgrade

a3355b4

chore: update DeployL2SharedBridge script

425dc35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom USDC bridge #1

Custom USDC bridge #1

fedealconada commented Sep 18, 2024 •

edited

Loading

linear bot commented Sep 19, 2024

federava left a comment

kelemeno Sep 24, 2024

fedealconada Sep 25, 2024

kelemeno Sep 24, 2024

kelemeno Sep 24, 2024

fedealconada Sep 25, 2024

kelemeno commented Sep 24, 2024 •

edited

Loading

kelemeno Sep 24, 2024

fedealconada Sep 25, 2024

kelemeno Sep 24, 2024

fedealconada Oct 2, 2024

kelemeno Sep 24, 2024 •

edited

Loading

kelemeno Sep 24, 2024

fedealconada Oct 2, 2024 •

edited

Loading

kelemeno Sep 24, 2024

kelemeno Sep 24, 2024

fedealconada Oct 2, 2024

kelemeno Sep 24, 2024

kelemeno commented Sep 24, 2024

fedealconada commented Sep 25, 2024

fedealconada commented Oct 2, 2024

fedealconada Oct 2, 2024

kelemeno Oct 2, 2024

fedealconada commented Oct 2, 2024

Custom USDC bridge #1

Are you sure you want to change the base?

Custom USDC bridge #1

Conversation

fedealconada commented Sep 18, 2024 • edited Loading

linear bot commented Sep 19, 2024

federava left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kelemeno commented Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kelemeno Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedealconada Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kelemeno commented Sep 24, 2024

fedealconada commented Sep 25, 2024

fedealconada commented Oct 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedealconada commented Oct 2, 2024

fedealconada commented Sep 18, 2024 •

edited

Loading

kelemeno commented Sep 24, 2024 •

edited

Loading

kelemeno Sep 24, 2024 •

edited

Loading

fedealconada Oct 2, 2024 •

edited

Loading