feat(solidity-devops): gas pricing, verification and deployment utils #2439
Conversation
WalkthroughThe recent updates to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2439 +/- ##
===================================================
+ Coverage 46.91894% 46.92720% +0.00825%
===================================================
Files 400 383 -17
Lines 30655 29273 -1382
Branches 83 83
===================================================
- Hits 14383 13737 -646
+ Misses 14737 14073 -664
+ Partials 1535 1463 -72
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Review Status
Configuration used: .coderabbit.yaml
Files selected for processing (21)
- packages/solidity-devops/.env.example (1 hunks)
- packages/solidity-devops/README.md (1 hunks)
- packages/solidity-devops/deployments/eth_sepolia/.chainId (1 hunks)
- packages/solidity-devops/deployments/eth_sepolia/BasicContract.json (1 hunks)
- packages/solidity-devops/devops.json (1 hunks)
- packages/solidity-devops/foundry.toml (2 hunks)
- packages/solidity-devops/js/forgeScriptRun.js (2 hunks)
- packages/solidity-devops/js/forgeVerifyContract.js (1 hunks)
- packages/solidity-devops/js/saveDeployment.js (1 hunks)
- packages/solidity-devops/js/utils/cast.js (2 hunks)
- packages/solidity-devops/js/utils/chain.js (4 hunks)
- packages/solidity-devops/js/utils/deployments.js (5 hunks)
- packages/solidity-devops/js/utils/etherscan.js (1 hunks)
- packages/solidity-devops/js/utils/logger.js (1 hunks)
- packages/solidity-devops/js/utils/utils.js (2 hunks)
- packages/solidity-devops/js/verifyProxy.js (1 hunks)
- packages/solidity-devops/package.json (1 hunks)
- packages/solidity-devops/script/DeployBasicContract.s.sol (1 hunks)
- packages/solidity-devops/src/deploy/DeploymentSaver.sol (4 hunks)
- packages/solidity-devops/src/writer/ChainAwareWriter.sol (1 hunks)
- packages/solidity-devops/src/writer/DataWriter.sol (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/solidity-devops/deployments/eth_sepolia/.chainId
Additional comments not posted (25)
packages/solidity-devops/js/saveDeployment.js (1)
10-19: Consider adding error handling for external calls and validating input arguments to ensure robustness and prevent potential failures during execution.packages/solidity-devops/js/utils/logger.js (1)
23-27: Ensure the regex pattern inredactKeysaccurately targets only the intended sensitive keys to avoid redacting non-sensitive information.Verification successful
The regex pattern in
redactKeysaccurately targets and redacts values following options that end with-key, as demonstrated by the script output. This confirms that the function works as intended for enhancing security by redacting sensitive information in command logs.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Example command that might contain sensitive keys example_command="deploy --api-key 12345 --other-key 67890" # Apply redactKeys function logic echo "$example_command" | sed -E 's/(--\S+-key) (\S+)/\1 <REDACTED-VALUE>/g'Length of output: 226
packages/solidity-devops/deployments/eth_sepolia/BasicContract.json (1)
1-34: Ensure that no sensitive information is included in the deployment JSON files committed to public repositories.packages/solidity-devops/devops.json (1)
3-20: Verify the effectiveness of the--auto-gas-1559configuration across the specified chains to ensure optimal gas pricing strategies are applied.packages/solidity-devops/foundry.toml (1)
25-34: Confirm that the removal of theffiflag does not negatively impact any existing functionality that previously relied onffi.packages/solidity-devops/js/utils/cast.js (1)
7-14: Add error handling for the externalcastcommands ingetChainGasPricingRPCto ensure robustness in case of command failure or unexpected output.packages/solidity-devops/src/writer/ChainAwareWriter.sol (1)
15-17: Ensure that the mergedwriteDeploymentArtifactfunction correctly handles both cases of deployment artifacts, with and without ABI, as intended.packages/solidity-devops/package.json (1)
37-40: Verify that the new binary commands (fvc,sd,vp) are correctly mapped to their respective scripts and that the scripts are executable.Verification successful
The verification process has confirmed that the new binary commands (
fvc,sd,vp) are correctly mapped to their respective scripts in thepackage.jsonfile, as indicated by the presence of the corresponding entries. Additionally, the script files (forgeVerifyContract.js,saveDeployment.js,verifyProxy.js) referenced by these commands exist within the repository. This confirms that the commands are properly set up and should be executable as intended.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Example verification for one command # Replace <COMMAND> with one of the new commands (fvc, sd, vp) which <COMMAND>Length of output: 158
Script:
#!/bin/bash # Verify the presence of the commands in the package.json file fd package.json --exec grep '"fvc":' {} fd package.json --exec grep '"sd":' {} fd package.json --exec grep '"vp":' {} # Verify the existence of the script files referenced by the commands fd forgeVerifyContract.js fd saveDeployment.js fd verifyProxy.jsLength of output: 444
packages/solidity-devops/js/forgeScriptRun.js (1)
58-60: Ensure thatsaveNewDeploymentfunction properly handles errors or exceptions that may occur during the saving process.packages/solidity-devops/.env.example (1)
46-50: Ensure theETH_SEPOLIA_VERIFIER_KEYplaceholder is clearly marked as a placeholder to be replaced by the user's actual Etherscan API key to avoid confusion.packages/solidity-devops/js/utils/utils.js (1)
76-83: Validate the use ofAtomics.waitfor implementingsyncSleep. Ensure it's compatible with the environments where this script will run, asAtomics.waitcannot be called on the main thread in some environments (e.g., web browsers).packages/solidity-devops/src/writer/DataWriter.sol (1)
25-37: Ensure that the removal of thecontractNameandartifactWithoutABIparameters fromwriteDeploymentArtifactfunction aligns with the intended functionality and that all calls to this function have been updated accordingly.packages/solidity-devops/script/DeployBasicContract.s.sol (1)
19-67: Ensure that the deployment methods (deployAndSave,deployAndSaveAs, and their callbacks) properly handle errors or exceptions that may occur during the deployment process.packages/solidity-devops/js/utils/chain.js (3)
3-3: Consider renaminggetChainGasPricingRPCto more accurately reflect its purpose, such asfetchChainGasPricing.
13-14: Ensure the naming convention for constants is consistent across the project. If other constants use uppercase with underscores, these should too.
16-18: Verify that the verifier constants (VERIFIER_ETHERSCAN,VERIFIER_BLOCKSCOUT,VERIFIER_SOURCIFY) are used consistently across the project.Verification successful
The verification process has confirmed that the verifier constants (
VERIFIER_ETHERSCAN,VERIFIER_BLOCKSCOUT,VERIFIER_SOURCIFY) are used consistently across the project. No hardcoded string values were found, indicating adherence to best practices in using these constants.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for inconsistent usage of verifier constants across JavaScript files. rg --type js 'VERIFIER_(ETHERSCAN|BLOCKSCOUT|SOURCIFY)'Length of output: 990
Script:
#!/bin/bash # Search for hardcoded string values of the verifier constants across JavaScript files. rg --type js '"etherscan"|"blockscout"|"sourcify"'Length of output: 52
packages/solidity-devops/src/deploy/DeploymentSaver.sol (1)
114-114: Confirm that merging the functionality ofsaveDeploymentWithoutABIintosaveDeploymentdoes not introduce any regressions or unwanted behavior.Verification successful
The search for any remaining references to
saveDeploymentWithoutABIin Solidity files usingrgdid not produce any output. This indicates that there are no references tosaveDeploymentWithoutABIin the codebase, suggesting that the function has been successfully removed or merged without leaving any unintended references. Based on this information, it can be inferred that merging the functionality ofsaveDeploymentWithoutABIintosaveDeploymentlikely does not introduce any regressions or unwanted behavior related to the direct usage ofsaveDeploymentWithoutABI.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to saveDeploymentWithoutABI. ast-grep --lang solidity --pattern $'saveDeploymentWithoutABI($_, $_, $_)'Length of output: 192
Script:
#!/bin/bash # Search for any remaining references to saveDeploymentWithoutABI in Solidity files. rg 'saveDeploymentWithoutABI' --glob '*.sol'Length of output: 44
packages/solidity-devops/js/utils/deployments.js (2)
71-80: Consider extracting the contract name extraction logic into a utility module if it's used in multiple places across the project.
132-137: Verify thatgetSavedDeploymentis used consistently and that its error handling is sufficient for all use cases.packages/solidity-devops/README.md (6)
17-25: Clarify the installation instructions to include steps for users who might not be familiar with npm or yarn.
49-58: Add a note about the importance of keeping the verifier API key secure and not committing it to version control.
84-98: Recommend including a link to Foundry's official documentation for users to learn more about the configuration options infoundry.toml.
101-111: Encourage the use of comments within deployment and configuration scripts to improve readability and maintainability.
112-134: Advise on testing scripts in a controlled environment before broadcasting them to a live network to prevent unintended consequences.
149-160: Highlight the importance of verifying deployment artifacts and ensuring they match the on-chain deployment before proceeding with further actions.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Deploying sanguine-fe with
|
| Latest commit: |
f24fb3b
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: .coderabbit.yaml
Files selected for processing (3)
- packages/solidity-devops/js/forgeVerifyContract.js (1 hunks)
- packages/solidity-devops/js/utils/etherscan.js (1 hunks)
- packages/solidity-devops/js/verifyProxy.js (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/solidity-devops/js/forgeVerifyContract.js
- packages/solidity-devops/js/verifyProxy.js
Additional Context Used
GitHub Check Runs (1)
lint failure (4)
packages/solidity-devops/js/utils/etherscan.js: [warning] 67-67:
ReplaceRequest·with·GUID·${guid}·not·completed·after·${MAX_ATTEMPTS}·attempts.·Consider·checking·the·network·status·or·increasing·the·retry·interval.with⏎····Request·with·GUID·${guid}·not·completed·after·${MAX_ATTEMPTS}·attempts.·Consider·checking·the·network·status·or·increasing·the·retry·interval.⏎··
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- packages/solidity-devops/js/utils/etherscan.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/solidity-devops/js/utils/etherscan.js
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yaml
Files selected for processing (5)
- packages/solidity-devops/js/forgeVerifyContract.js (1 hunks)
- packages/solidity-devops/js/utils/cast.js (2 hunks)
- packages/solidity-devops/js/utils/chain.js (3 hunks)
- packages/solidity-devops/js/utils/etherscan.js (1 hunks)
- packages/solidity-devops/js/verifyProxy.js (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- packages/solidity-devops/js/forgeVerifyContract.js
- packages/solidity-devops/js/utils/cast.js
- packages/solidity-devops/js/utils/chain.js
- packages/solidity-devops/js/utils/etherscan.js
- packages/solidity-devops/js/verifyProxy.js
Description
PR to add a bit of everything since the last release:
--auto-gas-1559and--auto-gas-legacythat override the default Forge gas pricing with fresh values fetched from the RPC nodeeth_gasPriceis used2*base_fee + priority_feefor max gas pricepriority_feefor max priority feeffiturned on is no longer a requirement. Complex stuff such as extracting contract ABI has been moved to post-run routine.yarn fvcuses forge to verify a previously deployed contract.yarn sdsaves a deployment artifact for a previously deployed contract.yarn vpverifies a deployed proxy (currently only required for Etherscan explorers)README.mdSummary by CodeRabbit
New Features
castcommands.Documentation
Refactor
Bug Fixes