Merged
Conversation
7534493 to
b4dfbb7
Compare
jflatow
approved these changes
Apr 22, 2022
Contributor
jflatow
left a comment
There was a problem hiding this comment.
Ok, some nits esp about the unused vars in unit tests, but I guess this is actually pretty awesome 💪 🧼
scott-silver
commented
Apr 22, 2022
Contributor
Author
|
@jflatow yeah, a lot of the alterations are prepending There's a regex that determines what an acceptable name is for an unused variable. Right now, the regex is We could alter that rule to allow for variables with the pattern of I'm pretty sure we could alter that rule only for test files as well, so for most files the rule would be underscores before unused variables, but for test files it would be underscores or letter+number. [UPDATE] synced offline; for simplicity, I think we should stick with prepending underscores to unused variables (i.e. |
a82e3e8 to
865e140
Compare
scott-silver
commented
Apr 25, 2022
e31b7b8 to
555bf45
Compare
5c3bc0a to
c533d09
Compare
c533d09 to
3694eea
Compare
scott-silver
commented
May 10, 2022
3d04cf9 to
1693305
Compare
Merged
kevincheng96
added a commit
that referenced
this pull request
Jun 14, 2023
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.
kevincheng96
added a commit
that referenced
this pull request
Jun 22, 2023
* 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>
kevincheng96
added a commit
that referenced
this pull request
Jun 28, 2023
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 <>
crypto-knightjk98
added a commit
to crypto-knightjk98/comet
that referenced
this pull request
Sep 27, 2025
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 compound-finance/comet#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.
crypto-knightjk98
added a commit
to crypto-knightjk98/comet
that referenced
this pull request
Sep 27, 2025
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/comet#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 compound-finance/comet#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 <>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now suggesting this In favor of #132.
Prettier is an extremely opinionated formatting tool that is intentionally limited in how customizable it is. We have limited options for customizing the rules we disagree with (like enforcing line length).
This PR deletes Prettier and replaces it with ESLint, which provides much greater flexibility in terms of which rules to enforce and how to enforce them.
The end result is a pretty light touch; mostly just identifying unused variables, enforcing single quotes, enforcing spacing, and enforcing some rules around TypeScript declarations.