Skip to content

Refactor and simplify token gateway contracts#733

Closed
maurelian wants to merge 6 commits intov0.3.0-rcfrom
refactor/622/gateway
Closed

Refactor and simplify token gateway contracts#733
maurelian wants to merge 6 commits intov0.3.0-rcfrom
refactor/622/gateway

Conversation

@maurelian
Copy link
Copy Markdown
Contributor

@maurelian maurelian commented May 3, 2021

  • add from and data args to L1ERC20Gateway
  • rename 'DepositedToken' to 'TokenGateway
  • add counterpartGateway() functions
  • add data to gateway events
  • rename deposit/withdraw gateway func to in/outbound
  • move L2 ERC20 out of TokenGateway
  • rename OVM_L2DepositedERC20 to OVM_L2TokenGateway
  • refactor OVM_ETH to continue being its own gateway

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented May 3, 2021

🦋 Changeset detected

Latest commit: dadac9b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/contracts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@maurelian maurelian force-pushed the refactor/622/gateway branch 3 times, most recently from 0df7c34 to 6b2f8e5 Compare May 4, 2021 00:33
@maurelian maurelian requested a review from ben-chain May 4, 2021 00:38
maurelian added 3 commits May 3, 2021 20:40
rename OVM_L2DepositedERC20 to OVM_L2TokenGateway
refactor OVM_ETH to continue being its own gateway

refactor(contracts): rename
'DepositedToken' to 'TokenGateway'
@maurelian maurelian force-pushed the refactor/622/gateway branch from 6b2f8e5 to eea2140 Compare May 4, 2021 00:43
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I liked this more generic naming.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
// We omit _data here because events only support bytes32 types.

That's not true.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This provide the option to bring your own token, or have one deployed for you.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think seeing Uniswap here is confusing. We might want to rename it and keep the impl, or even just capitulate and use an OZ impl (tho the one I added is ownable, so we do need a separate one).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since

  1. I removed the token from the gateway
  2. I didn't want to make this token ownable

I had to put the gateway into OVM_ETH.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Shoot, this is mostly just the above file renamed... looks like I forgot to git mv it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Names of the OZ internal variables.

@maurelian maurelian force-pushed the refactor/622/gateway branch from eea2140 to 1e61f6a Compare May 4, 2021 02:00
@maurelian maurelian force-pushed the refactor/622/gateway branch from 1e61f6a to 2b89353 Compare May 4, 2021 02:03
@maurelian
Copy link
Copy Markdown
Contributor Author

gonna break this up into small PRs, and build from master.

@maurelian maurelian closed this May 7, 2021
@maurelian maurelian deleted the refactor/622/gateway branch May 7, 2021 23:54
OptimismBot pushed a commit that referenced this pull request Dec 8, 2025
chore: sync sc-feat/opcm2-add-cgt with develop
theochap pushed a commit that referenced this pull request Dec 10, 2025
* fix(derive): Support modern action tests + holocene fixes

* update commit
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.

1 participant