Skip to content

Commit

Permalink
Fixes (#792)
Browse files Browse the repository at this point in the history
  • Loading branch information
@vladbochok
vladbochok authored Sep 11, 2024
1 parent 6de2963 commit 07d0462
Show file tree
Hide file tree
Showing 29 changed files with 216 additions and 198 deletions.
4 changes: 2 additions & 2 deletions l1-contracts/contracts/bridge/L1ERC20Bridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
message: _message,
merkleProof: _merkleProof
});
L1_NULLIFIER.finalizeWithdrawalLegacyContracts(finalizeWithdrawalParams);
L1_NULLIFIER.finalizeDeposit(finalizeWithdrawalParams);
}

/// @notice Initiates a deposit by locking funds on the contract and sending the request
Expand Down Expand Up @@ -195,7 +195,7 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
}

l2TxHash = L1_ASSET_ROUTER.depositLegacyErc20Bridge{value: msg.value}({
_prevMsgSender: msg.sender,
_originalCaller: msg.sender,
_l2Receiver: _l2Receiver,
_l1Token: _l1Token,
_amount: _amount,
Expand Down
109 changes: 56 additions & 53 deletions l1-contracts/contracts/bridge/L1Nullifier.sol
View file

Large diffs are not rendered by default.

11 changes: 3 additions & 8 deletions l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable,
emit AssetHandlerRegisteredInitial(assetId, _assetHandlerAddress, _assetRegistrationData, sender);
}

function _setAssetHandlerAddress(bytes32 _assetId, address _assetAddress) internal {
assetHandlerAddress[_assetId] = _assetAddress;
emit AssetHandlerRegistered(_assetId, _assetAddress);
}

/*//////////////////////////////////////////////////////////////
Receive transaction Functions
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -127,15 +122,15 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable,
/// @param _chainId The chain ID of the ZK chain to which to deposit.
/// @param _nextMsgValue The L2 `msg.value` from the L1 -> L2 deposit transaction.
/// @param _assetId The deposited asset ID.
/// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one.
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
/// @param _transferData The encoded data, which is used by the asset handler to determine L2 recipient and amount. Might include extra information.
/// @param _passValue Boolean indicating whether to pass msg.value in the call.
/// @return bridgeMintCalldata The calldata used by remote asset handler to mint tokens for recipient.
function _burn(
uint256 _chainId,
uint256 _nextMsgValue,
bytes32 _assetId,
address _prevMsgSender,
address _originalCaller,
bytes memory _transferData,
bool _passValue
) internal returns (bytes memory bridgeMintCalldata) {
Expand All @@ -149,7 +144,7 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable,
_chainId: _chainId,
_msgValue: _nextMsgValue,
_assetId: _assetId,
_prevMsgSender: _prevMsgSender,
_originalCaller: _originalCaller,
_data: _transferData
});
}
Expand Down
16 changes: 8 additions & 8 deletions l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ interface IL1AssetRouter is IAssetRouterBase {
/// of processing an L2 transaction where tokens would be minted.
/// @dev If the token is bridged for the first time, the L2 token contract will be deployed. Note however, that the
/// newly-deployed token does not support any custom logic, i.e. rebase tokens' functionality is not supported.
/// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one.
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
/// @param _l2Receiver The account address that should receive funds on L2.
/// @param _l1Token The L1 token address which is deposited.
/// @param _amount The total amount of tokens to be bridged.
Expand All @@ -62,7 +62,7 @@ interface IL1AssetRouter is IAssetRouterBase {
/// the funds would be lost.
/// @return txHash The L2 transaction hash of deposit finalization.
function depositLegacyErc20Bridge(
address _prevMsgSender,
address _originalCaller,
address _l2Receiver,
address _l1Token,
uint256 _amount,
Expand Down Expand Up @@ -97,8 +97,8 @@ interface IL1AssetRouter is IAssetRouterBase {
/// @notice Transfers funds to Native Token Vault, if the asset is registered with it. Does nothing for ETH or non-registered tokens.
/// @dev assetId is not the padded address, but the correct encoded id (NTV stores respective format for IDs)
/// @param _amount The asset amount to be transferred to native token vault.
/// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one.
function transferFundsToNTV(bytes32 _assetId, uint256 _amount, address _prevMsgSender) external returns (bool);
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
function transferFundsToNTV(bytes32 _assetId, uint256 _amount, address _originalCaller) external returns (bool);

/// @notice Finalize the withdrawal and release funds
/// @param _chainId The chain ID of the transaction to check
Expand All @@ -118,7 +118,7 @@ interface IL1AssetRouter is IAssetRouterBase {

/// @notice Initiates a transfer transaction within Bridgehub, used by `requestL2TransactionTwoBridges`.
/// @param _chainId The chain ID of the ZK chain to which deposit.
/// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one.
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
/// @param _value The `msg.value` on the target chain tx.
/// @param _data The calldata for the second bridge deposit.
/// @return request The data used by the bridgehub to create L2 transaction request to specific ZK chain.
Expand All @@ -131,7 +131,7 @@ interface IL1AssetRouter is IAssetRouterBase {
/// bytes _transferData
function bridgehubDeposit(
uint256 _chainId,
address _prevMsgSender,
address _originalCaller,
uint256 _value,
bytes calldata _data
) external payable returns (L2TransactionRequestTwoBridgesInner memory request);
Expand All @@ -152,12 +152,12 @@ interface IL1AssetRouter is IAssetRouterBase {
/// @dev If the corresponding L2 transaction fails, refunds are issued to a refund recipient on L2.
/// @param _chainId The chain ID of the ZK chain to which deposit.
/// @param _assetId The deposited asset ID.
/// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one.
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
/// @param _amount The total amount of tokens to be bridged.
function bridgehubDepositBaseToken(
uint256 _chainId,
bytes32 _assetId,
address _prevMsgSender,
address _originalCaller,
uint256 _amount
) external payable;

Expand Down
61 changes: 31 additions & 30 deletions l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {IAssetRouterBase, LEGACY_ENCODING_VERSION, NEW_ENCODING_VERSION, SET_ASS
import {AssetRouterBase} from "./AssetRouterBase.sol";

import {IL1AssetHandler} from "../interfaces/IL1AssetHandler.sol";
import {IL1ERC20Bridge} from "../interfaces/IL1ERC20Bridge.sol";
import {IAssetHandler} from "../interfaces/IAssetHandler.sol";
import {IL1Nullifier, FinalizeL1DepositParams} from "../interfaces/IL1Nullifier.sol";
import {INativeTokenVault} from "../ntv/INativeTokenVault.sol";
Expand Down Expand Up @@ -49,7 +50,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
INativeTokenVault public nativeTokenVault;

/// @dev Address of legacy bridge.
address public legacyBridge;
IL1ERC20Bridge public legacyBridge;

/// @notice Checks that the message sender is the nullifier.
modifier onlyNullifier() {
Expand Down Expand Up @@ -123,11 +124,11 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
/// @notice Sets the L1ERC20Bridge contract address.
/// @dev Should be called only once by the owner.
/// @param _legacyBridge The address of the legacy bridge.
function setL1Erc20Bridge(address _legacyBridge) external onlyOwner {
function setL1Erc20Bridge(IL1ERC20Bridge _legacyBridge) external onlyOwner {
if (address(legacyBridge) != address(0)) {
revert AddressAlreadyUsed(address(legacyBridge));
}
if (_legacyBridge == address(0)) {
if (address(_legacyBridge) == address(0)) {
revert ZeroAddress();
}
legacyBridge = _legacyBridge;
Expand Down Expand Up @@ -158,19 +159,20 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
/// @notice Used to set the asset handler address for a given asset ID on a remote ZK chain
/// @dev No access control on the caller, as msg.sender is encoded in the assetId.
/// @param _chainId The ZK chain ID.
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
/// @param _assetId The encoding of asset ID.
/// @param _assetHandlerAddressOnCounterpart The address of the asset handler, which will hold the token of interest.
/// @return request The tx request sent to the Bridgehub
function _setAssetHandlerAddressOnCounterpart(
uint256 _chainId,
address _prevMsgSender,
address _originalCaller,
bytes32 _assetId,
address _assetHandlerAddressOnCounterpart
) internal view returns (L2TransactionRequestTwoBridgesInner memory request) {
IL1AssetDeploymentTracker(assetDeploymentTracker[_assetId]).bridgeCheckCounterpartAddress(
_chainId,
_assetId,
_prevMsgSender,
_originalCaller,
_assetHandlerAddressOnCounterpart
);

Expand All @@ -195,7 +197,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
function bridgehubDepositBaseToken(
uint256 _chainId,
bytes32 _assetId,
address _prevMsgSender,
address _originalCaller,
uint256 _amount
) public payable virtual override onlyBridgehubOrEra(_chainId) whenNotPaused {
address assetHandler = assetHandlerAddress[_assetId];
Expand All @@ -208,18 +210,18 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
_chainId: _chainId,
_msgValue: 0,
_assetId: _assetId,
_prevMsgSender: _prevMsgSender,
_originalCaller: _originalCaller,
_data: abi.encode(_amount, address(0))
});

// Note that we don't save the deposited amount, as this is for the base token, which gets sent to the refundRecipient if the tx fails
emit BridgehubDepositBaseTokenInitiated(_chainId, _prevMsgSender, _assetId, _amount);
emit BridgehubDepositBaseTokenInitiated(_chainId, _originalCaller, _assetId, _amount);
}

/// @inheritdoc IL1AssetRouter
function bridgehubDeposit(
uint256 _chainId,
address _prevMsgSender,
address _originalCaller,
uint256 _value,
bytes calldata _data
)
Expand All @@ -241,14 +243,14 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
return
_setAssetHandlerAddressOnCounterpart(
_chainId,
_prevMsgSender,
_originalCaller,
_assetId,
_assetHandlerAddressOnCounterpart
);
} else if (encodingVersion == NEW_ENCODING_VERSION) {
(assetId, transferData) = abi.decode(_data[1:], (bytes32, bytes));
} else if (encodingVersion == LEGACY_ENCODING_VERSION) {
(assetId, transferData) = _handleLegacyData(_data, _prevMsgSender);
(assetId, transferData) = _handleLegacyData(_data, _originalCaller);
} else {
revert UnsupportedEncodingVersion();
}
Expand All @@ -261,21 +263,21 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
_chainId: _chainId,
_nextMsgValue: _value,
_assetId: assetId,
_prevMsgSender: _prevMsgSender,
_originalCaller: _originalCaller,
_transferData: transferData,
_passValue: true
});

bytes32 txDataHash = DataEncoding.encodeTxDataHash({
_nativeTokenVault: address(nativeTokenVault),
_encodingVersion: encodingVersion,
_prevMsgSender: _prevMsgSender,
_originalCaller: _originalCaller,
_assetId: assetId,
_transferData: transferData
});

request = _requestToBridge({
_prevMsgSender: _prevMsgSender,
_originalCaller: _originalCaller,
_assetId: assetId,
_bridgeMintCalldata: bridgeMintCalldata,
_txDataHash: txDataHash
Expand All @@ -284,7 +286,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
emit BridgehubDepositInitiated({
chainId: _chainId,
txDataHash: txDataHash,
from: _prevMsgSender,
from: _originalCaller,
assetId: assetId,
bridgeMintCalldata: bridgeMintCalldata
});
Expand Down Expand Up @@ -340,7 +342,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
/// @notice Decodes the transfer input for legacy data and transfers allowance to NTV.
/// @dev Is not applicable for custom asset handlers.
/// @param _data The encoded transfer data (address _l1Token, uint256 _depositAmount, address _l2Receiver).
// / @param _prevMsgSender The address of the deposit initiator.
/// @return Tuple of asset ID and encoded transfer data to conform with new encoding standard.
function _handleLegacyData(bytes calldata _data, address) internal returns (bytes32, bytes memory) {
(address _l1Token, uint256 _depositAmount, address _l2Receiver) = abi.decode(
Expand All @@ -366,7 +367,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
function transferFundsToNTV(
bytes32 _assetId,
uint256 _amount,
address _prevMsgSender
address _originalCaller
) external onlyNativeTokenVault returns (bool) {
address l1TokenAddress = INativeTokenVault(address(nativeTokenVault)).tokenAddress(_assetId);
if (l1TokenAddress == address(0) || l1TokenAddress == ETH_TOKEN_ADDRESS) {
Expand All @@ -377,29 +378,29 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
// Do the transfer if allowance to Shared bridge is bigger than amount
// And if there is not enough allowance for the NTV
if (
l1Token.allowance(_prevMsgSender, address(this)) >= _amount &&
l1Token.allowance(_prevMsgSender, address(nativeTokenVault)) < _amount
l1Token.allowance(_originalCaller, address(this)) >= _amount &&
l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount
) {
// slither-disable-next-line arbitrary-send-erc20
l1Token.safeTransferFrom(_prevMsgSender, address(nativeTokenVault), _amount);
l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount);
return true;
}
return false;
}

/// @dev The request data that is passed to the bridgehub.
/// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one.
/// @param _originalCaller The `msg.sender` address from the external call that initiated current one.
/// @param _assetId The deposited asset ID.
/// @param _bridgeMintCalldata The calldata used by remote asset handler to mint tokens for recipient.
/// @param _txDataHash The keccak256 hash of 0x01 || abi.encode(bytes32, bytes) to identify deposits.
/// @return request The data used by the bridgehub to create L2 transaction request to specific ZK chain.
function _requestToBridge(
address _prevMsgSender,
address _originalCaller,
bytes32 _assetId,
bytes memory _bridgeMintCalldata,
bytes32 _txDataHash
) internal view virtual returns (L2TransactionRequestTwoBridgesInner memory request) {
bytes memory l2TxCalldata = getDepositCalldata(_prevMsgSender, _assetId, _bridgeMintCalldata);
bytes memory l2TxCalldata = getDepositCalldata(_originalCaller, _assetId, _bridgeMintCalldata);

request = L2TransactionRequestTwoBridgesInner({
magicValue: TWO_BRIDGES_MAGIC_VALUE,
Expand Down Expand Up @@ -459,7 +460,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {

/// @inheritdoc IL1AssetRouter
function depositLegacyErc20Bridge(
address _prevMsgSender,
address _originalCaller,
address _l2Receiver,
address _l1Token,
uint256 _amount,
Expand All @@ -483,19 +484,19 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
_chainId: ERA_CHAIN_ID,
_nextMsgValue: 0,
_assetId: _assetId,
_prevMsgSender: _prevMsgSender,
_originalCaller: _originalCaller,
_transferData: abi.encode(_amount, _l2Receiver),
_passValue: false
});
}

{
bytes memory l2TxCalldata = getDepositCalldata(_prevMsgSender, _assetId, bridgeMintCalldata);
bytes memory l2TxCalldata = getDepositCalldata(_originalCaller, _assetId, bridgeMintCalldata);

// If the refund recipient is not specified, the refund will be sent to the sender of the transaction.
// Otherwise, the refund will be sent to the specified address.
// If the recipient is a contract on L1, the address alias will be applied.
address refundRecipient = AddressAliasHelper.actualRefundRecipient(_refundRecipient, _prevMsgSender);
address refundRecipient = AddressAliasHelper.actualRefundRecipient(_refundRecipient, _originalCaller);

L2TransactionRequestDirect memory request = L2TransactionRequestDirect({
chainId: ERA_CHAIN_ID,
Expand All @@ -514,14 +515,14 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
// Save the deposited amount to claim funds on L1 if the deposit failed on L2
L1_NULLIFIER.bridgehubConfirmL2TransactionForwarded(
ERA_CHAIN_ID,
keccak256(abi.encode(_prevMsgSender, _l1Token, _amount)),
keccak256(abi.encode(_originalCaller, _l1Token, _amount)),
txHash
);

emit LegacyDepositInitiated({
chainId: ERA_CHAIN_ID,
l2DepositTxHash: txHash,
from: _prevMsgSender,
from: _originalCaller,
to: _l2Receiver,
l1Asset: _l1Token,
amount: _amount
Expand Down Expand Up @@ -549,7 +550,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
message: _message,
merkleProof: _merkleProof
});
L1_NULLIFIER.finalizeWithdrawalLegacyContracts(finalizeWithdrawalParams);
L1_NULLIFIER.finalizeDeposit(finalizeWithdrawalParams);
}

/// @dev Withdraw funds from the initiated deposit, that failed when finalizing on L2.
Expand Down
Loading

0 comments on commit 07d0462

Please sign in to comment.