Skip to content

Commit

Permalink
chore: add fixes from reviews: rmv legacy code, modify interfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
@fedealconada
fedealconada committed Oct 2, 2024
1 parent bb00ffc commit dc65224
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 151 deletions.
1 change: 0 additions & 1 deletion .env.example
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ SOPH_TOKEN=0x06c03F9319EBbd84065336240dcc243bda9D8896
L1_USDC_TOKEN=0xBF4FdF7BF4014EA78C0A07259FBc4315Cb10d94E

# bridge addresses on ethereum sepolia
ERA_DIAMOND_PROXY=0x9A6DE0f62Aa270A8bCB1e2610078650D539B1Ef9
SEPOLIA_L1_BRIDGEHUB=0x35A54c8C757806eB6820629bc82d90E056394C92
SEPOLIA_SHARED_BRIDGE_L1=0x3E8b2fe58675126ed30d0d12dea2A9bda72D18Ae
SEPOLIA_CUSTOM_SHARED_BRIDGE_L1=0x3f842b5FaD08Bac49D0517C975d393f5f466Fd3b
Expand Down
8 changes: 2 additions & 6 deletions script/DeployL1SharedBridge.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,8 @@ contract DeployL1SharedBridge is Script {

vm.startBroadcast();

L1SharedBridge sharedBridgeImpl = new L1SharedBridge(
vm.envAddress("L1_USDC_TOKEN"),
IBridgehub(vm.envAddress("SEPOLIA_L1_BRIDGEHUB")),
vm.envUint("SOPHON_SEPOLIA_CHAIN_ID"),
vm.envAddress("ERA_DIAMOND_PROXY")
);
L1SharedBridge sharedBridgeImpl =
new L1SharedBridge(vm.envAddress("L1_USDC_TOKEN"), IBridgehub(vm.envAddress("SEPOLIA_L1_BRIDGEHUB")));

TransparentUpgradeableProxy sharedBridgeProxy = new TransparentUpgradeableProxy(
address(sharedBridgeImpl),
Expand Down
127 changes: 16 additions & 111 deletions src/L1SharedBridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

import {IL1ERC20Bridge} from "@era-contracts/l1-contracts/contracts/bridge/interfaces/IL1ERC20Bridge.sol";
import {IL1SharedBridge} from "@era-contracts/l1-contracts/contracts/bridge/interfaces/IL1SharedBridge.sol";
import {IL1SharedBridge} from "./interfaces/IL1SharedBridge.sol";
import {IL2Bridge} from "@era-contracts/l1-contracts/contracts/bridge/interfaces/IL2Bridge.sol";

import {IMailbox} from "@era-contracts/l1-contracts/contracts/state-transition/chain-interfaces/IMailbox.sol";
Expand Down Expand Up @@ -40,12 +40,6 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade
/// @dev Bridgehub smart contract that is used to operate with L2 via asynchronous L2 <-> L1 communication.
IBridgehub public immutable override BRIDGE_HUB;

/// @dev Era's chainID
uint256 public immutable ERA_CHAIN_ID;

/// @dev The address of zkSync Era diamond proxy contract.
address public immutable ERA_DIAMOND_PROXY;

/// @dev A mapping chainId => bridgeProxy. Used to store the bridge proxy's address, and to see if it has been deployed yet.
mapping(uint256 chainId => address l2Bridge) public override l2BridgeAddress;

Expand Down Expand Up @@ -80,15 +74,6 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade
_;
}

/// @notice Checks that the message sender is the bridgehub or zkSync Era Diamond Proxy.
modifier onlyBridgehubOrEra(uint256 _chainId) {
require(
msg.sender == address(BRIDGE_HUB) || (_chainId == ERA_CHAIN_ID && msg.sender == ERA_DIAMOND_PROXY),
"L1SharedBridge: not bridgehub or era chain"
);
_;
}

/// @notice Checks that the message sender is the shared bridge itself.
modifier onlySelf() {
require(msg.sender == address(this), "USDC-ShB not shared bridge");
Expand All @@ -103,14 +88,12 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade

/// @dev Contract is expected to be used as proxy implementation.
/// @dev Initialize the implementation to prevent Parity hack.
constructor(address _l1UsdcAddress, IBridgehub _bridgehub, uint256 _eraChainId, address _eraDiamondProxy)
constructor(address _l1UsdcAddress, IBridgehub _bridgehub)
reentrancyGuardInitializer
{
_disableInitializers();
L1_USDC_TOKEN = _l1UsdcAddress;
BRIDGE_HUB = _bridgehub;
ERA_CHAIN_ID = _eraChainId;
ERA_DIAMOND_PROXY = _eraDiamondProxy;
}

/// @dev Initializes a contract bridge for later use. Expected to be used in the proxy
Expand Down Expand Up @@ -196,7 +179,8 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade
whenNotPaused
returns (L2TransactionRequestTwoBridgesInner memory request)
{
require(l2BridgeAddress[_chainId] != address(0), "USDC-ShB l2 bridge not deployed");
address l2Bridge = l2BridgeAddress[_chainId];
require(l2Bridge != address(0), "USDC-ShB l2 bridge not deployed");

(address _l1Token, uint256 _depositAmount, address _l2Receiver) = abi.decode(_data, (address, uint256, address));
require(_l1Token == L1_USDC_TOKEN, "USDC-ShB: Only USDC deposits supported");
Expand All @@ -212,18 +196,18 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade
chainBalance[_chainId][_l1Token] += _depositAmount;
}

{
// Request the finalization of the deposit on the L2 side
bytes memory l2TxCalldata = _getDepositL2Calldata(_prevMsgSender, _l2Receiver, _l1Token, _depositAmount);

request = L2TransactionRequestTwoBridgesInner({
magicValue: TWO_BRIDGES_MAGIC_VALUE,
l2Contract: l2BridgeAddress[_chainId],
l2Calldata: l2TxCalldata,
factoryDeps: new bytes[](0),
txDataHash: txDataHash
});
}
// Request the finalization of the deposit on the L2 side
bytes memory l2TxCalldata = abi.encodeCall(
IL2Bridge.finalizeDeposit, (_prevMsgSender, _l2Receiver, _l1Token, _depositAmount, abi.encode("USD Coin", "USDC", 6))
);

request = L2TransactionRequestTwoBridgesInner({
magicValue: TWO_BRIDGES_MAGIC_VALUE,
l2Contract: l2Bridge,
l2Calldata: l2TxCalldata,
factoryDeps: new bytes[](0),
txDataHash: txDataHash
});

emit BridgehubDepositInitiated({
chainId: _chainId,
Expand All @@ -248,20 +232,6 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade
emit BridgehubDepositFinalized(_chainId, _txDataHash, _txHash);
}

/// @dev Generate a calldata for calling the deposit finalization on the L2 bridge contract
function _getDepositL2Calldata(address _l1Sender, address _l2Receiver, address _l1Token, uint256 _amount)
internal
view
returns (bytes memory)
{
(, bytes memory data1) = _l1Token.staticcall(abi.encodeCall(IERC20Metadata.name, ()));
(, bytes memory data2) = _l1Token.staticcall(abi.encodeCall(IERC20Metadata.symbol, ()));
(, bytes memory data3) = _l1Token.staticcall(abi.encodeCall(IERC20Metadata.decimals, ()));
return abi.encodeCall(
IL2Bridge.finalizeDeposit, (_l1Sender, _l2Receiver, _l1Token, _amount, abi.encode(data1, data2, data3))
);
}

/// @dev Withdraw funds from the initiated deposit, that failed when finalizing on L2
/// @param _depositSender The address of the deposit initiator
/// @param _l1Token The address of the deposited L1 ERC20 token
Expand Down Expand Up @@ -460,71 +430,6 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade
}
}

/*//////////////////////////////////////////////////////////////
UNUSED BUT REQUIRED BY INTERFACE
//////////////////////////////////////////////////////////////*/

/// @dev The address of the WETH token on L1.
address public immutable override L1_WETH_TOKEN;

/// @dev Legacy bridge smart contract that used to hold ERC20 tokens.
IL1ERC20Bridge public override legacyBridge; // unused but interface requires it

function setEraPostDiamondUpgradeFirstBatch(uint256) external pure {
return;
}

function setEraPostLegacyBridgeUpgradeFirstBatch(uint256) external pure {
return;
}

function setEraLegacyBridgeLastDepositTime(uint256, uint256) external pure {
return;
}

function bridgehubDepositBaseToken(uint256, address, address, uint256)
external
payable
virtual
{
revert("NOT_IMPLEMENTED");
}

/*//////////////////////////////////////////////////////////////
ERA LEGACY FUNCTIONS
//////////////////////////////////////////////////////////////*/

function depositLegacyErc20Bridge(address, address, address, uint256, uint256, uint256, address)
external
payable
override
returns (bytes32)
{
revert("NOT_IMPLEMENTED");
}

function finalizeWithdrawalLegacyErc20Bridge(uint256, uint256, uint16, bytes calldata, bytes32[] calldata)
external
pure
override
returns (address, address, uint256)
{
revert("NOT_IMPLEMENTED");
}

function claimFailedDepositLegacyErc20Bridge(
address,
address,
uint256,
bytes32,
uint256,
uint256,
uint16,
bytes32[] calldata
) external pure override {
revert("NOT_IMPLEMENTED");
}

/*//////////////////////////////////////////////////////////////
PAUSE
//////////////////////////////////////////////////////////////*/
Expand Down
12 changes: 1 addition & 11 deletions src/L2SharedBridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ contract L2SharedBridge is IL2SharedBridge, Initializable {

/// @dev Contract is expected to be used as proxy implementation.
/// @dev Disable the initialization to prevent Parity hack.
// uint256 immutable ERA_CHAIN_ID; // TODO: check if i need this!

/// @dev The address of the USDC token on L1.
address public immutable L1_USDC_TOKEN;
Expand All @@ -41,7 +40,6 @@ contract L2SharedBridge is IL2SharedBridge, Initializable {
address public immutable L2_USDC_TOKEN;

constructor(address _l1UsdcToken, address _l2UsdcToken) {
// ERA_CHAIN_ID = _eraChainId; // TODO: checke if we need this!
L1_USDC_TOKEN = _l1UsdcToken;
L2_USDC_TOKEN = _l2UsdcToken;
_disableInitializers();
Expand All @@ -50,7 +48,7 @@ contract L2SharedBridge is IL2SharedBridge, Initializable {
/// @notice Initializes the bridge contract for later use. Expected to be used in the proxy.
/// @param _l1SharedBridge The address of the L1 Bridge contract.
/// _l1Bridge The address of the legacy L1 Bridge contract.
function initialize(address _l1SharedBridge) external reinitializer(2) {
function initialize(address _l1SharedBridge) external reinitializer(1) {
require(_l1SharedBridge != address(0), "bf");
l1SharedBridge = _l1SharedBridge;
}
Expand Down Expand Up @@ -99,14 +97,6 @@ contract L2SharedBridge is IL2SharedBridge, Initializable {
return L2_USDC_TOKEN;
}

/*//////////////////////////////////////////////////////////////
UNUSED BUT REQUIRED BY INTERFACE
//////////////////////////////////////////////////////////////*/

/// @dev The address of the legacy L1 erc20 bridge counterpart.
/// This is non-zero only on Era, and should not be renamed for backward compatibility with the SDKs.
address public override l1Bridge;

/*//////////////////////////////////////////////////////////////
UTILS
//////////////////////////////////////////////////////////////*/
Expand Down
102 changes: 102 additions & 0 deletions src/interfaces/IL1SharedBridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,83 @@

pragma solidity 0.8.24;

import {IBridgehub, L2TransactionRequestTwoBridgesInner} from "@era-contracts/l1-contracts/contracts/bridgehub/IBridgehub.sol";
import {IL1ERC20Bridge} from "./IL1ERC20Bridge.sol";

/// @title L1 Bridge contract interface
/// @author Matter Labs
/// @custom:security-contact [email protected]
interface IL1SharedBridge {
/// @notice pendingAdmin is changed
/// @dev Also emitted when new admin is accepted and in this case, `newPendingAdmin` would be zero address
event NewPendingAdmin(address indexed oldPendingAdmin, address indexed newPendingAdmin);

/// @notice Admin changed
event NewAdmin(address indexed oldAdmin, address indexed newAdmin);

event LegacyDepositInitiated(
uint256 indexed chainId,
bytes32 indexed l2DepositTxHash,
address indexed from,
address to,
address l1Token,
uint256 amount
);

event BridgehubDepositInitiated(
uint256 indexed chainId,
bytes32 indexed txDataHash,
address indexed from,
address to,
address l1Token,
uint256 amount
);

event BridgehubDepositBaseTokenInitiated(
uint256 indexed chainId,
address indexed from,
address l1Token,
uint256 amount
);

event BridgehubDepositFinalized(
uint256 indexed chainId,
bytes32 indexed txDataHash,
bytes32 indexed l2DepositTxHash
);

event WithdrawalFinalizedSharedBridge(
uint256 indexed chainId,
address indexed to,
address indexed l1Token,
uint256 amount
);

event ClaimedFailedDepositSharedBridge(
uint256 indexed chainId,
address indexed to,
address indexed l1Token,
uint256 amount
);

function isWithdrawalFinalized(
uint256 _chainId,
uint256 _l2BatchNumber,
uint256 _l2MessageIndex
) external view returns (bool);

function claimFailedDeposit(
uint256 _chainId,
address _depositSender,
address _l1Token,
uint256 _amount,
bytes32 _l2TxHash,
uint256 _l2BatchNumber,
uint256 _l2MessageIndex,
uint16 _l2TxNumberInBatch,
bytes32[] calldata _merkleProof
) external;

function finalizeWithdrawal(
uint256 _chainId,
uint256 _l2BatchNumber,
Expand All @@ -14,4 +87,33 @@ interface IL1SharedBridge {
bytes calldata _message,
bytes32[] calldata _merkleProof
) external;

function BRIDGE_HUB() external view returns (IBridgehub);

function l2BridgeAddress(uint256 _chainId) external view returns (address);

function depositHappened(uint256 _chainId, bytes32 _l2TxHash) external view returns (bytes32);

/// data is abi encoded :
/// address _l1Token,
/// uint256 _amount,
/// address _l2Receiver
function bridgehubDeposit(
uint256 _chainId,
address _prevMsgSender,
uint256 _l2Value,
bytes calldata _data
) external payable returns (L2TransactionRequestTwoBridgesInner memory request);

function bridgehubConfirmL2Transaction(uint256 _chainId, bytes32 _txDataHash, bytes32 _txHash) external;

function receiveEth(uint256 _chainId) external payable;

/// @notice Starts the transfer of admin rights. Only the current admin can propose a new pending one.
/// @notice New admin can accept admin rights by calling `acceptAdmin` function.
/// @param _newPendingAdmin Address of the new admin
function setPendingAdmin(address _newPendingAdmin) external;

/// @notice Accepts transfer of admin rights. Only pending admin can accept the role.
function acceptAdmin() external;
}
2 changes: 0 additions & 2 deletions src/interfaces/IL2SharedBridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,5 @@ interface IL2SharedBridge {

function l2TokenAddress(address _l1Token) external view returns (address);

function l1Bridge() external view returns (address);

function l1SharedBridge() external view returns (address);
}
4 changes: 1 addition & 3 deletions test/e2e/SharedBridgesTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ contract SharedBridgesTest is Test {
// deploy implementation
L1SharedBridge sharedBridgeImpl = new L1SharedBridge{salt: "1"}(
vm.envAddress("L1_USDC_TOKEN"),
IBridgehub(vm.envAddress("SEPOLIA_L1_BRIDGEHUB")), // Sepolia L1 Bridgehub
vm.envUint("SOPHON_SEPOLIA_CHAIN_ID"), // Sophon chain ID
vm.envAddress("ERA_DIAMOND_PROXY") // Era Diamond Proxy);
IBridgehub(vm.envAddress("SEPOLIA_L1_BRIDGEHUB")) // Sepolia L1 Bridgehub
);

// deploy proxy
Expand Down
5 changes: 0 additions & 5 deletions test/l1ShareBridge/L1SharedBridgeFails.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest {
);
}

function test_bridgehubDepositBaseToken_NotImplemented() public {
vm.expectRevert("NOT_IMPLEMENTED");
sharedBridge.bridgehubDepositBaseToken(chainId, alice, address(token), 0);
}

function test_bridgehubDeposit_unsupportedErc() public {
vm.prank(bridgehubAddress);
vm.expectRevert("USDC-ShB: Only USDC deposits supported");
Expand Down
Loading

0 comments on commit dc65224

Please sign in to comment.