Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not recursive verifier #34

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 86 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,26 +1,105 @@
name: CI

on:
pull_request
on: pull_request

jobs:
lint:
runs-on: ubuntu-latest

defaults:
run:
working-directory: ethereum

steps:
- name: Checkout the repository
uses: actions/checkout@v3

- name: Use Node.js
uses: actions/setup-node@v3
with:
node-version: 16.15.1
cache: yarn
cache-dependency-path: ethereum/yarn.lock

- name: Install yarn
run: npm install -g yarn

- name: Install dependencies
run: yarn install

- name: Lint
run: yarn lint

build:
runs-on: ubuntu-latest

defaults:
run:
working-directory: ethereum

steps:
- name: Checkout the repository
uses: actions/checkout@v3

- name: Use Node.js
uses: actions/setup-node@v3
with:
node-version: 16.15.1
cache: yarn
cache-dependency-path: ethereum/yarn.lock

- name: Install yarn
run: npm install -g yarn

- name: Install dependencies
run: yarn install

- name: Build artifacts
run: yarn build

- name: Create cache
uses: actions/cache/save@v3
with:
key: artifacts-${{ github.sha }}
path: |
ethereum/artifacts
ethereum/cache
ethereum/typechain

test:
needs: [build, lint]
runs-on: ubuntu-latest

defaults:
run:
working-directory: ethereum

steps:
- uses: actions/checkout@v3
- name: Checkout the repository
uses: actions/checkout@v3

- name: Use Node.js
uses: actions/setup-node@v3
with:
node-version: '16.15.1'
node-version: 16.15.1
cache: yarn
cache-dependency-path: ethereum/yarn.lock

- name: Install yarn
run: npm install -g yarn

- name: Install dependencies
run: cd ethereum && yarn install
run: yarn install

- name: Restore artifacts cache
uses: actions/cache/restore@v3
with:
fail-on-cache-miss: true
key: artifacts-${{ github.sha }}
path: |
ethereum/artifacts
ethereum/cache
ethereum/typechain

- name: Run tests
working-directory: ethereum
run: yarn test
run: yarn test --no-compile
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.idea
tools/target
tools/data/Verifier.sol
9 changes: 9 additions & 0 deletions ethereum/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[*]
charset = utf-8
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true

[*.sol]
indent_style = space
indent_size = 4
16 changes: 16 additions & 0 deletions ethereum/.prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"plugins": ["prettier-plugin-solidity"],
"overrides": [
{
"files": "*.sol",
"options": {
"parser": "solidity-parse",
"printWidth": 120,
"tabWidth": 4,
"useTabs": false,
"singleQuote": false,
"bracketSpacing": false
}
}
]
}
21 changes: 15 additions & 6 deletions ethereum/.solhint.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
{
"extends": "solhint:default",
"plugins": ["prettier"],
"rules": {
"prettier/prettier": "error",
"no-inline-assembly": false
}
"extends": "solhint:recommended",
"plugins": ["prettier"],
"rules": {
"prettier/prettier": ["error"],
"func-visibility": ["error", { "ignoreConstructors": true }],
"compiler-version": ["error", ">=0.8.0"],
"max-line-length": ["error", 120],
"var-name-mixedcase": "off",
"func-name-mixedcase": "off",
"no-inline-assembly": "off",
"custom-errors": "off",
"no-global-import": "off",
"no-complex-fallback": "off",
"immutable-vars-naming": ["warn", { "immutablesAsConstants": false }]
}
}
7 changes: 7 additions & 0 deletions ethereum/.vscode/extensions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"recommendations": [
"editorconfig.editorconfig",
"esbenp.prettier-vscode",
"nomicfoundation.hardhat-solidity"
]
}
6 changes: 6 additions & 0 deletions ethereum/.vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.formatOnSave": true,
"prettier.documentSelectors": ["**/*.sol"],
"solidity.formatter": "prettier"
}
55 changes: 24 additions & 31 deletions ethereum/contracts/bridge/L1ERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
/// @notice At the time of the function call, it is not yet deployed in L2, but knowledge of its address
/// @notice is necessary for determining L2 token address by L1 address, see `l2TokenAddress(address)` function
/// @param _governor Address which can change L2 token implementation and upgrade the bridge
/// @param _deployBridgeImplementationFee How much of the sent value should be allocated to deploying the L2 bridge implementation
/// @param _deployBridgeImplementationFee How much of the sent value should be allocated to deploying the L2 bridge
/// implementation
/// @param _deployBridgeProxyFee How much of the sent value should be allocated to deploying the L2 bridge proxy
function initialize(
bytes[] calldata _factoryDeps,
Expand Down Expand Up @@ -123,7 +124,8 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
_deployBridgeProxyFee,
l2BridgeProxyBytecodeHash,
l2BridgeProxyConstructorData,
new bytes[](0) // No factory deps are needed for L2 bridge proxy, because it is already passed in previous step
// No factory deps are needed for L2 bridge proxy, because it is already passed in previous step
new bytes[](0)
);
}

Expand All @@ -136,7 +138,8 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
/// @param _l2TxGasLimit The L2 gas limit to be used in the corresponding L2 transaction
/// @param _l2TxGasPerPubdataByte The gasPerPubdataByteLimit to be used in the corresponding L2 transaction
/// @return l2TxHash The L2 transaction hash of deposit finalization
/// NOTE: the function doesn't use `nonreentrant` and `senderCanCallFunction` modifiers, because the inner method does.
/// NOTE: the function doesn't use `nonreentrant` and `senderCanCallFunction` modifiers, because the inner
/// method does.
function deposit(
address _l2Receiver,
address _l1Token,
Expand All @@ -156,14 +159,18 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
/// @param _l2TxGasPerPubdataByte The gasPerPubdataByteLimit to be used in the corresponding L2 transaction
/// @param _refundRecipient The address on L2 that will receive the refund for the transaction.
/// @dev If the L2 deposit finalization transaction fails, the `_refundRecipient` will receive the `_l2Value`.
/// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses out of control.
/// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses
/// out of control.
/// - If `_refundRecipient` is a contract on L1, the refund will be sent to the aliased `_refundRecipient`.
/// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will be sent to the `msg.sender` address.
/// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be sent to the aliased `msg.sender` address.
/// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds are controllable through the Mailbox,
/// since the Mailbox applies address aliasing to the from address for the L2 tx if the L1 msg.sender is a contract.
/// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx requests
/// through the Mailbox to use or withdraw the funds from L2, and the funds would be lost.
/// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will
/// be sent to the `msg.sender` address.
/// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be
/// sent to the aliased `msg.sender` address.
/// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds
/// are controllable through the Mailbox, since the Mailbox applies address aliasing to the from address for the
/// L2 tx if the L1 msg.sender is a contract. Without address aliasing for L1 contracts as refund recipients they
/// would not be able to make proper L2 tx requests through the Mailbox to use or withdraw the funds from L2, and
/// the funds would be lost.
/// @return l2TxHash The L2 transaction hash of deposit finalization
function deposit(
address _l2Receiver,
Expand Down Expand Up @@ -205,11 +212,7 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua

/// @dev Transfers tokens from the depositor address to the smart contract address
/// @return The difference between the contract balance before and after the transferring of funds
function _depositFunds(
address _from,
IERC20 _token,
uint256 _amount
) internal returns (uint256) {
function _depositFunds(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) {
uint256 balanceBefore = _token.balanceOf(address(this));
_token.safeTransferFrom(_from, address(this), _amount);
uint256 balanceAfter = _token.balanceOf(address(this));
Expand Down Expand Up @@ -316,17 +319,12 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
}

/// @dev Decode the withdraw message that came from L2
function _parseL2WithdrawalMessage(bytes memory _l2ToL1message)
internal
pure
returns (
address l1Receiver,
address l1Token,
uint256 amount
)
{
function _parseL2WithdrawalMessage(
bytes memory _l2ToL1message
) internal pure returns (address l1Receiver, address l1Token, uint256 amount) {
// Check that the message length is correct.
// It should be equal to the length of the function signature + address + address + uint256 = 4 + 20 + 20 + 32 = 76 (bytes).
// It should be equal to the length of the function signature + address + address + uint256 = 4 + 20 + 20 + 32 =
// 76 (bytes).
require(_l2ToL1message.length == 76, "kk");

(uint32 functionSignature, uint256 offset) = UnsafeBytes.readUint32(_l2ToL1message, 0);
Expand All @@ -338,12 +336,7 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
}

/// @dev Verify the deposit limit is reached to its cap or not
function _verifyDepositLimit(
address _l1Token,
address _depositor,
uint256 _amount,
bool _claiming
) internal {
function _verifyDepositLimit(address _l1Token, address _depositor, uint256 _amount, bool _claiming) internal {
IAllowList.Deposit memory limitData = IAllowList(allowList).getTokenDepositLimitData(_l1Token);
if (!limitData.depositLimitation) return; // no deposit limitation is placed for this token

Expand Down
50 changes: 28 additions & 22 deletions ethereum/contracts/bridge/L1WethBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,7 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {

/// @dev Contract is expected to be used as proxy implementation.
/// @dev Initialize the implementation to prevent Parity hack.
constructor(
address payable _l1WethAddress,
IZkSync _zkSync,
IAllowList _allowList
) reentrancyGuardInitializer {
constructor(address payable _l1WethAddress, IZkSync _zkSync, IAllowList _allowList) reentrancyGuardInitializer {
l1WethAddress = _l1WethAddress;
zkSync = _zkSync;
allowList = _allowList;
Expand All @@ -77,8 +73,10 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
/// @notice _factoryDeps[1] == a raw bytecode of proxy that is used as L2 WETH bridge
/// @param _l2WethAddress Pre-calculated address of L2 WETH token
/// @param _governor Address which can change L2 WETH token implementation and upgrade the bridge
/// @param _deployBridgeImplementationFee The fee that will be paid for the L1 -> L2 transaction for deploying L2 bridge implementation
/// @param _deployBridgeProxyFee The fee that will be paid for the L1 -> L2 transaction for deploying L2 bridge proxy
/// @param _deployBridgeImplementationFee The fee that will be paid for the L1 -> L2 transaction for deploying L2
/// bridge implementation
/// @param _deployBridgeProxyFee The fee that will be paid for the L1 -> L2 transaction for deploying L2 bridge
/// proxy
function initialize(
bytes[] calldata _factoryDeps,
address _l2WethAddress,
Expand Down Expand Up @@ -129,7 +127,8 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
_deployBridgeProxyFee,
l2WethBridgeProxyBytecodeHash,
l2WethBridgeProxyConstructorData,
new bytes[](0) // No factory deps are needed for L2 bridge proxy, because it is already passed in the previous step
// No factory deps are needed for L2 bridge proxy, because it is already passed in the previous step
new bytes[](0)
);
}

Expand All @@ -142,13 +141,18 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
/// @param _l2TxGasPerPubdataByte The gasPerPubdataByteLimit to be used in the corresponding L2 transaction
/// @param _refundRecipient The address on L2 that will receive the refund for the transaction.
/// @dev If the L2 deposit finalization transaction fails, the `_refundRecipient` will receive the `_l2Value`.
/// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses out of control.
/// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses
/// out of control.
/// - If `_refundRecipient` is a contract on L1, the refund will be sent to the aliased `_refundRecipient`.
/// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will be sent to the `msg.sender` address.
/// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be sent to the aliased `msg.sender` address.
/// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds are controllable through the Mailbox,
/// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will
/// be sent to the `msg.sender` address.
/// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be
/// sent to the aliased `msg.sender` address.
/// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds
/// are controllable through the Mailbox,
/// since the Mailbox applies address aliasing to the from address for the L2 tx if the L1 msg.sender is a contract.
/// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx requests
/// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx
/// requests
/// through the Mailbox to use or withdraw the funds from L2, and the funds would be lost.
/// @return txHash The L2 transaction hash of deposit finalization
function deposit(
Expand Down Expand Up @@ -204,7 +208,8 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
}

/// @notice Withdraw funds from the initiated deposit, that failed when finalizing on L2.
/// Note: Refund is performed by sending an equivalent amount of ETH on L2 to the specified deposit refund recipient address.
/// Note: Refund is performed by sending an equivalent amount of ETH on L2 to the specified deposit refund
/// recipient address.
function claimFailedDeposit(
address, // _depositSender,
address, // _l1Token,
Expand All @@ -219,7 +224,8 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {

/// @notice Finalize the withdrawal and release funds
/// @param _l2BlockNumber The L2 block number where the ETH (WETH) withdrawal was processed
/// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the ETH withdrawal message containing additional data about WETH withdrawal
/// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the ETH
/// withdrawal message containing additional data about WETH withdrawal
/// @param _l2TxNumberInBlock The L2 transaction number in a block, in which the ETH withdrawal log was sent
/// @param _message The L2 withdraw data, stored in an L2 -> L1 message
/// @param _merkleProof The Merkle proof of the inclusion L2 -> L1 message about withdrawal initialization
Expand Down Expand Up @@ -260,15 +266,15 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
emit WithdrawalFinalized(l1WethWithdrawReceiver, l1WethAddress, amount);
}

/// @dev Decode the ETH withdraw message with additional data about WETH withdrawal that came from L2EthToken contract
function _parseL2EthWithdrawalMessage(bytes memory _message)
internal
view
returns (address l1WethReceiver, uint256 ethAmount)
{
/// @dev Decode the ETH withdraw message with additional data about WETH withdrawal that came from L2EthToken
/// contract
function _parseL2EthWithdrawalMessage(
bytes memory _message
) internal view returns (address l1WethReceiver, uint256 ethAmount) {
// Check that the message length is correct.
// additionalData (WETH withdrawal data): l2 sender address + weth receiver address = 20 + 20 = 40 (bytes)
// It should be equal to the length of the function signature + eth receiver address + uint256 amount + additionalData = 4 + 20 + 32 + 40 = 96 (bytes).
// It should be equal to the length of the function signature + eth receiver address + uint256 amount +
// additionalData = 4 + 20 + 32 + 40 = 96 (bytes).
require(_message.length == 96, "Incorrect ETH message with additional data length");

(uint32 functionSignature, uint256 offset) = UnsafeBytes.readUint32(_message, 0);
Expand Down
Loading