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

feat: support permit2 on superchainweth #12596

Merged

Conversation

0xDiscotech
Copy link
Contributor

@0xDiscotech 0xDiscotech commented Oct 23, 2024

Description

Permit2 functionality added to the SuperchainWETH contract.

Changes made to WETH98 to support this:

  • Updated the visibility of the allowance mapping to internal and created a public getter.
  • Updated the visibility of the balanceOf mapping to internal and created a public getter for consistency.

In SuperchainWETH:

  • Override the allowance getter to return the maximum allowance when using Permit2.

Tests

New unit tests to cover the Permit2 integration with SuperchainWETH.

Additional context

The SuperchainWETH contract inherits from WETH98, which has allowance as a public mapping, making it non-overridable. Our approach was to follow OpenZeppelin's pattern by renaming allowance to _allowance and making it an internal state variable. This allowed us to add an allowance getter and make it virtual, enabling easy overriding. Some logic in this contract, as well as in DelayedWETH, had to be modified accordingly.
The change in balanceOf is only for consistency to follow the allowance pattern.

Metadata

#11899

@mslipper
Copy link
Collaborator

/ci authorize 5224eb9

@tynes
Copy link
Contributor

tynes commented Oct 24, 2024

Looks like a flake:

    sequencer_failover_setup.go:84: 
                Error Trace:    /var/opt/circleci/data/workdir/op-e2e/system/conductor/sequencer_failover_setup.go:84
                                                        /var/opt/circleci/data/workdir/op-e2e/system/conductor/sequencer_failover_test.go:19
                Error:          Received unexpected error:
                                leadership lost while committing log
                Test:           TestSequencerFailover_SetupCluster

@tynes tynes added this pull request to the merge queue Oct 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2024
@tynes tynes added this pull request to the merge queue Oct 24, 2024
Merged via the queue into ethereum-optimism:develop with commit 196613d Oct 24, 2024
49 checks passed
samlaf pushed a commit to samlaf/optimism that referenced this pull request Nov 10, 2024
* feat: support permit2 on superchainweth

* chore: run pre-pr

---------

Co-authored-by: agusduha <[email protected]>
Co-authored-by: gotzenx <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants