Skip to content

Commit

Permalink
Enforce solhint func-named-parameters (matter-labs#274)
Browse files Browse the repository at this point in the history
  • Loading branch information
dnkolegov authored and matzayonc committed Mar 27, 2024
1 parent f4a9656 commit 0ce3595
Show file tree
Hide file tree
Showing 34 changed files with 874 additions and 613 deletions.
1 change: 1 addition & 0 deletions .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"use-forbidden-name": "error",
"visibility-modifier-order": "error",
"reentrancy": "error",
"func-named-parameters": ["error", 5],
"compiler-version": ["error", "^0.8.0"]
}
}
47 changes: 27 additions & 20 deletions 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 Expand Up @@ -142,15 +149,15 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
uint256 amount = _depositFundsToSharedBridge(msg.sender, IERC20(_l1Token), _amount);
require(amount == _amount, "3T"); // The token has non-standard transfer logic

l2TxHash = sharedBridge.depositLegacyErc20Bridge{value: msg.value}(
msg.sender,
_l2Receiver,
_l1Token,
_amount,
_l2TxGasLimit,
_l2TxGasPerPubdataByte,
_refundRecipient
);
l2TxHash = sharedBridge.depositLegacyErc20Bridge{value: msg.value}({
_msgSender: msg.sender,
_l2Receiver: _l2Receiver,
_l1Token: _l1Token,
_amount: _amount,
_l2TxGasLimit: _l2TxGasLimit,
_l2TxGasPerPubdataByte: _l2TxGasPerPubdataByte,
_refundRecipient: _refundRecipient
});
depositAmount[msg.sender][_l1Token][l2TxHash] = _amount;
emit DepositInitiated(l2TxHash, msg.sender, _l2Receiver, _l1Token, _amount);
}
Expand Down Expand Up @@ -186,16 +193,16 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
require(amount != 0, "2T"); // empty deposit
delete depositAmount[_depositSender][_l1Token][_l2TxHash];

sharedBridge.claimFailedDepositLegacyErc20Bridge(
_depositSender,
_l1Token,
amount,
_l2TxHash,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_merkleProof
);
sharedBridge.claimFailedDepositLegacyErc20Bridge({
_depositSender: _depositSender,
_l1Token: _l1Token,
_amount: amount,
_l2TxHash: _l2TxHash,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_merkleProof: _merkleProof
});
emit ClaimedFailedDeposit(_depositSender, _l1Token, amount);
}

Expand Down
112 changes: 67 additions & 45 deletions l1-contracts/contracts/bridge/L1SharedBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
function bridgehubDeposit(
uint256 _chainId,
address _prevMsgSender,
uint256, // l2Value, needed for Weth deposits in the future
// solhint-disable-next-line no-unused-vars
uint256 _l2Value, // l2Value, needed for Weth deposits in the future
bytes calldata _data
) external payable override onlyBridgehub returns (L2TransactionRequestTwoBridgesInner memory request) {
require(l2BridgeAddress[_chainId] != address(0), "ShB l2 bridge not deployed");
Expand Down Expand Up @@ -224,7 +225,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 @@ -285,18 +293,18 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
uint16 _l2TxNumberInBatch,
bytes32[] calldata _merkleProof
) external override {
_claimFailedDeposit(
false,
_chainId,
_depositSender,
_l1Token,
_amount,
_l2TxHash,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_merkleProof
);
_claimFailedDeposit({
_checkedInLegacyBridge: false,
_chainId: _chainId,
_depositSender: _depositSender,
_l1Token: _l1Token,
_amount: _amount,
_l2TxHash: _l2TxHash,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_merkleProof: _merkleProof
});
}

/// @dev Processes claims of failed deposit, whether they originated from the legacy bridge or the current system.
Expand All @@ -313,15 +321,15 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
bytes32[] calldata _merkleProof
) internal nonReentrant {
{
bool proofValid = bridgehub.proveL1ToL2TransactionStatus(
_chainId,
_l2TxHash,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_merkleProof,
TxStatus.Failure
);
bool proofValid = bridgehub.proveL1ToL2TransactionStatus({
_chainId: _chainId,
_l2TxHash: _l2TxHash,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_merkleProof: _merkleProof,
_status: TxStatus.Failure
});
require(proofValid, "yn");
}
require(_amount > 0, "y1");
Expand Down Expand Up @@ -395,7 +403,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 +614,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 +641,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 All @@ -651,17 +673,17 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Initializable, Owna
uint16 _l2TxNumberInBatch,
bytes32[] calldata _merkleProof
) external override onlyLegacyBridge {
_claimFailedDeposit(
true,
ERA_CHAIN_ID,
_depositSender,
_l1Token,
_amount,
_l2TxHash,
_l2BatchNumber,
_l2MessageIndex,
_l2TxNumberInBatch,
_merkleProof
);
_claimFailedDeposit({
_checkedInLegacyBridge: true,
_chainId: ERA_CHAIN_ID,
_depositSender: _depositSender,
_l1Token: _l1Token,
_amount: _amount,
_l2TxHash: _l2TxHash,
_l2BatchNumber: _l2BatchNumber,
_l2MessageIndex: _l2MessageIndex,
_l2TxNumberInBatch: _l2TxNumberInBatch,
_merkleProof: _merkleProof
});
}
}
19 changes: 10 additions & 9 deletions l1-contracts/contracts/bridgehub/Bridgehub.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2Step {
uint256 _chainId,
address _stateTransitionManager,
address _baseToken,
uint256, //_salt
// solhint-disable-next-line no-unused-vars
uint256 _salt,
address _admin,
bytes calldata _initData
) external onlyOwnerOrAdmin nonReentrant returns (uint256 chainId) {
Expand Down Expand Up @@ -184,14 +185,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 @@ -65,6 +65,7 @@ library L2ContractHelper {
) internal pure returns (address) {
bytes32 senderBytes = bytes32(uint256(uint160(_sender)));
bytes32 data = keccak256(
// solhint-disable-next-line func-named-parameters
bytes.concat(CREATE2_PREFIX, senderBytes, _salt, _bytecodeHash, _constructorInputHash)
);

Expand Down
19 changes: 17 additions & 2 deletions l1-contracts/contracts/dev-contracts/test/ReenterL1ERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,25 @@ 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(address(0), address(0), bytes32(0), 0, 0, 0, merkleProof);
l1Erc20Bridge.claimFailedDeposit({
_depositSender: address(0),
_l1Token: address(0),
_l2TxHash: bytes32(0),
_l2BatchNumber: 0,
_l2MessageIndex: 0,
_l2TxNumberInBatch: 0,
_merkleProof: merkleProof
});
} else if (functionToCall == FunctionToCall.FinalizeWithdrawal) {
bytes32[] memory merkleProof;
l1Erc20Bridge.finalizeWithdrawal(0, 0, 0, bytes(""), merkleProof);
Expand Down
22 changes: 12 additions & 10 deletions l1-contracts/contracts/state-transition/StateTransitionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,17 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own
validatorTimelock = _initializeData.validatorTimelock;

// We need to initialize the state hash because it is used in the commitment of the next batch
IExecutor.StoredBatchInfo memory batchZero = IExecutor.StoredBatchInfo(
0,
_initializeData.genesisBatchHash,
_initializeData.genesisIndexRepeatedStorageChanges,
0,
EMPTY_STRING_KECCAK,
DEFAULT_L2_LOGS_TREE_ROOT_HASH,
0,
_initializeData.genesisBatchCommitment
);
IExecutor.StoredBatchInfo memory batchZero = IExecutor.StoredBatchInfo({
batchNumber: 0,
batchHash: _initializeData.genesisBatchHash,
indexRepeatedStorageChanges: _initializeData.genesisIndexRepeatedStorageChanges,
numberOfLayer1Txs: 0,
priorityOperationsHash: EMPTY_STRING_KECCAK,
l2LogsTreeRoot: DEFAULT_L2_LOGS_TREE_ROOT_HASH,
timestamp: 0,
commitment: _initializeData.genesisBatchCommitment
});

storedBatchZero = keccak256(abi.encode(batchZero));

initialCutHash = keccak256(abi.encode(_initializeData.diamondCut));
Expand Down Expand Up @@ -258,6 +259,7 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own
// construct init data
bytes memory initData;
/// all together 4+9*32=292 bytes
// solhint-disable-next-line func-named-parameters
initData = bytes.concat(
IDiamondInit.initialize.selector,
bytes32(_chainId),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,16 @@ contract ExecutorFacet is ZkSyncStateTransitionBase, IExecutor {
bytes32 commitment = _createBatchCommitment(_newBatch, logOutput.stateDiffHash, blobCommitments, blobHashes);

return
StoredBatchInfo(
_newBatch.batchNumber,
_newBatch.newStateRoot,
_newBatch.indexRepeatedStorageChanges,
_newBatch.numberOfLayer1Txs,
_newBatch.priorityOperationsHash,
logOutput.l2LogsTreeRoot,
_newBatch.timestamp,
commitment
);
StoredBatchInfo({
batchNumber: _newBatch.batchNumber,
batchHash: _newBatch.newStateRoot,
indexRepeatedStorageChanges: _newBatch.indexRepeatedStorageChanges,
numberOfLayer1Txs: _newBatch.numberOfLayer1Txs,
priorityOperationsHash: _newBatch.priorityOperationsHash,
l2LogsTreeRoot: logOutput.l2LogsTreeRoot,
timestamp: _newBatch.timestamp,
commitment: commitment
});
}

/// @notice checks that the timestamps of both the new batch and the new L2 block are correct.
Expand Down Expand Up @@ -514,6 +514,7 @@ contract ExecutorFacet is ZkSyncStateTransitionBase, IExecutor {

function _batchPassThroughData(CommitBatchInfo calldata _batch) internal pure returns (bytes memory) {
return
// solhint-disable-next-line func-named-parameters
abi.encodePacked(
_batch.indexRepeatedStorageChanges,
_batch.newStateRoot,
Expand All @@ -537,6 +538,7 @@ contract ExecutorFacet is ZkSyncStateTransitionBase, IExecutor {
bytes32 l2ToL1LogsHash = keccak256(_batch.systemLogs);

return
// solhint-disable-next-line func-named-parameters
abi.encode(
l2ToL1LogsHash,
_stateDiffHash,
Expand Down
Loading

0 comments on commit 0ce3595

Please sign in to comment.