feat: add ownable liquidity#671
Conversation
There was a problem hiding this comment.
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, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Bug: Liquidity Default Blocks Max Value.
The default value for NativeAssetLiquidityAmount in defaultOverrides() is set to a non-nil pointer to zero ((*hexutil.Big)(big.NewInt(0))). When custom gas token is enabled via overrides without explicitly specifying liquidity amount, this zero pointer gets assigned to the intent, causing GetInitialLiquidity() to return 0 instead of the intended default of type(uint248).max. The default should be nil to trigger the type(uint248).max fallback in GetInitialLiquidity() when CGT is enabled.
op-deployer/pkg/deployer/pipeline/l2genesis.go#L209-L210
optimism/op-deployer/pkg/deployer/pipeline/l2genesis.go
Lines 209 to 210 in 18ae9d8
There was a problem hiding this comment.
Bug: Configuration Overrides Silently Ignored
The l2GenesisOverrides struct is missing a LiquidityControllerOwner field. When users provide liquidityControllerOwner in GlobalDeployOverrides (as shown in test case at l2genesis_test.go:235), the JSON merge in calculateL2GenesisOverrides silently ignores it since the field doesn't exist. This forces LiquidityControllerOwner to always default to L2ProxyAdminOwner when using overrides, even when users explicitly specify a different owner, creating an unexpected configuration gap.
op-deployer/pkg/deployer/pipeline/l2genesis.go#L23-L42
optimism/op-deployer/pkg/deployer/pipeline/l2genesis.go
Lines 23 to 42 in ba94130
* feat(cgt): custom gas token Signed-off-by: Hex <165055168+hexshire@users.noreply.github.com> Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: Ashitaka <96790496+ashitakah@users.noreply.github.com> Co-authored-by: AgusDuha <81362284+agusduha@users.noreply.github.com> * feat: add gasPayingTokenName and gasPayingTokenSymbol on json config files * fix: contracts semver * fix: remove system config bool * test: add OptimismPortal2CGT tests * chore(cgt): set cgt flag l1block & fixes * fix(linter): resolve goimports formatting issue * feat: add separate l2 contracts for cgt (#530) * feat: add L1BlockCGT * feat: add L2ToL1MessagePasserCGT * chore: remove test exclution in test validation * test(cgt): fix failing tests (#529) * test(cgt): fix failing tests * test(cgt): fix portal version * test(cgt): skip tests on forked mode * fix: predeploys test cgt mismatch * test(cgt): minor fixes --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> * fix(cgt): revert weth * cgt: feature flag integration Updates the implementation to use the new feature flagging system * config: make backwards compatible Use the legacy config name so that less needs to change for legacy CGT chains * fix: build issue Remove leftover merge conflict * fix: better backwards compatibility * build: fix * lint: fix * snapshots: update * op-deployer: apply test with CGT Add a specific test for CGT in the intent * contracts-bedrock: fix versioning * lint: fix * deployer: remove standard values * contracts: semver lock * contracts-bedrock: fix semver + abis * cgt: solidity test cleanup * solidity: fmt * tests: remove dead imports * tests: fixup * cgt: configurable liquidity amount * feat: configurable native asset liquidity balance Fix the build for this feature * cleanup: merge L1Block logic so that we inherit Keeps it simple because development on Jovian is happening and we will inherit any Jovian modifications automatically * snapshots: update * cgt: inherit logic for L2ToL1MessagePasser Generally simplifies the code * semver-lock: update * lint: fix * lint: fixup * interfaces: fix * fixes: smol * deploy-config: sane default * lint: fix * linting: fix * lint: fix * semgrep: fix * lint: fix * tests: fix * tests: fix fuzz * fix: custom gas token rebase (ethereum-optimism#17484) * fix: import in OptimismPortal2CGT test and pre-pr * fix(cgt): add missing native asset amount (#543) * fix: upgrade contract name * feat: add nativeAssetLiquidityAmount to config json files * fix: add correct nativeAssetLiquidityAmount in op-deployer * fix: add correct nativeAssetLiquidityAmount in op-e2e and fix withCustomGasToken argument on op-devstack * fix: restore OptimismPortal2CGT * fix: all comments * fix: comments * fix: governace * fix: remove aux * fix: remove aux * fix: deploy cgt * fix: revert cgt deploy config * fix: deploy config * chore: run linter * fix: remove unnecessary GetNativeAssetLiquidityAmount --------- Co-authored-by: Ashitaka <ashitaka@defi.sucks> Co-authored-by: agusduha <agusnduha@gmail.com> Co-authored-by: hexshire <hex@wonderland.xyz> * fix: l2 genesis pipeline (#554) * fix: add nativeAssetLiquidityAmount in e2e apply test (#555) * fix: failing test --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: Ashitaka <ashitaka@defi.sucks> Co-authored-by: hexshire <hex@wonderland.xyz> Co-authored-by: Hex <165055168+hexshire@users.noreply.github.com> * fix: semver lock * fix: CGT review fixes (ethereum-optimism#17534) * fix: json default zero * feat(cgt): add native asset liquidity amount test & omitempty on cgt struct * test: check native asset liquidity amount is correctly configured * feat: add omitempty to name and symbol on cgt intent struct * fix: check value in cgt * feat(cgt): add cgt dev feature * feat: add cgt dev feature * refactor: set custom gas token (#563) * feat: add cgt devFeatures and remove useCustomGasToken from DeployOPChainInput --------- Co-authored-by: Hex <165055168+hexshire@users.noreply.github.com> * fix: ir informational * fix: informational * fix: legacy tag * feat(cgt): add setNativeAssetLiquidityAmount & L2Genesis max amount check * feat: add setNativeAssetLiquidityAmount & L2Genesis max amount check * test: fix bound param in liquidity controller * fix: remove unused import * refactor: move max amount check inside setNativeAssetLiquidity * refactor: make chainIntent.CustomGasToken a non-pointer (#574) * refactor: make chainIntent.CustomGasToken a non-pointer * test: fix custom gas token text (#576) --------- Co-authored-by: Hex <165055168+hexshire@users.noreply.github.com> * fix: undo tests Co-authored-by: hexshire <hex@wonderland.xyz> * fix: cgt portal (#577) * fix: semver * fix: cgt tests coverage (#582) * fix: l2genesis expectRevert amount test * test: improve l1block coverage * test: improve l1blockcgt coverage * test: add isCustomGasToken tests on SystemConfig * test: improve L2ToL1MessagePasserCGT coverage * test: improve L2ToL1MessagePasser coverage * fix: naming pre-pr * fix(cgt): change L2ToL1MessagePasser & OPContractsManager semver (#584) * fix: change L2ToL1MessagePasser semver * fix: change L2ToL1MessagePasser & OPContractsManager semver * fix: cgt feature tests (#585) * fix: cgt feature tests * fix: deposits with amount 0 when cgt enabled * Revert "fix: deposits with amount 0 when cgt enabled" This reverts commit 8bee464. * fix: cgt portal (#587) * fix: ci fixes (#588) --------- Co-authored-by: Ashitaka <96790496+ashitakah@users.noreply.github.com> Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: Hex <165055168+hexshire@users.noreply.github.com> Co-authored-by: hexshire <hex@wonderland.xyz> * fix: semver * fix: opcm version * fix: tests * feat(op-acceptance-tests): add acceptance tests for native CGT across L1/L2 (ethereum-optimism#17451) - Gate suite via L1Block.isCustomGasToken(); assert name/symbol non-empty - L2: value transfer pays CGT token (typed amounts via op-service/eth) - L2: XDM rejects callvalue; L2StandardBridge legacy withdraw reverts - L1: Portal receive/deposit rejects ETH; introspect SystemConfig addr - L1: assert SystemConfig.isCustomGasToken() is true * fix: custom gas token rebase review comments (ethereum-optimism#17577) * chore: use w3 library * fix: msg error * refactor(cgt): rename amount & add geq 0 check (#600) * feat: add NativeAssetLiquidityAmount sign check * refactor: rename NativeAssetLiquidityAmount to InitialLiquidity * refactor(cgt): rename json nativeAssetLiquidityAmount to initialLiquidity (#601) * refactor: rename json nativeAssetLiquidityAmount to initialLiquidity * fix: only add initialLiquidity to cgt intent struct --------- Co-authored-by: Hex <165055168+hexshire@users.noreply.github.com> Co-authored-by: Ashitaka <96790496+ashitakah@users.noreply.github.com> Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> * fix: cgt review (ethereum-optimism#17612) * refactor: replace enableCustomGasToken with cgt devfeature * feat: remove fund in NativeAssetLiquidity (#605) * fix(cgt): remove OptimismPortal2 test from exclusions (#606) * fix: remove OptimismPortal2 test from exclusions * fix: remove virtual and restore constructor in portal test * refactor: use low level call to receive test * refactor(cgt): migrate L1Block and L2ToL1MessagePasser cgt tests (#608) * refactor: migrate L1Block cgt tests * refactor: migrate L2ToL1MessagePasser cgt tests * fix: comments (#607) * fix: comments * fix: custom error * fix: errors and tests * fix: errors and tests * fix: cgt tests (#610) * fix: comments * fix: custom error * fix: errors and tests * fix: errors and tests * fix: tests * fix: tests * fix: weth * fix: cgt tests (#611) * fix: comments * fix: custom error * fix: errors and tests * fix: errors and tests * fix: tests * fix: tests * fix: weth * fix: remove types * fix: coverage tests (#612) * fix: separate setUp function tests (#613) --------- Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: Ashitaka <96790496+ashitakah@users.noreply.github.com> * test: custom gas token invariants (ethereum-optimism#17489) * test(inv): setup and total sup inv * test(inv): accounting invariants * chore: doc * fix: import in OptimismPortal2CGT test and pre-pr * fix(cgt): add missing native asset amount (#543) * fix: upgrade contract name * feat: add nativeAssetLiquidityAmount to config json files * fix: add correct nativeAssetLiquidityAmount in op-deployer * fix: add correct nativeAssetLiquidityAmount in op-e2e and fix withCustomGasToken argument on op-devstack * fix: restore OptimismPortal2CGT * fix: all comments * fix: comments * fix: governace * fix: remove aux * fix: remove aux * fix: deploy cgt * fix: revert cgt deploy config * fix: deploy config * chore: run linter * fix: remove unnecessary GetNativeAssetLiquidityAmount --------- Co-authored-by: Ashitaka <ashitaka@defi.sucks> Co-authored-by: agusduha <agusnduha@gmail.com> Co-authored-by: hexshire <hex@wonderland.xyz> * fix: l2 genesis pipeline (#554) * fix: add nativeAssetLiquidityAmount in e2e apply test (#555) * fix: failing test * chore: doc Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> * chore: remove unused imports * chore: fix semgrep * chore: change error name --------- Co-authored-by: drgorillamd <83670532+drgorillamd@users.noreply.github.com> Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: Ashitaka <ashitaka@defi.sucks> Co-authored-by: agusduha <agusnduha@gmail.com> Co-authored-by: hexshire <hex@wonderland.xyz> Co-authored-by: AgusDuha <81362284+agusduha@users.noreply.github.com> Co-authored-by: Hex <165055168+hexshire@users.noreply.github.com> Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> * refactor: use skipIfDevFeatureDisabled for cgt predeploys test (#614) (ethereum-optimism#17621) Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> * fix: tests * fix: auth minter helper & bound _mint in depositTransaction tests (ethereum-optimism#17642) * refactor: auth minter helper tests (#618) * fix: bound _mint correctly in depositTransaction test function (#619) * chore: run pre-pr * chore: address ci errors * chore: fix go tests * chore: run checks * test(wip): fix failing tests * chore: run pre-pr * fix: cgt flag tests * test: skip fee split invriant when CGT enabled * test: fix l2 genesis override test (#673) * feat: add ownable liquidity (#671) * feat: add ownable liquidity * feat: remove enable and add default liquidity * feat: pre pr fixed * fix: initializer * fix: comments * fix: comments * fix: pre-pr * fix: override * fix: tests * fix: cursor issues * fix: cursor bug bot * fix: cursor bot * fix: remove override * fix: comment * feat: add checks * fix: tests * fix: nil in cgt config --------- Co-authored-by: agusduha <agusnduha@gmail.com> * fix: semver * test: fix cgt fork tests (#676) * test: fix cgt fork tests * fix: remove dev flag in L1 tests * fix: go linter (#679) * fix: go linter * fix: with custom gas token func * test: fix go fuzz (#680) * fix: cgt config jsons (#681) * fix: zero owner and sepolia address (#685) * feat: predeploys (#684) * fix: predeploys * fix: predeploys * fix: L2 genesis * test: fix cgt with revenue sharing tests (#691) * fix: semver * fix: cgt liquidity controller natspec (#694) * fix: remove dev feature flag from opcm (#695) * fix: remove dev feature flag from opcm * fix: lint --------- Signed-off-by: Hex <165055168+hexshire@users.noreply.github.com> Co-authored-by: niha <205694301+0xniha@users.noreply.github.com> Co-authored-by: Ashitaka <96790496+ashitakah@users.noreply.github.com> Co-authored-by: AgusDuha <81362284+agusduha@users.noreply.github.com> Co-authored-by: Mark Tyneway <mark.tyneway@gmail.com> Co-authored-by: Ashitaka <ashitaka@defi.sucks> Co-authored-by: agusduha <agusnduha@gmail.com> Co-authored-by: Stefano Charissis <stefano@oplabs.co> Co-authored-by: Simon Something /DrGoNoGo <83670532+simon-something@users.noreply.github.com> Co-authored-by: drgorillamd <83670532+drgorillamd@users.noreply.github.com> Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
CLOSES OPT-1297
CLOSES OPT-1286
CLOSES OPT-1316
https://cantina.xyz/code/940ca124-08fb-4789-8dc4-5c677d3997f2/findings?finding=10
Note
Introduce Ownable LiquidityController with configurable owner, refactor custom gas token enablement/defaults, and wire through deploy/genesis, ABI, and tests.
LiquidityControllernowOwnableUpgradeable;initializetakes_owner;authorize/deauthorizegated byonlyOwner.ILiquidityControllerinterface/ABI and storage layout; add ownership events/functions.OPContractsManagerversion to 5.6.1.L2Genesis.InputaddsliquidityControllerOwner; initialize controller with owner; assert owner in tests.liquidityControllerOwner(defaultfinalSystemOwner).nameandsymbolset.initialLiquidity, usetype(uint248).max; provideGetLiquidityControllerOwner()(defaults toL2ProxyAdminOwner).liquidityControllerOwner) to L2 genesis; validate non-zero owner inGasTokenDeployConfig.Written by Cursor Bugbot for commit 8d0c373. This will update automatically on new commits. Configure here.