-
Notifications
You must be signed in to change notification settings - Fork 2
feat(cgt): custom gas token #478
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
Conversation
* feat: custom gas token l1 * fix: comments * fix: comments * fix: error * fix: comments * fix: natspec and tests --------- Co-authored-by: Ashitaka <[email protected]>
* feat: add NativeAssetLiquidity and LiquidityController predeploys initial version * fix: set fixed contract version and update interfaces * test: add NA & LC predeploys to L2Genesis script and test * test: add predeploys to setup * feat: add semver to NA & LC predeploys * test: add enableCustomGasToken setup * test: add native asset liquidity unit tests * test: add liquidity controller unit tests * fix: unused imports on test * refactor: custom gas token setup in L2Genesis * refactor: replace onlyMinter modifier in LiquidityController * refactor: add liquidity controller events and remove gasTokenDecimals * fix: remove unused stdStorage * refactor: make liquidity controller initializable * refactor: fix setLiquidityController safety invariant and remove unnecessary payable casts * docs: add natspec to liquidity controller event arguments * chore: fix predeploys version * feat: add burn function on NativeAssetLiquidity * test: assert burner balance is 0 * fix: replaced ownable with proxy admin owner * refactor: removed isCustomGasToken unnecessary check * fix: predeploys addresses * feat(cgt): poc custom gas token l2 l1block upgrade * feat: update l1block and weth for cgt * feat: add cgt check on initiateWithdrawal * test: add l1block unit tests * test: add l2ToL1MessagePasser unit tests * docs: add comments on L1Block test * refactor: leave cgt tests with uncategorized tag * refactor: rename error and fix unused import * test: move cgt related tests to separate files --------- Co-authored-by: niha <[email protected]>
|
I do not think you need to do anything based on my comments around the network upgrade transactions, looks like we will need to implement them for the upgrade. I can handle that outside of this PR |
This is WIP so far but meant to show how it can work. The big todo is to rename the feature flag with the name of the hardfork that the CGT upgrade is included in. This does leak knowledge of CGT into the client software so that it can optionally deploy the CGT predeploys. It is expected that the CGT predeploys will be in the genesis if the hardfork is active at genesis only on CGT chains. Relevant links: - ethereum-optimism/design-docs#305 - ethereum-optimism/design-docs#313 - defi-wonderland#478
* fix: var naming * fix: var naming
* fix: stack too deep * fix: scripts * fix: scripts
* fix(cgt): remove gasPayingToken function in L1Block
* refactor: move cgt flag to portal * test(cgt): add custom gas token enabled * chore(cgt): nit remove unused param * test(cgt): update initialize test * refactor(cgt): use stdstore
…imism into poc/custom-gas-token
* feat(cgt): add cgt flag check in finalizeWithdrawal
* feat: add withdrawal network type check on fee vaults deploy * test: add l2genesis revert test cases for invalid withdrawal network types
* feat: add deauthorizeMinter to liquidity controller * refactor: replace set minter to false for delete
* feat: add cgt config in op-deployer * feat: add cgt config in op-chain-ops * feat: add enable cgt in op-up & sysgo deployer * test: add isCustomGasToken tests for DeployOPChain * feat: add cgt integration script * chore(cgt): make l1 rpc ip static * feat: add cgt config to chain intent and fix deployer * test: add cgt tests in op deployer * test: add cgt test in intent builder * refactor: intent test * feat: add cgt standard values * fix: l2genesis override * refactor: remove config log and change builder test * feat: add cgt check in checkSystemConfig * fix: deploy opchain tests * chore: remove cgt demo config --------- Co-authored-by: hexshire <[email protected]>
| "test/L2/CrossDomainOwnable3.t.sol", // Contains contracts not matching CrossDomainOwnable3 base name | ||
| "test/L2/GasPriceOracle.t.sol", // Contains contracts not matching GasPriceOracle base name | ||
| "test/universal/StandardBridge.t.sol", // Contains contracts not matching StandardBridge base name | ||
| "test/L1/OptimismPortal2.t.sol", // Contains contracts not matching OptimismPortal2 base name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a different test file for CGT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPT-1057
| _setImplementationCode(Predeploys.SUPERCHAIN_TOKEN_BRIDGE); | ||
| } | ||
|
|
||
| /// @notice This predeploy is following the safety invariant #1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check invariant number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPT-1062
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The safety invariant # 1 is ok for this case. Ref.
| /// @custom:semver 4.6.1 | ||
| function version() public pure virtual returns (string memory) { | ||
| return "4.6.0"; | ||
| return "4.6.1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to 4.7.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPT-1061 - We should revisit other contracts too.
|
|
||
| /// @notice Returns whether the gas token is custom by reading from the OptimismPortal. | ||
| /// @return bool True if the gas token is custom, false otherwise. | ||
| function isCustomGasToken() public view returns (bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPT-1060
Co-authored-by: AgusDuha <[email protected]> Signed-off-by: Hex <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, please have a team admin upgrade your team to Bugbot Pro by visiting the Cursor dashboard. Your first 14 days will be free!
| is_ = false; | ||
| function gasPayingTokenSymbol() public view returns (string memory symbol_) { | ||
| symbol_ = | ||
| isCustomGasToken() ? ILiquidityController(Predeploys.LIQUIDITY_CONTROLLER).gasPayingTokenSymbol() : "ETH"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Custom Gas Token Functions Fail Prematurely
The gasPayingTokenName() and gasPayingTokenSymbol() functions can revert if setCustomGasToken() enables custom gas token mode when the LIQUIDITY_CONTROLLER predeploy isn't fully ready. These view functions then attempt to call an uninitialized or non-existent contract, leading to unexpected failures.
| "daBondSize": 1000, | ||
| "daResolverRefundPercentage": 50 | ||
| "daResolverRefundPercentage": 50, | ||
| "isCustomGasToken": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPT-1056
Closes: OPT-969