-
Notifications
You must be signed in to change notification settings - Fork 4
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
Added baklava and alfajores rollup deployment configs #280
base: celo10
Are you sure you want to change the base?
Conversation
Merge commits are not allowed on this repository
"l1BlockTime": 12, | ||
"maxSequencerDrift": 600, | ||
"sequencerWindowSize": 7200, | ||
"channelTimeout": 300, |
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.
With the results from https://github.com/celo-org/celo-blockchain-planning/issues/804, I think we can leave this at the default (50). Note: so far nobody reviewed my reasoning in that issue.
"governanceTokenSymbol": "OP", | ||
"governanceTokenName": "Optimism", | ||
"governanceTokenOwner": "0xc07C5A1fBF6c7BC6b4f321E7dd031c0E1E98d32d", |
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.
Since we don't use these, can we remove them? Not that someone thinks we are subject to OP's governance.
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.
We'd need to check the code as right now it's required:
│ ├─ [0] VM::parseJsonUint("<stringified JSON>", "$.sequencerFeeVaultWithdrawalNetwork") [staticcall]
│ │ └─ ← [Return] 0
│ ├─ [0] VM::parseJsonString("<stringified JSON>", "$.governanceTokenName") [staticcall]
│ │ └─ ← [Revert] vm.parseJsonString: path "$.governanceTokenName" must return exactly one JSON value
│ └─ ← [Revert] vm.parseJsonString: path "$.governanceTokenName" must return exactly one JSON value
└─ ← [Revert] vm.parseJsonString: path "$.governanceTokenName" must return exactly one JSON value
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.
Ok, then leave that in. We could consider changing it to obvious placeholders, though.
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.
If you configure enableGovernance: false
then it will remove this error
enableGovernance = stdJson.readBool(_json, "$.enableGovernance"); |
This error in validate-deploy-configs looks like a problem:
|
It'd require merging this PR but I'm testing removing that config for this dry-run |
To prevent people from accidentally deploying GovToken with the same name as OP colliding with OP Mainnet's token, turn off by default. `op-deployer` should be doing this already, just doing this for people who deploy through the legacy path. See celo-org#280 (comment) for context.
To prevent people from accidentally deploying GovToken with the same name as OP colliding with OP Mainnet's token, turn off by default. `op-deployer` should be doing this already, just doing this for people who deploy through the legacy path. See celo-org#280 (comment) for context.
To prevent people from accidentally deploying GovToken with the same name as OP colliding with OP Mainnet's token, turn off by default. `op-deployer` should be doing this already, just doing this for people who deploy through the legacy path. See celo-org#280 (comment) for context.
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
No description provided.