Linea Goerli cUSDCv3 deploy#758
Linea Goerli cUSDCv3 deploy#758kevincheng96 merged 27 commits intocompound-finance:linea/usdc-deployfrom Consensys:Linea-support
Conversation
kevincheng96
left a comment
There was a problem hiding this comment.
Looks good overall! Left some minor comments here and there. A lot of the diff is formatting related which I think can be reverted.
deployments/goerli/usdc/roots.json
Outdated
| "baseL1CrossDomainMessenger": "0x8e5693140eA606bcEB98761d9beB1BC87383706D", | ||
| "baseL1StandardBridge": "0xfA6D8Ee5BE770F84FC001D098C4bD604Fe01284a" | ||
| "baseL1StandardBridge": "0xfA6D8Ee5BE770F84FC001D098C4bD604Fe01284a", | ||
| "zkEvmV2": "0x70BaD09280FD342D02fe64119779BC1f0791BAC2" |
There was a problem hiding this comment.
I think lineaMessageService is a better name for this. zkEvmV2 seems too broad and generic.
| import './IMessageService.sol'; | ||
| import '../BaseBridgeReceiver.sol'; | ||
|
|
||
| contract LineaBridgeReceiver is BaseBridgeReceiver { |
There was a problem hiding this comment.
Talked about this offline. Since this contract is payable, we should extend the SweepableBridgeReceiver contract.
| "baseToken": "USDC", | ||
| "baseTokenAddress": "0xf56dc6695cF1f5c364eDEbC7Dc7077ac9B586068", | ||
| "baseTokenPriceFeed": "0xcb81fffa91cd5f3436318d25057c1d5df8e23f95", | ||
| "borrowMin": "100e6", |
There was a problem hiding this comment.
100e6 seems too high for an L2. I recommend something like 1e6.
| "baseSupplySpeed": "0.000011574074074074073e15", | ||
| "baseBorrowSpeed": "0.0011458333333333333e15", |
There was a problem hiding this comment.
Ideally, we set these as 0 on deploy and increase them with a cross-chain governance proposal.
| "borrowCF": 0.775, | ||
| "liquidateCF": 0.825, | ||
| "liquidationFactor": 0.95, | ||
| "supplyCap": "50_000e18" |
There was a problem hiding this comment.
Same here. We set them as 0 on deploy and increase them afterwards.
There was a problem hiding this comment.
Could we remove the changes in this file? Seems like these are all just formatting changes.
There was a problem hiding this comment.
Can remove this change.
There was a problem hiding this comment.
Same here. We can remove these since these are all just formatting changes.
plugins/import/etherscan.ts
Outdated
| // TODO remove this condition | ||
| if (network === 'linea-goerli') { | ||
| res = ( | ||
| await axios.get(url, { | ||
| params: data, | ||
| auth: { | ||
| username: 'eslineastage', | ||
| password: 'QW$eKsy3J%~v' | ||
| } | ||
| }) | ||
| )['data']; |
There was a problem hiding this comment.
Note: Remember to remove.
There was a problem hiding this comment.
Good catch thank you, completely forgot about that TODO
There was a problem hiding this comment.
Following up on this again, did you want to remove this before merging?
There was a problem hiding this comment.
Are the diffs in this file all just formatting changes? If so, let's revert the changes.
| constructor(address _messageService) { | ||
| messageService = IMessageService(_messageService); | ||
| } |
Check notice
Code scanning / Semgrep
Semgrep Finding: rules.solidity.performance.non-payable-constructor
| constructor(address _messageService) { | ||
| messageService = IMessageService(_messageService); | ||
| } |
Check warning
Code scanning / Semgrep
Semgrep Finding: compound.solidity.missing-constructor-sanity-checks
hardhat.config.ts
Outdated
| for (let i = 0; i < n; i++) | ||
| yield (BigInt('0x' + pk) + BigInt(i)).toString(16); | ||
| function* deriveAccounts(pk: string, n: number = 10) { | ||
| for (let i = 0; i < n; i++) yield (BigInt('0x' + pk) + BigInt(i)).toString(16); |
There was a problem hiding this comment.
Can we remove the formatting changes to this file? Seems unnecessary
|
One thing that is missing is the migration script to create a cross-chain proposal to initialize Comet (increase supply caps, set COMP speeds, etc.). Here's an example. |
|
I just added the changes to fix all the comments you guys had except the one for the migration file that I will in another commit. And a lot of files don’t respect those rules, I know it’s not the PR to do this kind of clean up, I just wanted to bring that up :) |
What values should I put in this file, can I just use the exact same values as what's been done for base-goerli ? |
Yeah, that is our bad. Through this PR, we realized that we should either enforce the Prettier or just remove it. We removed it in #760.
Yes, values such as the amount of COMP to bridge could be the same. However, a lot of the values used in the proposal are actually pulled from the
We usually keep the migration script in a separate PR for this reason. |
Thank you very clear, in that case if you can review my last changes taking care of all the comments except the migration that'd be great. |
Thanks for making the changes. Overall looks good! Left a few comments about some formatting changes, but not urgent and blocking. Looks like the CI jobs aren't running because your forked repo doesn't have the env vars set up. What we could do is have you merge the PR into a feature branch so we can run the tests using the env vars in our Comet repo. I created the
I don't think we should let the merging of this PR block the migration work. The code to make a proposal can still be executed without being merged into main. In fact, we run the migration and make sure the proposal passes first before merging. |
There was a problem hiding this comment.
Looks like roots have been cleared and may require a re-deploy. We'll need this to update the front-end.
There was a problem hiding this comment.
Oh my bad I meant to remove the ones in the localhost folder.
We still needed to redo a deployment anyway for the LineaBridgerReceiver change
There was a problem hiding this comment.
I don't think there are any diffs in this file outside of formatting changes right? Let's revert
There was a problem hiding this comment.
Not urgent, but would prefer to remove the formatting diffs.
There was a problem hiding this comment.
Not urgent, but would prefer to remove the formatting diffs.
There was a problem hiding this comment.
Not urgent, but would prefer to remove the formatting diffs.
…enBridge to the configuration
|
I pushed the latest changes to account for the latest comments. I'm working on a different branch for the migration script: #763 |
kevincheng96
left a comment
There was a problem hiding this comment.
Thanks for making the changes. Left a final comment to follow up on removing the password. Everything else looks good!
| 'Comet#bulker > (non-WETH base) all non-reward actions in one txn', | ||
| { | ||
| filter: async (ctx) => await isBulkerSupported(ctx) && !matchesDeployment(ctx, [{deployment: 'weth'}, {network: 'mumbai'}]), | ||
| filter: async (ctx) => await isBulkerSupported(ctx) && !matchesDeployment(ctx, [{deployment: 'weth'}, {network: 'mumbai'}, { network: 'linea-goerli' }]), |
There was a problem hiding this comment.
Curious why we need to exclude Linea from this test?
There was a problem hiding this comment.
Otherwise the scenario was failing, I can't remember should I remove it ?
There was a problem hiding this comment.
Yeah I think it's better to let the scenario fail so we can see if it's an actual problem or not.
plugins/import/etherscan.ts
Outdated
| // TODO remove this condition | ||
| if (network === 'linea-goerli') { | ||
| res = ( | ||
| await axios.get(url, { | ||
| params: data, | ||
| auth: { | ||
| username: 'eslineastage', | ||
| password: 'QW$eKsy3J%~v' | ||
| } | ||
| }) | ||
| )['data']; |
There was a problem hiding this comment.
Following up on this again, did you want to remove this before merging?
|
Another question: Are the supported assets minted by Linea or bridged from Goerli? If the former, I recommend sending a good amount to the |
|
#758 (comment) |
COMP, USDC, ETH should be bridged from goerli to Linea. |
* feat: messageService wrapper (WIP) * feat: deployment script (WIP) * feat: deployment script - adapt for Linea * feat: config - adapt for Linea * chore: delete unnecessary file * feat: update hardhat config (WIP) * fix: pragma conflict * fix: LineaBridgeReceiver issue * feat: add Linea config * fix: add lineascan_key * fix: add lineascan_key to github actions * feat: wip linea deployment * Base Goerli cWETHv3 deploy (#751) This PR introduces the cWETHv3 deployment on Base Goerli as well as the proposal to initialize the market there. When initializing the cWETHv3 market, we bridge over some WETH reserves. However, Optimism/Base does not support bridging WETH directly, so we bridge over ETH instead and wrap it before sending it to cWETHv3. I had to add some logic to our mock relayer code to support bridging of ETH. * Deploy script for cWETHv3 on Base Goerli * Create proposal to initialize cWETHv3 and bridge ETH as reserves * Run base-goerli-weth in scenarios CI * Fix pricefeeds path after rebase * Address PR comments * Modified deployment roots from GitHub Actions * Update proposal * Modified migration from GitHub Actions --------- Co-authored-by: GitHub Actions Bot <> * Adding the Semgrep worflow by Decurity (#742) This Semgrep workflow runs 2 rule sets over all the pull requests. The rule sets include both generic Solidity security detections and the comet-specific ones. We've reviewed the comet repository issues, pull requests and commits and created a few rules according to the fixes that were proposed by OZ/ChainSecurity/Certora and have been accepted by Compound. Most of the issues have been closed without fixes, and some of others are unfeasible to detect with Semgrep. You can see it in action here: Decurity#2 Right now if you run these rules over the whole comet codebase, the stats is as follows: Ran 9 rules on 36 files: 32 findings. 23 of these are missing sanity checks on constructor arguments (each argument = 1 finding). Of course many of them might not require the sanity checks so this rule can spam the alerts if run over the whole codebase but can be useful if run over the new contracts (there's only one constructor, so there won't be overwhelmingly many findings) Also there's a couple of "A constant name is not in UPPER_CASE like other constant variables." and "Event parameters with type 'address' should be indexed" findings. * Create .github/workflows/semgrep.yml * add compound specific semgrep rules * add env vars --------- Co-authored-by: Raz0r <me@raz0r.name> * feat: added Linea L1 message service contract in the configuration * feat: working solution, missing verification on etherscan * chore: rename arg no-deploy to noDeploy * feat: added linea cross chain governance scenario * feat: added relayMessage for linea * fix: cross chain governance tests for linea-goerli * feat: mage LineaBridgeReceiver fallback payable and removed comments * chore: new deployment with different account * feat: added missing changes for linea deployment/scenarios * fix: take into account compound team's comment except the migration file * feat: fixed formatting, reset configuration.json and added lineaL1TokenBridge to the configuration * feat: reduced baseMinForRewards to 1000e6 * feat: added migration script for linea-goerli to set reward config and ens text record * feat: added linea comp address in the migration script, remove COMP asset from the configuration.json * feat: fixed typo unit256 to uint256 + changed config values to be the the same as base goerli * Remove prettier (#760) The Comet repo currently has rules set up for both Prettier and ESLint, even though we only enforce the rules for ESLint. The move from Prettier to ESLint was decided in #321. Keeping the Prettier configs in the repo while not enforcing them is arguably worse than just not having the configs in the first place, since contributors may end up running the Prettier (e.g. via VSCode auto-format) for some files and introduce very large diffs. Removing Prettier should help reduce the formatting diffs introduced by contributors. * Make OptimismBridgeReceiver sweepable (#759) This change should extend the `OptimismBridgeReceiver` with some functionality to sweep ERC20 and native tokens. * Linea Goerli cUSDCv3 deploy (#758) * feat: messageService wrapper (WIP) * feat: deployment script (WIP) * feat: deployment script - adapt for Linea * feat: config - adapt for Linea * chore: delete unnecessary file * feat: update hardhat config (WIP) * fix: pragma conflict * fix: LineaBridgeReceiver issue * feat: add Linea config * fix: add lineascan_key * fix: add lineascan_key to github actions * feat: wip linea deployment * feat: added linea cross chain governance scenario * feat: added Linea L1 message service contract in the configuration * feat: added relayMessage for linea * feat: working solution, missing verification on etherscan * chore: rename arg no-deploy to noDeploy * fix: cross chain governance tests for linea-goerli * feat: mage LineaBridgeReceiver fallback payable and removed comments * chore: new deployment with different account * feat: added missing changes for linea deployment/scenarios * fix: take into account compound team's comment except the migration file * feat: fixed formatting, reset configuration.json and added lineaL1TokenBridge to the configuration * feat: reduced baseMinForRewards to 1000e6 --------- Co-authored-by: Arthur <arthur@coeos.xyz> * fix: LineaBridgeReceiver artifact path * feat: updated linea COMP address * fix: migration issues * feat: added WBTC as collateral * feat: added verification for liquidateCollateralFactor * fix: WBTC contract + added usdc specific bridge to transfer usdc from L1 to linea * fix: usdc decimals * fix: fixed WBTC contract + use usdcbridge in the scenario * fix: fixed last isssues --------- Co-authored-by: Arthur <arthur@coeos.xyz> Co-authored-by: Kevin Cheng <kevincheng96@hotmail.com> Co-authored-by: Omar Ganiev <beched@decurity.io> Co-authored-by: Raz0r <me@raz0r.name>
This PR supports a cUSDCv3 deployment on Linea Goerli (deploy script + migration script). Most of the work was done by the Linea dev team. * Linea Goerli cUSDCv3 deploy (#758) * feat: messageService wrapper (WIP) * feat: deployment script (WIP) * feat: deployment script - adapt for Linea * feat: config - adapt for Linea * chore: delete unnecessary file * feat: update hardhat config (WIP) * fix: pragma conflict * fix: LineaBridgeReceiver issue * feat: add Linea config * fix: add lineascan_key * fix: add lineascan_key to github actions * feat: wip linea deployment * feat: added linea cross chain governance scenario * feat: added Linea L1 message service contract in the configuration * feat: added relayMessage for linea * feat: working solution, missing verification on etherscan * chore: rename arg no-deploy to noDeploy * fix: cross chain governance tests for linea-goerli * feat: mage LineaBridgeReceiver fallback payable and removed comments * chore: new deployment with different account * feat: added missing changes for linea deployment/scenarios * fix: take into account compound team's comment except the migration file * feat: fixed formatting, reset configuration.json and added lineaL1TokenBridge to the configuration * feat: reduced baseMinForRewards to 1000e6 --------- Co-authored-by: Arthur <arthur@coeos.xyz> * Feat: linea migration script (#763) * feat: messageService wrapper (WIP) * feat: deployment script (WIP) * feat: deployment script - adapt for Linea * feat: config - adapt for Linea * chore: delete unnecessary file * feat: update hardhat config (WIP) * fix: pragma conflict * fix: LineaBridgeReceiver issue * feat: add Linea config * fix: add lineascan_key * fix: add lineascan_key to github actions * feat: wip linea deployment * Base Goerli cWETHv3 deploy (#751) This PR introduces the cWETHv3 deployment on Base Goerli as well as the proposal to initialize the market there. When initializing the cWETHv3 market, we bridge over some WETH reserves. However, Optimism/Base does not support bridging WETH directly, so we bridge over ETH instead and wrap it before sending it to cWETHv3. I had to add some logic to our mock relayer code to support bridging of ETH. * Deploy script for cWETHv3 on Base Goerli * Create proposal to initialize cWETHv3 and bridge ETH as reserves * Run base-goerli-weth in scenarios CI * Fix pricefeeds path after rebase * Address PR comments * Modified deployment roots from GitHub Actions * Update proposal * Modified migration from GitHub Actions --------- Co-authored-by: GitHub Actions Bot <> * Adding the Semgrep worflow by Decurity (#742) This Semgrep workflow runs 2 rule sets over all the pull requests. The rule sets include both generic Solidity security detections and the comet-specific ones. We've reviewed the comet repository issues, pull requests and commits and created a few rules according to the fixes that were proposed by OZ/ChainSecurity/Certora and have been accepted by Compound. Most of the issues have been closed without fixes, and some of others are unfeasible to detect with Semgrep. You can see it in action here: Decurity#2 Right now if you run these rules over the whole comet codebase, the stats is as follows: Ran 9 rules on 36 files: 32 findings. 23 of these are missing sanity checks on constructor arguments (each argument = 1 finding). Of course many of them might not require the sanity checks so this rule can spam the alerts if run over the whole codebase but can be useful if run over the new contracts (there's only one constructor, so there won't be overwhelmingly many findings) Also there's a couple of "A constant name is not in UPPER_CASE like other constant variables." and "Event parameters with type 'address' should be indexed" findings. * Create .github/workflows/semgrep.yml * add compound specific semgrep rules * add env vars --------- Co-authored-by: Raz0r <me@raz0r.name> * feat: added Linea L1 message service contract in the configuration * feat: working solution, missing verification on etherscan * chore: rename arg no-deploy to noDeploy * feat: added linea cross chain governance scenario * feat: added relayMessage for linea * fix: cross chain governance tests for linea-goerli * feat: mage LineaBridgeReceiver fallback payable and removed comments * chore: new deployment with different account * feat: added missing changes for linea deployment/scenarios * fix: take into account compound team's comment except the migration file * feat: fixed formatting, reset configuration.json and added lineaL1TokenBridge to the configuration * feat: reduced baseMinForRewards to 1000e6 * feat: added migration script for linea-goerli to set reward config and ens text record * feat: added linea comp address in the migration script, remove COMP asset from the configuration.json * feat: fixed typo unit256 to uint256 + changed config values to be the the same as base goerli * Remove prettier (#760) The Comet repo currently has rules set up for both Prettier and ESLint, even though we only enforce the rules for ESLint. The move from Prettier to ESLint was decided in #321. Keeping the Prettier configs in the repo while not enforcing them is arguably worse than just not having the configs in the first place, since contributors may end up running the Prettier (e.g. via VSCode auto-format) for some files and introduce very large diffs. Removing Prettier should help reduce the formatting diffs introduced by contributors. * Make OptimismBridgeReceiver sweepable (#759) This change should extend the `OptimismBridgeReceiver` with some functionality to sweep ERC20 and native tokens. * Linea Goerli cUSDCv3 deploy (#758) * feat: messageService wrapper (WIP) * feat: deployment script (WIP) * feat: deployment script - adapt for Linea * feat: config - adapt for Linea * chore: delete unnecessary file * feat: update hardhat config (WIP) * fix: pragma conflict * fix: LineaBridgeReceiver issue * feat: add Linea config * fix: add lineascan_key * fix: add lineascan_key to github actions * feat: wip linea deployment * feat: added linea cross chain governance scenario * feat: added Linea L1 message service contract in the configuration * feat: added relayMessage for linea * feat: working solution, missing verification on etherscan * chore: rename arg no-deploy to noDeploy * fix: cross chain governance tests for linea-goerli * feat: mage LineaBridgeReceiver fallback payable and removed comments * chore: new deployment with different account * feat: added missing changes for linea deployment/scenarios * fix: take into account compound team's comment except the migration file * feat: fixed formatting, reset configuration.json and added lineaL1TokenBridge to the configuration * feat: reduced baseMinForRewards to 1000e6 --------- Co-authored-by: Arthur <arthur@coeos.xyz> * fix: LineaBridgeReceiver artifact path * feat: updated linea COMP address * fix: migration issues * feat: added WBTC as collateral * feat: added verification for liquidateCollateralFactor * fix: WBTC contract + added usdc specific bridge to transfer usdc from L1 to linea * fix: usdc decimals * fix: fixed WBTC contract + use usdcbridge in the scenario * fix: fixed last isssues --------- Co-authored-by: Arthur <arthur@coeos.xyz> Co-authored-by: Kevin Cheng <kevincheng96@hotmail.com> Co-authored-by: Omar Ganiev <beched@decurity.io> Co-authored-by: Raz0r <me@raz0r.name> * Small clean up * More fixes * Modified migration from GitHub Actions * feat: increase supply caps for ETH and WBTC for the linea quest (#772) * feat: increase supply caps for ETH and WBTC for the linea quest * feat: removed unecessary spider call + nit * Fix supply cap migration script * Modified migration from GitHub Actions * Remove empty roots.json * Fix min borrow amount * Add COMP whale --------- Co-authored-by: Julink <julien.fontanel12@gmail.com> Co-authored-by: Arthur <arthur@coeos.xyz> Co-authored-by: Omar Ganiev <beched@decurity.io> Co-authored-by: Raz0r <me@raz0r.name> Co-authored-by: GitHub Actions Bot <>
Add changes to support deployment and scenarios for linea goerli network.