Skip to content

Commit

Permalink
fix params
Browse files Browse the repository at this point in the history
  • Loading branch information
dnkolegov committed Mar 15, 2024
1 parent 75a090e commit 2074bf5
Show file tree
Hide file tree
Showing 18 changed files with 454 additions and 284 deletions.
2 changes: 1 addition & 1 deletion .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"use-forbidden-name": "error",
"visibility-modifier-order": "error",
"reentrancy": "error",
"func-named-parameters": ["error", 6],
"func-named-parameters": ["error", 5],
"compiler-version": ["error", "^0.8.0"]
}
}
9 changes: 8 additions & 1 deletion l1-contracts/contracts/bridge/L1ERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,14 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
uint256 _l2TxGasLimit,
uint256 _l2TxGasPerPubdataByte
) external payable returns (bytes32 l2TxHash) {
l2TxHash = deposit(_l2Receiver, _l1Token, _amount, _l2TxGasLimit, _l2TxGasPerPubdataByte, address(0));
l2TxHash = deposit({
_l2Receiver: _l2Receiver,
_l1Token: _l1Token,
_amount: _amount,
_l2TxGasLimit: _l2TxGasLimit,
_l2TxGasPerPubdataByte: _l2TxGasPerPubdataByte,
_refundRecipient: address(0)
});
}

/// @notice Initiates a deposit by locking funds on the contract and sending the request
Expand Down
43 changes: 32 additions & 11 deletions l1-contracts/contracts/bridge/L1SharedBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,14 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
txDataHash: txDataHash
});
}
emit BridgehubDepositInitiated(_chainId, txDataHash, _prevMsgSender, _l2Receiver, _l1Token, amount);
emit BridgehubDepositInitiated({
chainId: _chainId,
txDataHash: txDataHash,
from: _prevMsgSender,
to: _l2Receiver,
l1Token: _l1Token,
amount: amount
});
}

/// @notice Confirms the acceptance of a transaction by the Mailbox, as part of the L2 transaction process within Bridgehub.
Expand Down Expand Up @@ -395,7 +402,14 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
if (_isEraLegacyWithdrawal(_chainId, _l2BatchNumber)) {
require(!legacyBridge.isWithdrawalFinalized(_l2BatchNumber, _l2MessageIndex), "ShB: legacy withdrawal");
}
_finalizeWithdrawal(_chainId, _l2BatchNumber, _l2MessageIndex, _l2TxNumberInBatch, _message, _merkleProof);
_finalizeWithdrawal({
_chainId: _chainId,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_message: _message,
_merkleProof: _merkleProof
});
}

struct MessageParams {
Expand Down Expand Up @@ -599,7 +613,14 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
// Save the deposited amount to claim funds on L1 if the deposit failed on L2
depositHappened[ERA_CHAIN_ID][l2TxHash] = txDataHash;

emit LegacyDepositInitiated(ERA_CHAIN_ID, l2TxHash, _prevMsgSender, _l2Receiver, _l1Token, _amount);
emit LegacyDepositInitiated({
chainId: ERA_CHAIN_ID,
l2DepositTxHash: l2TxHash,
from: _prevMsgSender,
to: _l2Receiver,
l1Token: _l1Token,
amount: _amount
});
}

/// @notice Finalizes the withdrawal for transactions initiated via the legacy ERC20 bridge.
Expand All @@ -619,14 +640,14 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
bytes calldata _message,
bytes32[] calldata _merkleProof
) external override onlyLegacyBridge returns (address l1Receiver, address l1Token, uint256 amount) {
(l1Receiver, l1Token, amount) = _finalizeWithdrawal(
ERA_CHAIN_ID,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_message,
_merkleProof
);
(l1Receiver, l1Token, amount) = _finalizeWithdrawal({
_chainId: ERA_CHAIN_ID,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_message: _message,
_merkleProof: _merkleProof
});
}

/// @notice Withdraw funds from the initiated deposit, that failed when finalizing on zkSync Era chain.
Expand Down
16 changes: 8 additions & 8 deletions l1-contracts/contracts/bridgehub/Bridgehub.sol
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,14 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2Step {
) external view override returns (bool) {
address stateTransition = getStateTransition(_chainId);
return
IZkSyncStateTransition(stateTransition).proveL1ToL2TransactionStatus(
_l2TxHash,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_merkleProof,
_status
);
IZkSyncStateTransition(stateTransition).proveL1ToL2TransactionStatus({
_l2TxHash: _l2TxHash,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_merkleProof: _merkleProof,
_status: _status
});
}

/// @notice forwards function call to Mailbox based on ChainId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,14 @@ contract ReenterL1ERC20Bridge {
if (functionToCall == FunctionToCall.LegacyDeposit) {
l1Erc20Bridge.deposit(address(0), address(0), 0, 0, 0);
} else if (functionToCall == FunctionToCall.Deposit) {
l1Erc20Bridge.deposit(address(0), address(0), 0, 0, 0, address(0));
l1Erc20Bridge.deposit({
_l2Receiver: address(0),
_l1Token: address(0),
_amount: 0,
_l2TxGasLimit: 0,
_l2TxGasPerPubdataByte: 0,
_refundRecipient: address(0)
});
} else if (functionToCall == FunctionToCall.ClaimFailedDeposit) {
bytes32[] memory merkleProof;
l1Erc20Bridge.claimFailedDeposit({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ contract MailboxFacet is ZkSyncStateTransitionBase, IMailbox {
require(_batchNumber <= s.totalBatchesExecuted, "xx");

bytes32 hashedLog = keccak256(
// solhint-disable-next-line func-named-parameters
abi.encodePacked(_log.l2ShardId, _log.isService, _log.txNumberInBatch, _log.sender, _log.key, _log.value)
);
// Check that hashed log is not the default one,
Expand Down Expand Up @@ -181,14 +182,14 @@ contract MailboxFacet is ZkSyncStateTransitionBase, IMailbox {
bytes32[] calldata _merkleProof
) external nonReentrant {
require(s.chainId == ERA_CHAIN_ID, "finalizeEthWithdrawal only available for Era on mailbox");
IL1SharedBridge(s.baseTokenBridge).finalizeWithdrawal(
ERA_CHAIN_ID,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_message,
_merkleProof
);
IL1SharedBridge(s.baseTokenBridge).finalizeWithdrawal({
_chainId: ERA_CHAIN_ID,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_message: _message,
_merkleProof: _merkleProof
});
}

/// @inheritdoc IMailbox
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,14 @@ contract ExperimentalBridgeTest is Test {
if (randomCaller != deployerAddress && randomCaller != bridgeOwner) {
vm.prank(randomCaller);
vm.expectRevert(bytes("Bridgehub: not owner or admin"));
bridgeHub.createNewChain(chainId, address(mockSTM), address(testToken), uint256(123), admin, bytes(""));
bridgeHub.createNewChain({
_chainId: chainId,
_stateTransitionManager: address(mockSTM),
_baseToken: address(testToken),
_salt: uint256(123),
_admin: admin,
_initData: bytes("")
});
}

chainId = bound(chainId, 1, type(uint48).max);
Expand Down Expand Up @@ -374,14 +381,15 @@ contract ExperimentalBridgeTest is Test {
bytes("")
);

newChainId = bridgeHub.createNewChain(
chainId,
address(mockSTM),
address(testToken),
uint256(chainId * 2),
admin,
_newChainInitData
);
newChainId = bridgeHub.createNewChain({
_chainId: chainId,
_stateTransitionManager: address(mockSTM),
_baseToken: address(testToken),
_salt: uint256(chainId * 2),
_admin: admin,
_initData: _newChainInitData
});

vm.stopPrank();
vm.clearMockedCalls();

Expand Down Expand Up @@ -455,14 +463,14 @@ contract ExperimentalBridgeTest is Test {
assertTrue(bridgeHub.getStateTransition(mockChainId) == address(mockChainContract));

// Creating a random L2Log::l2Log so that we pass the correct parameters to `proveL2LogInclusion`
L2Log memory l2Log = _createMockL2Log(
randomL2ShardId,
randomIsService,
randomTxNumInBatch,
randomSender,
randomKey,
randomValue
);
L2Log memory l2Log = _createMockL2Log({
randomL2ShardId: randomL2ShardId,
randomIsService: randomIsService,
randomTxNumInBatch: randomTxNumInBatch,
randomSender: randomSender,
randomKey: randomKey,
randomValue: randomValue
});

// Since we have used random data for the `bridgeHub.proveL2LogInclusion` function which basically forwards the call
// to the same function in the mailbox, we will mock the call to the mailbox to return true and see if it works.
Expand Down Expand Up @@ -977,14 +985,14 @@ contract ExperimentalBridgeTest is Test {
bridgeHub.addStateTransitionManager(address(mockSTM));
vm.stopPrank();

L2Log memory l2Log = _createMockL2Log(
randomL2ShardId,
randomIsService,
randomTxNumInBatch,
randomSender,
randomKey,
randomValue
);
L2Log memory l2Log = _createMockL2Log({
randomL2ShardId: randomL2ShardId,
randomIsService: randomIsService,
randomTxNumInBatch: randomTxNumInBatch,
randomSender: randomSender,
randomKey: randomKey,
randomValue: randomValue
});

vm.mockCall(
address(bridgeHub),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ contract DepositTest is L1Erc20BridgeTest {

function test_RevertWhen_depositAmountIsZero() public {
vm.expectRevert(bytes("0T"));
bridge.deposit(randomSigner, address(token), 0, 0, 0, address(0));
bridge.deposit({
_l2Receiver: randomSigner,
_l1Token: address(token),
_l2TxGasLimit: 0,
_l2TxGasPerPubdataByte: 0,
_amount: 0,
_refundRecipient: address(0)
});
}

function test_RevertWhen_legacyDepositAmountIsZero() public {
Expand Down Expand Up @@ -68,7 +75,14 @@ contract DepositTest is L1Erc20BridgeTest {
vm.prank(alice);
vm.expectEmit(true, true, true, true, address(bridge));
emit DepositInitiated(dummyL2DepositTxHash, alice, randomSigner, address(token), amount);
bytes32 txHash = bridge.deposit(randomSigner, address(token), amount, 0, 0, address(0));
bytes32 txHash = bridge.deposit({
_l2Receiver: randomSigner,
_l1Token: address(token),
_amount: 0,
_l2TxGasLimit: 0,
_l2TxGasPerPubdataByte: 0,
_refundRecipient: address(0)
});
assertEq(txHash, dummyL2DepositTxHash);

uint256 depositedAmount = bridge.depositAmount(alice, address(token), dummyL2DepositTxHash);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ contract ReentrancyTest is L1Erc20BridgeTest {

vm.prank(alice);
vm.expectRevert(bytes("r1"));
bridgeReenterItself.deposit(randomSigner, address(token), amount, 0, 0, address(0));
bridgeReenterItself.deposit({
_l2Receiver: randomSigner,
_l1Token: address(token),
_amount: amount,
_l2TxGasLimit: 0,
_l2TxGasPerPubdataByte: 0,
_refundRecipient: address(0)
});
}

function _legacyDepositExpectRevertOnReentrancy() internal {
Expand Down
Loading

0 comments on commit 2074bf5

Please sign in to comment.