contracts-bedrock: port custom gas token to portal2

[tynes]

Description

Ports the custom gas token feature to OptimismPortal2. This will enable fault proofs to run on custom gas token chains.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 16.44%. Comparing base (822df50) to head (3d2d8dd). Report is 7 commits behind head on develop.

:exclamation: There is a different number of reports uploaded between BASE (822df50) and HEAD (3d2d8dd). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (822df50)	HEAD (3d2d8dd)
cannon-go-tests	1	0

Additional details and impacted files

@@             Coverage Diff              @@
##           develop   #10780       +/-   ##
============================================
- Coverage    60.69%   16.44%   -44.25%     
============================================
  Files           20        8       -12     
  Lines         1781      535     -1246     
  Branches        71       71               
============================================
- Hits          1081       88      -993     
+ Misses         667      447      -220     
+ Partials        33        0       -33     

Flag	Coverage Δ
cannon-go-tests	?
chain-mon-tests	27.14% <ø> (ø)
sdk-tests	16.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 12 files with indirect coverage changes

[tynes]

@coderabbitai summary

[coderabbitai[bot]]

Walkthrough

Walkthrough

The OptimismPortal2 contract has been significantly updated to improve its functionality and security. Key changes include the addition of new imports, the use of SafeERC20 for safer token interactions, and the introduction of new functions for managing balances, deposits, and gas payment tokens. Additionally, test files have been updated to cover these new functionalities, and deployment addresses have been modified.

Changes

File Path	Change Summary
packages/contracts-bedrock/src/L1/OptimismPortal2.sol	Added imports (SafeERC20, IERC20, L1Block, Predeploys), introduced SafeERC20 usage, new functions (balance, depositERC20Transaction, _depositTransaction, setGasPayingToken), updated version function, added SYSTEM_DEPOSIT_GAS_LIMIT constant, and _balance variable.
packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol	Added imports (VmSafe, Predeploys, L1Block, GasPayingToken, MockERC20), and new test functions for gas paying tokens, ERC20 deposits, and withdrawal finalization.
packages/contracts-bedrock/test/kontrol/proofs/utils/DeploymentSummaryFaultProofs.sol	Updated optimismPortal2Address constant to a new address.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant OptimismPortal2
    participant L2System

    User->>OptimismPortal2: depositERC20Transaction()
    OptimismPortal2->>OptimismPortal2: _depositTransaction()
    OptimismPortal2->>L2System: Forward transaction details
    L2System-->>OptimismPortal2: Acknowledge transaction
    OptimismPortal2-->>User: Confirm deposit

Possibly related issues

• ethereum-optimism/optimism#10592: The changes to OptimismPortal2 include the introduction of an overridable version() function and updates to ERC20 handling, which could be relevant to the ongoing updates tracked in this issue.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

[tynes]

@coderabbitai Double check that all of the tests from OptimismPortal.t.sol involving custom gas token have been ported to OptimismPortal2.t.sol

[coderabbitai[bot]]

[!TIP] For best results, initiate chat on the files or code changes.

@tynes, it appears that there are no test functions explicitly labeled with "gas token" in either OptimismPortal.t.sol or OptimismPortal2.t.sol. This could imply that the tests related to custom gas tokens might be described under different terminology or integrated into other test functions without explicit naming.

To ensure a thorough verification, I can extend the search to include broader terms related to the functionality of gas tokens, or examine the entire test files for related logic. Would you like me to proceed with this extended search or analyze the complete test files?

Analysis chain

Scripts executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Fetching test functions related to custom gas tokens from OptimismPortal.t.sol
rg --no-filename --no-heading "function test" packages/contracts-bedrock/test/L1/OptimismPortal.t.sol | grep -i "gas token"

# Fetching test functions related to custom gas tokens from OptimismPortal2.t.sol
rg --no-filename --no-heading "function test" packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol | grep -i "gas token"

Length of output: 252

[tynes]

@mds1 - I went through and made sure all of the tests were ported from the original implementation

[tynes]

I assume in a follow up PRs we will delete the original portal then rename portal2 to portal?

Yup, exactly!

Unfortunately I did not get a chance to double check this, but would just ask if we can get someone else to verify none are missing before merge. Worst case I can confirm once I'm back from vacation

Nobody has taken the time to do this review yet, so this is still pending. No major rush, this is nice to have to enable the interop devnet to run with the fault proof code, will rebase https://github.com/ethereum-optimism/optimism/pull/11051 on top of this after its merged on develop

[tynes]

@mds1 There has been no progress on this PR while you have been on vacation, but I did go through myself to double check that all the tests were ported and didn't find anything missing

[semgrep-app[bot]]

Semgrep found 2 todos_require_linear findings:

• packages/devnet-tasks/src/tasks/deposit-eth.ts
  • L415 - Triage
• packages/devnet-tasks/src/tasks/deposit-erc20.ts
  • L355 - Triage

Please create a GitHub ticket for this TODO.

_{Ignore this finding from todos_require_linear.}

[tynes]

@mds1 bump on this, I know you are busy but I do think this is good to merge. It will unblock me from doing another custom gas token release