Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

tests: L1StandardBridge#396

Closed
tynes wants to merge 17 commits intomainfrom
test/l1standardbridge
Closed

tests: L1StandardBridge#396
tynes wants to merge 17 commits intomainfrom
test/l1standardbridge

Conversation

@tynes
Copy link
Contributor

@tynes tynes commented May 2, 2022

Description
Adds tests for the L1StandardBridge contract. Adding these tests found a bug where
finalizeETHWithdrawal should be payable because the OptimismPortal needs to send
ETH to it to be forwarded to the end user.

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2022

Codecov Report

Merging #396 (d124ac5) into main (0c05488) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #396      +/-   ##
==========================================
- Coverage   39.70%   39.69%   -0.02%     
==========================================
  Files          50       50              
  Lines        6301     6301              
==========================================
- Hits         2502     2501       -1     
- Misses       3491     3492       +1     
  Partials      308      308              
Impacted Files Coverage Δ
opnode/rollup/driver/state.go 77.30% <0.00%> (-0.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c05488...d124ac5. Read the comment docs.

tynes added 17 commits May 4, 2022 09:40
The `L1StandardBridge` does not hold on to ETH balances,
the `OptimismPortal` does so. When depositing/withdrawing
ETH through the standard bridge, the `OptimismPortal` must
forward ETH to the `L1StandardBridge` for it to be forwarded
to the user. This means that the `finalizeETHWithdrawal`
function needs to be `payable`. This commit pulls in the
interfaces for the `IL1ERC20Bridge` and the `IL1StandardBridge`
contracts and adds a `payable` modifier to `finalizeETHWithdrawal`.
This is a diff from the previous deployment of the contracts,
but it does not impact the ABI so there is no diff for the
end user because this function is only callable by a smart contract.
When using the L1 bridge, the ETH deposit should
go through the L2 bridge.
@tynes tynes force-pushed the test/l1standardbridge branch from 804b866 to 067c60b Compare May 4, 2022 16:42
@tynes tynes mentioned this pull request May 11, 2022
@mslipper mslipper closed this May 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants