Skip to content
33 changes: 17 additions & 16 deletions core/tests/loadnext/src/sdk/ethereum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,28 +407,29 @@ impl<S: EthereumSigner> EthereumProvider<S> {
.base_cost(gas_limit, L1_TO_L2_GAS_PER_PUBDATA, Some(gas_price))
.await
.map_err(|e| ClientError::NetworkError(e.to_string()))?;
let value = base_cost + operator_tip + l2_value;
let tx_data = self.client().encode_tx_data(
"requestL2Transaction",
(
contract_address,
l2_value,
calldata,
gas_limit,
U256::from(L1_TO_L2_GAS_PER_PUBDATA),
factory_deps,
refund_recipient,
)
.into_tokens(),
);
let mint_value = base_cost + operator_tip + l2_value;
let request_token = ethabi::Token::Tuple(vec![
ethabi::Token::Address(self.client().sender_account()),
ethabi::Token::Address(contract_address),
ethabi::Token::Uint(mint_value),
ethabi::Token::Uint(l2_value),
ethabi::Token::Bytes(calldata),
ethabi::Token::Uint(gas_limit),
ethabi::Token::Uint(U256::from(L1_TO_L2_GAS_PER_PUBDATA)),
ethabi::Token::Array(factory_deps.into_iter().map(ethabi::Token::Bytes).collect()),
ethabi::Token::Address(refund_recipient),
]);
let tx_data = self
.client()
.encode_tx_data("bridgehubRequestL2Transaction", vec![request_token]);

let tx = self
.client()
.sign_prepared_tx(
tx_data,
Options::with(|f| {
f.gas = Some(U256::from(300000));
f.value = Some(value);
f.value = Some(mint_value);
f.gas_price = Some(gas_price)
}),
Comment on lines 430 to 434
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The f.value field is missing in the Options struct. In the previous implementation, f.value was set to the total cost (base_cost + operator_tip + l2_value). Since this transaction initiates an L2 transaction that requires ETH for the base cost and value, omitting f.value will result in a msg.value of 0, which will cause the transaction to fail on-chain as the contract expects the sent value to match the mint_value provided in the request tuple.

Suggested change
Options::with(|f| {
f.gas = Some(U256::from(300000));
f.value = Some(value);
f.gas_price = Some(gas_price)
}),
Options::with(|f| {
f.gas = Some(U256::from(300000));
f.value = Some(mint_value);
f.gas_price = Some(gas_price)
}),

)
Expand Down Expand Up @@ -536,7 +537,7 @@ impl<S: EthereumSigner> EthereumProvider<S> {
)
.await?
} else {
// TODO(EVM-571): This should be moved to the shared bridge, and the `requestL2Transaction` method
// TODO(EVM-571): This should be moved to the shared bridge end-to-end for deposit flows
let bridge_address =
bridge_address.unwrap_or(self.default_bridges.l1_erc20_default_bridge.unwrap());
let contract_function = self
Expand Down
33 changes: 17 additions & 16 deletions core/tests/via_loadnext/src/sdk/ethereum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,28 +406,29 @@ impl<S: EthereumSigner> EthereumProvider<S> {
.base_cost(gas_limit, L1_TO_L2_GAS_PER_PUBDATA, Some(gas_price))
.await
.map_err(|e| ClientError::NetworkError(e.to_string()))?;
let value = base_cost + operator_tip + l2_value;
let tx_data = self.client().encode_tx_data(
"requestL2Transaction",
(
contract_address,
l2_value,
calldata,
gas_limit,
U256::from(L1_TO_L2_GAS_PER_PUBDATA),
factory_deps,
refund_recipient,
)
.into_tokens(),
);
let mint_value = base_cost + operator_tip + l2_value;
let request_token = ethabi::Token::Tuple(vec![
ethabi::Token::Address(self.client().sender_account()),
ethabi::Token::Address(contract_address),
ethabi::Token::Uint(mint_value),
ethabi::Token::Uint(l2_value),
ethabi::Token::Bytes(calldata),
ethabi::Token::Uint(gas_limit),
ethabi::Token::Uint(U256::from(L1_TO_L2_GAS_PER_PUBDATA)),
ethabi::Token::Array(factory_deps.into_iter().map(ethabi::Token::Bytes).collect()),
ethabi::Token::Address(refund_recipient),
]);
let tx_data = self
.client()
.encode_tx_data("bridgehubRequestL2Transaction", vec![request_token]);

let tx = self
.client()
.sign_prepared_tx(
tx_data,
Options::with(|f| {
f.gas = Some(U256::from(300000));
f.value = Some(value);
f.value = Some(mint_value);
f.gas_price = Some(gas_price)
}),
Comment on lines 429 to 433
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The f.value field is missing in the Options struct. In the previous implementation, f.value was set to the total cost (base_cost + operator_tip + l2_value). Since this transaction initiates an L2 transaction that requires ETH for the base cost and value, omitting f.value will result in a msg.value of 0, which will cause the transaction to fail on-chain as the contract expects the sent value to match the mint_value provided in the request tuple.

Suggested change
Options::with(|f| {
f.gas = Some(U256::from(300000));
f.value = Some(value);
f.gas_price = Some(gas_price)
}),
Options::with(|f| {
f.gas = Some(U256::from(300000));
f.value = Some(mint_value);
f.gas_price = Some(gas_price)
}),

)
Expand Down Expand Up @@ -535,7 +536,7 @@ impl<S: EthereumSigner> EthereumProvider<S> {
)
.await?
} else {
// TODO(EVM-571): This should be moved to the shared bridge, and the `requestL2Transaction` method
// TODO(EVM-571): This should be moved to the shared bridge end-to-end for deposit flows
let bridge_address =
bridge_address.unwrap_or(self.default_bridges.l1_erc20_default_bridge.unwrap());
let contract_function = self
Expand Down
10 changes: 8 additions & 2 deletions docs/src/guides/advanced/02_deposits.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,14 @@ Input: 0xeb672419000000000000000000000000618263ce921f7dd5f4f40c29f6c524aaf97b9bb
```

The deposit command has called the contract on address `0xa6B` (which is exactly the `CONTRACTS_DIAMOND_PROXY_ADDR` from
`deployL1.log`), and it has called the method `0xeb672419` - which is the `requestL2Transaction` from
[Mailbox.sol](https://github.com/matter-labs/era-contracts/blob/f06a58360a2b8e7129f64413998767ac169d1efd/ethereum/contracts/zksync/facets/Mailbox.sol#L220)
`deployL1.log`), and it has called the method `0xeb672419` - which is the legacy `requestL2Transaction` from
[Mailbox.sol](https://github.com/matter-labs/era-contracts/blob/f06a58360a2b8e7129f64413998767ac169d1efd/ethereum/contracts/zksync/facets/Mailbox.sol#L220).

Deprecation note: `requestL2Transaction` is deprecated. New integrations should use
`Bridgehub.requestL2TransactionDirect`.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The documentation mentions Bridgehub.requestL2TransactionDirect as the replacement, but the code changes in core/tests/loadnext/src/sdk/ethereum/mod.rs use the function name bridgehubRequestL2Transaction. This inconsistency can be confusing for developers. Please ensure the documentation accurately reflects the recommended function name or explains the relationship between these two (e.g., if one is a wrapper for the other).


Implementation note: some SDK/internal call paths expose this via a wrapper entrypoint
(e.g. `bridgehubRequestL2Transaction` on Hyperchain ABI), which ultimately maps to the Bridgehub flow.

#### Quick note on our L1 contracts

Expand Down
6 changes: 4 additions & 2 deletions docs/src/guides/advanced/03_withdrawals.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,10 @@ about the withdrawal.

### Final step - finalizing withdrawal

Now we're ready to actually claim our ETH on L1. We do this by calling a `finalizeEthWithdrawal` function on the
DiamondProxy contract (Mailbox.sol to be exact).
Now we're ready to actually claim our ETH on L1.

Historically this was done by calling `finalizeEthWithdrawal` on the DiamondProxy (Mailbox). Deprecation note:
`finalizeEthWithdrawal` is deprecated; new integrations should use `L1Nullifier.finalizeDeposit`.

To prove that we actually can withdraw the money, we have to say in which L2 block the withdrawal happened, and provide
the merkle proof from our withdrawal log, to the root that is stored in the L1 contract.
16 changes: 12 additions & 4 deletions docs/src/specs/l1_l2_communication/l1_to_l2.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,18 @@ between system and user logs.

### Initiation

A new priority operation can be appended by calling the
[requestL2Transaction](https://github.com/code-423n4/2023-10-zksync/blob/ef99273a8fdb19f5912ca38ba46d6bd02071363d/code/contracts/ethereum/contracts/zksync/facets/Mailbox.sol#L236)
method on L1. This method will perform several checks for the transaction, making sure that it is processable and
provides enough fee to compensate the operator for this transaction. Then, this transaction will be
Historically, a new priority operation was appended by calling
[`requestL2Transaction`](https://github.com/code-423n4/2023-10-zksync/blob/ef99273a8fdb19f5912ca38ba46d6bd02071363d/code/contracts/ethereum/contracts/zksync/facets/Mailbox.sol#L236)
on Mailbox.

Deprecation note: `requestL2Transaction` is deprecated. New integrations should use
`Bridgehub.requestL2TransactionDirect`.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The documentation mentions Bridgehub.requestL2TransactionDirect as the replacement, but the code changes in core/tests/loadnext/src/sdk/ethereum/mod.rs use the function name bridgehubRequestL2Transaction. This inconsistency can be confusing for developers. Please ensure the documentation accurately reflects the recommended function name or explains the relationship between these two.


Implementation note: some SDK/internal call paths expose this via wrapper entrypoints
(such as `bridgehubRequestL2Transaction` on Hyperchain ABI), while preserving the same Bridgehub migration intent.

This flow performs several checks for the transaction, making sure that it is processable and provides enough fee to
compensate the operator. Then, this transaction is
[appended](https://github.com/code-423n4/2023-10-zksync/blob/ef99273a8fdb19f5912ca38ba46d6bd02071363d/code/contracts/ethereum/contracts/zksync/facets/Mailbox.sol#L369C1-L369C1)
to the priority queue.

Expand Down
9 changes: 6 additions & 3 deletions docs/src/specs/l1_smart_contracts.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,12 @@ applying address aliasing leaves room for future EVM compatibility.

The L1 -> L2 communication is also used for bridging ether. The user should include a `msg.value` when initiating a
transaction request on the L1 contract. Before executing a transaction on L2, the specified address will be credited
with the funds. To withdraw funds user should call `withdraw` function on the `L2EtherToken` system contracts. This will
burn the funds on L2, allowing the user to reclaim them through the `finalizeEthWithdrawal` function on the
`MailboxFacet`.
with the funds. To withdraw funds user should call `withdraw` function on the `L2EtherToken` system contracts.

Deprecation note: `finalizeEthWithdrawal` on `MailboxFacet` is deprecated. The replacement flow is
`L1Nullifier.finalizeDeposit`.

This will burn the funds on L2, allowing the user to reclaim them on L1 through the finalize flow.

More about L1->L2 operations can be found
[here](https://github.com/code-423n4/2023-10-zksync/blob/main/docs/Smart%20contract%20Section/Handling%20L1→L2%20ops%20on%20zkSync.md).
Expand Down
12 changes: 10 additions & 2 deletions via_verifier/lib/via_da_client/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@ use anyhow::Context;
use byteorder::{BigEndian, ReadBytesExt};
use zksync_types::{u256_to_bytes_be, u256_to_h256, Address, H160, H256, U256};

/// The function selector used in L2 to compute the message.
pub const WITHDRAW_FUNC_SIG: &str = "finalizeEthWithdrawal(uint256,uint256,uint16,bytes,bytes32[])";
/// Legacy function selector used in L2 messages before Bridgehub/L1Nullifier migration.
pub const WITHDRAW_FUNC_SIG_LEGACY: &str =
"finalizeEthWithdrawal(uint256,uint256,uint16,bytes,bytes32[])";

/// New function selector expected after migrating to L1Nullifier finalize flow.
pub const WITHDRAW_FUNC_SIG_NULLIFIER: &str =
"finalizeDeposit(uint256,uint256,uint16,bytes,bytes32[])";

/// Supported L2 message function signatures during migration.
pub const WITHDRAW_FUNC_SIGS: [&str; 2] = [WITHDRAW_FUNC_SIG_LEGACY, WITHDRAW_FUNC_SIG_NULLIFIER];

/// The L2 BaseToken address.
pub const L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR: &str = "000000000000000000000000000000000000800a";
Expand Down
66 changes: 60 additions & 6 deletions via_verifier/lib/via_withdrawal_client/src/withdraw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use bitcoin::{
Address as BitcoinAddress, Amount, Network,
};
use ethers::abi::{decode, ParamType};
use via_da_client::types::WITHDRAW_FUNC_SIG;
use via_da_client::types::WITHDRAW_FUNC_SIGS;
use via_verifier_types::withdrawal::WithdrawalRequest;
use zksync_basic_types::{web3::keccak256, U256};
use zksync_types::{api::Log, Address};
Expand Down Expand Up @@ -35,7 +35,10 @@ pub fn parse_l2_withdrawal_message(
}

let func_selector_bytes = &l2_to_l1_message[0..4];
if func_selector_bytes != _get_withdraw_function_selector() {
if !_get_supported_withdraw_function_selectors()
.iter()
.any(|selector| func_selector_bytes == selector)
{
return Err(anyhow::format_err!("Invalid message function selector."));
}

Expand Down Expand Up @@ -112,10 +115,15 @@ pub fn parse_l2_withdrawal_message(
})
}

/// Get the withdrawal function selector.
fn _get_withdraw_function_selector() -> Vec<u8> {
let hash = keccak256(WITHDRAW_FUNC_SIG.as_bytes());
hash[0..4].to_vec()
/// Get all supported withdrawal function selectors.
fn _get_supported_withdraw_function_selectors() -> Vec<Vec<u8>> {
WITHDRAW_FUNC_SIGS
.iter()
.map(|sig| {
let hash = keccak256(sig.as_bytes());
hash[0..4].to_vec()
})
.collect()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function _get_supported_withdraw_function_selectors recomputes Keccak-256 hashes and allocates vectors on every call. Since this is used within the log parsing loop in parse_l2_withdrawal_message, it introduces unnecessary overhead. Consider pre-calculating these selectors as constants or using a lazy initialization pattern (e.g., once_cell) to improve performance.


#[cfg(test)]
Expand Down Expand Up @@ -215,4 +223,50 @@ mod tests {
assert_eq!(res.receiver, expected_receiver);
assert_eq!(res.amount, expected_amount);
}

#[test]
fn test_parse_l2_withdrawal_message_accepts_nullifier_selector() {
let btc_bytes = b"1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_vec();
let amount = U256::from("0000000000000000000000000000000000000000000000000de0b6b3a7640000");
let encoded_data = encode(&[Token::Bytes(btc_bytes.clone()), Token::Uint(amount.clone())]);
let data = Bytes::from(encoded_data);

let log = Log {
block_timestamp: None,
l1_batch_number: Some(U64::one()),
address: H160::random(),
topics: vec![
H256::from_str(
"0x2d6ef0fc97a54b2a96a5f3c96e3e69dca5b8d5ef4f68f01472c9e7c2b8d1f17b",
)
.unwrap(),
H256::from_str(
"0x000000000000000000000000aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
)
.unwrap(),
],
data,
block_hash: None,
block_number: Some(U64::one()),
transaction_hash: Some(H256::zero()),
transaction_index: None,
log_index: Some(U256::zero()),
transaction_log_index: Some(U256::zero()),
log_type: None,
removed: None,
};

let mut l2_to_l1_message = hex::decode("6c0960f93141317a5031655035514765666932444d505466544c35534c6d7637446976664e610000000000000000000000000000000000000000000000000de0b6b3a7640000").unwrap();
let supported_selectors = _get_supported_withdraw_function_selectors();
l2_to_l1_message[0..4].copy_from_slice(&supported_selectors[1]);

let expected_receiver = BitcoinAddress::from_str("1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa")
.unwrap()
.assume_checked();
let expected_amount = Amount::from_sat(1000000000000000000);
let res = parse_l2_withdrawal_message(l2_to_l1_message, log, Network::Bitcoin).unwrap();

assert_eq!(res.receiver, expected_receiver);
assert_eq!(res.amount, expected_amount);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The new test case test_parse_l2_withdrawal_message_accepts_nullifier_selector contains inconsistent amount data that will cause it to fail (as noted in the PR description regarding baseline failures).

Specifically, the l2_to_l1_message contains an unscaled amount ($10^{18}$), while the parser compares it against a scaled-down log_amount ($10^8$) at line 91. Furthermore, the expected_amount is set to $10^{18}$, which doesn't match the 8-decimal precision expected for Bitcoin amounts in this client.

To make the test pass and align with the intended protocol behavior, the amount in the hex message should be scaled to 8 decimals ($10^8$), and the expected_amount should be updated accordingly.

Suggested change
let mut l2_to_l1_message = hex::decode("6c0960f93141317a5031655035514765666932444d505466544c35534c6d7637446976664e610000000000000000000000000000000000000000000000000de0b6b3a7640000").unwrap();
let supported_selectors = _get_supported_withdraw_function_selectors();
l2_to_l1_message[0..4].copy_from_slice(&supported_selectors[1]);
let expected_receiver = BitcoinAddress::from_str("1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa")
.unwrap()
.assume_checked();
let expected_amount = Amount::from_sat(1000000000000000000);
let res = parse_l2_withdrawal_message(l2_to_l1_message, log, Network::Bitcoin).unwrap();
assert_eq!(res.receiver, expected_receiver);
assert_eq!(res.amount, expected_amount);
let mut l2_to_l1_message = hex::decode("6c0960f93141317a5031655035514765666932444d505466544c35534c6d7637446976664e610000000000000000000000000000000000000000000000000000000005f5e100").unwrap();
let supported_selectors = _get_supported_withdraw_function_selectors();
l2_to_l1_message[0..4].copy_from_slice(&supported_selectors[1]);
let expected_receiver = BitcoinAddress::from_str("1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa")
.unwrap()
.assume_checked();
let expected_amount = Amount::from_sat(100000000);
let res = parse_l2_withdrawal_message(l2_to_l1_message, log, Network::Bitcoin).unwrap();
assert_eq!(res.receiver, expected_receiver);
assert_eq!(res.amount, expected_amount);

}
}
Loading