-
Notifications
You must be signed in to change notification settings - Fork 79
Deploy network hemi #1382
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
Deploy network hemi #1382
Conversation
WalkthroughAdds a new network "hemi" across configs, deployment manifests/logs, Foundry settings, and deployment scripts; introduces Security-contract support in helper functions and adjusts periphery/deploy task behavior. No existing network entries were removed; one gasZip router mapping moved to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (15)
script/tasks/diamondUpdateFacet.sh (1)
120-122: Gate debug echos behind verbose logging.These unconditional echoes can clutter PROD runs. Prefer echoDebug (consistent with rest of script).
- echo "SEND_PROPOSALS_DIRECTLY_TO_DIAMOND: $SEND_PROPOSALS_DIRECTLY_TO_DIAMOND" - echo "ENVIRONMENT: $ENVIRONMENT" + echoDebug "SEND_PROPOSALS_DIRECTLY_TO_DIAMOND: $SEND_PROPOSALS_DIRECTLY_TO_DIAMOND" + echoDebug "ENVIRONMENT: $ENVIRONMENT"script/helperFunctions.sh (2)
1460-1479: Include Security contracts – good; fix array handling to avoid word-splitting bugsThe inclusion of Security is correct. However, FACET_CONTRACTS, PERIPHERY_CONTRACTS, SECURITY_CONTRACTS, and DIAMOND_CONTRACTS are captured as strings, then expanded as arrays, which collapses multi‑item lists into single elements in some shells. Parse into arrays before merging.
- local FACET_CONTRACTS=$(getIncludedAndSortedFacetContractsArray "$EXCLUDE_CONFIG") - local PERIPHERY_CONTRACTS=$(getIncludedPeripheryContractsArray "$EXCLUDE_CONFIG") - # get all security contracts - local SECURITY_CONTRACTS=$(getIncludedSecurityContractsArray) - # get all diamond contracts - local DIAMOND_CONTRACTS=$(getContractNamesInFolder "src") - # merge - local ALL_CONTRACTS=("${DIAMOND_CONTRACTS[@]}" "${FACET_CONTRACTS[@]}" "${PERIPHERY_CONTRACTS[@]}" "${SECURITY_CONTRACTS[@]}") + local FACET_CONTRACTS PERIPHERY_CONTRACTS SECURITY_CONTRACTS DIAMOND_CONTRACTS + read -ra FACET_CONTRACTS <<<"$(getIncludedAndSortedFacetContractsArray "$EXCLUDE_CONFIG")" + read -ra PERIPHERY_CONTRACTS <<<"$(getIncludedPeripheryContractsArray "$EXCLUDE_CONFIG")" + read -ra SECURITY_CONTRACTS <<<"$(getIncludedSecurityContractsArray)" + read -ra DIAMOND_CONTRACTS <<<"$(getContractNamesInFolder "src")" + # merge + local ALL_CONTRACTS=("${DIAMOND_CONTRACTS[@]}" "${FACET_CONTRACTS[@]}" "${PERIPHERY_CONTRACTS[@]}" "${SECURITY_CONTRACTS[@]}")
2894-2913: New getIncludedSecurityContractsArray looks good; minor robustness nitFunction is fine and consistent with Periphery. If EXCLUDE_SECURITY_CONTRACTS can be empty, consider a safe default for the regex to avoid an empty group.
- local EXCLUDE_CONTRACTS_REGEX="^($(echo "$EXCLUDE_SECURITY_CONTRACTS" | tr ',' '|'))$" + local EXCLUDE_CONTRACTS_REGEX="^($(echo "${EXCLUDE_SECURITY_CONTRACTS:-__NONE__}" | tr ',' '|'))$"script/tasks/diamondUpdatePeriphery.sh (2)
21-26: Validate env even when NETWORK is passed inWhen NETWORK is provided via args, we still should validate .env requirements.
if [[ -z "$NETWORK" ]]; then checkNetworksJsonFilePath || checkFailure $? "retrieve NETWORKS_JSON_FILE_PATH" NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") - checkRequiredVariablesInDotEnv "$NETWORK" + checkRequiredVariablesInDotEnv "$NETWORK" fi + [[ -n "$NETWORK" ]] && checkRequiredVariablesInDotEnv "$NETWORK"
48-66: Unify selection across Periphery+Security; guard empty selection and parse safelyGood expansion to include Security. Add a guard if user cancels selection and avoid SC2207 by parsing with read -a.
- ALL_CONTRACTS="$PERIPHERY_CONTRACTS $SECURITY_CONTRACTS" - ALL_CONTRACTS_ARR=($(echo "$ALL_CONTRACTS" | tr ',' ' ')) + ALL_CONTRACTS="$PERIPHERY_CONTRACTS $SECURITY_CONTRACTS" + read -ra ALL_CONTRACTS_ARR <<<"$ALL_CONTRACTS" # ask user to select contracts to be updated - CONTRACTS=$(gum choose --no-limit "${ALL_CONTRACTS_ARR[@]}") + CONTRACTS=$(gum choose --no-limit "${ALL_CONTRACTS_ARR[@]}") + if [[ -z "$CONTRACTS" ]]; then + warning "No contracts selected; aborting." + return 1 + fiscript/deploy/deployPeripheryContracts.sh (1)
37-45: Include Security contracts in deploy loop; parse list safelyInclusion is good. Prefer parsing into an array to avoid word-splitting surprises.
- ALL_CONTRACTS="$PERIPHERY_CONTRACTS $SECURITY_CONTRACTS" + ALL_CONTRACTS="$PERIPHERY_CONTRACTS $SECURITY_CONTRACTS" + read -ra ALL_CONTRACTS_ARR <<<"$ALL_CONTRACTS" @@ - for CONTRACT in $ALL_CONTRACTS; do + for CONTRACT in "${ALL_CONTRACTS_ARR[@]}"; dodeployments/_deployments_log_file.json (9)
8834-8849: LiFiDiamond (hemi): constructor args reference deployed addresses—please double‑check.Ctor args embed 0x11f1022c… and 0x94a7ddfe…, which appear to reference other hemi deployments in this log. Confirm these are the intended timelock/owner (or init) addresses for hemi.
14788-14803: (Likely ERC20Proxy) 1.1.0 (hemi): verify Permit2 address in ctor.Ctor includes 0x11f1022c… as the sole arg—confirm this matches the intended Permit2 (or approval) contract for hemi.
17101-17116: FeeCollector 1.0.1 (hemi): verify beneficiary/admin.Single ctor address 0xAB483…—confirm it’s the correct treasury/beneficiary for hemi.
28234-28249: LiFuelFeeCollector 1.1.0 (hemi): check chain/owner args.Ctor packs chain/config (0x4200…06) and owner 0x156c…—please confirm these match hemi’s expected params.
30317-30332: StargateFacetV2 1.0.1 (hemi): router ref check.Ctor references 0xAF5191…—please confirm this is the StargateV2 router (or endpoint) for hemi.
30826-30841: ReceiverStargateV2 1.1.0 (hemi): cross‑check all linked deps.Ctor wires owner 0x156c…, Executor 0x765F…, Stargate 0xAF519…, token 0x6f47…, minGas 100000. Validate each against hemi config.
33157-33172: EmergencyPauseFacet 1.0.1 (hemi): pauser address check.Ctor references 0xD387…—confirm it’s the correct pauser/governance for hemi.
37018-37033: GasZipFacet 2.0.5 (hemi): router mapping move—sanity check.Ctor references 0x2a37d6… (router). Since mapping moved from Cronos to Hemi per PR summary, please confirm this is the intended router on hemi.
40124-40139: RelayFacet 1.0.1 (hemi): multiple packed params—please confirm.Ctor blob includes fee, arrays, and references to 0xAD6912… and LiFiDiamond 0x026F25… Ensure these align with hemi env expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
config/dexs.json(1 hunks)config/gaszip.json(1 hunks)config/networks.json(2 hunks)config/permit2Proxy.json(1 hunks)config/stargateV2.json(2 hunks)deployments/_deployments_log_file.json(23 hunks)deployments/hemi.diamond.json(1 hunks)deployments/hemi.json(1 hunks)foundry.toml(2 hunks)script/deploy/_targetState.json(1 hunks)script/deploy/deployAllContracts.sh(1 hunks)script/deploy/deployPeripheryContracts.sh(1 hunks)script/helperFunctions.sh(3 hunks)script/tasks/diamondUpdateFacet.sh(2 hunks)script/tasks/diamondUpdatePeriphery.sh(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
script/**/*.sh
📄 CodeRabbit inference engine (conventions.md)
script/**/*.sh: Bash scripts must start with #!/bin/bash, be modular with sections (Logging, Error handling and logging, Deployment functions), follow DRY via helpers, and organize core operations into functions
Bash style: consistent indentation/naming, comments, usage instructions, and TODO/limitations documented
Bash error handling and logging: use helper logging functions, validate inputs early, check exit status with checkFailure, and use set -e where appropriate
Files:
script/deploy/deployAllContracts.shscript/deploy/deployPeripheryContracts.shscript/helperFunctions.shscript/tasks/diamondUpdateFacet.shscript/tasks/diamondUpdatePeriphery.sh
{script/**/*.sh,preinstall.sh}
📄 CodeRabbit inference engine (conventions.md)
Environment: load from .env or config.sh, declare globals in config, update .env.example, validate env vars early; add system packages to preinstall.sh
Files:
script/deploy/deployAllContracts.shscript/deploy/deployPeripheryContracts.shscript/helperFunctions.shscript/tasks/diamondUpdateFacet.shscript/tasks/diamondUpdatePeriphery.sh
🧠 Learnings (57)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1168
File: script/deploy/_targetState.json:1564-1589
Timestamp: 2025-05-27T12:36:26.987Z
Learning: When reviewing deployment PRs in the lifinance/contracts repository, target state configuration files (like script/deploy/_targetState.json) may be updated for multiple networks even when the PR is focused on deploying to a specific network. The scope should be determined by the PR title and description, not just by all configuration changes present in the files.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
📚 Learning: 2025-09-09T10:39:26.383Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1357
File: deployments/lens.diamond.json:48-51
Timestamp: 2025-09-09T10:39:26.383Z
Learning: In the lifinance/contracts repository, when deployment JSON files show address changes (like AcrossFacetV3 address updates in deployments/*.diamond.json), the corresponding _deployments_log_file.json updates may be handled in separate PRs rather than the same PR that updates the diamond configuration files.
Applied to files:
script/deploy/deployAllContracts.shdeployments/hemi.jsondeployments/hemi.diamond.jsonscript/tasks/diamondUpdateFacet.shdeployments/_deployments_log_file.json
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/deploy/deployAllContracts.shscript/deploy/deployPeripheryContracts.shdeployments/_deployments_log_file.jsonscript/deploy/_targetState.json
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/deploy/deployAllContracts.shscript/deploy/deployPeripheryContracts.shdeployments/_deployments_log_file.jsonscript/deploy/_targetState.json
📚 Learning: 2025-06-05T10:00:01.583Z
Learnt from: mirooon
PR: lifinance/contracts#1145
File: script/tasks/diamondSyncWhitelistedAddresses.sh:208-209
Timestamp: 2025-06-05T10:00:01.583Z
Learning: Task scripts in script/tasks/ directory (like diamondSyncWhitelistedAddresses.sh) define functions that are sourced by deployAllContracts.sh and called from there, rather than executing directly. They don't need to call their own functions at the end of the file.
Applied to files:
script/deploy/deployAllContracts.shscript/deploy/deployPeripheryContracts.shscript/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2025-02-13T03:07:05.721Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#994
File: script/config.example.sh:107-108
Timestamp: 2025-02-13T03:07:05.721Z
Learning: The DEPLOY_NEW_NETWORK_MODE flag in deployment scripts is required during initial contract deployment because ownership hasn't been transferred to SAFE yet. This mode allows direct execution of diamondCut and registerPeriphery transactions by the deployer.
Applied to files:
script/deploy/deployAllContracts.sh
📚 Learning: 2025-08-07T06:34:07.709Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1316
File: script/deploy/deployAllContracts.sh:346-350
Timestamp: 2025-08-07T06:34:07.709Z
Learning: In script/deploy/deployAllContracts.sh, 0xDEnYO prefers not to add automated retry mechanisms for cast send commands in deployment scripts, preferring manual re-execution if network issues occur. This prioritizes simplicity over automated error handling.
Applied to files:
script/deploy/deployAllContracts.shscript/deploy/deployPeripheryContracts.shscript/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2024-11-26T01:18:52.125Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: script/deploy/deploySingleContract.sh:231-231
Timestamp: 2024-11-26T01:18:52.125Z
Learning: In the `deploySingleContract.sh` script, environment variable assignments preceding the `forge script` command (e.g., `DEPLOYSALT=$DEPLOYSALT ... forge script ...`) are acceptable and function correctly, even when variable expansions like `$DIAMOND_TYPE` and `$NETWORK` are used later in the same command.
Applied to files:
script/deploy/deployAllContracts.shscript/tasks/diamondUpdateFacet.sh
📚 Learning: 2025-06-24T04:41:50.775Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1233
File: script/deploy/deployAllContracts.sh:37-50
Timestamp: 2025-06-24T04:41:50.775Z
Learning: The script/deploy/deployAllContracts.sh deployment script is intentionally designed to be interactive-only and should never be run headless. The gum choose command for stage selection is a deliberate design choice requiring manual user interaction.
Applied to files:
script/deploy/deployAllContracts.sh
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Applied to files:
script/deploy/deployAllContracts.shdeployments/hemi.jsondeployments/_deployments_log_file.json
📚 Learning: 2025-01-28T14:27:50.689Z
Learnt from: ezynda3
PR: lifinance/contracts#924
File: script/deploy/zksync/DeployReceiverStargateV2.s.sol:19-21
Timestamp: 2025-01-28T14:27:50.689Z
Learning: In LiFi's deployment scripts, the `deploy` method in `DeployScriptBase` handles the concatenation of constructor arguments with the contract's creation code, so child contracts don't need to concatenate the arguments explicitly.
Applied to files:
script/deploy/deployAllContracts.sh
📚 Learning: 2025-04-21T03:15:12.236Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.diamond.json:84-85
Timestamp: 2025-04-21T03:15:12.236Z
Learning: In deployment JSON files that contain "diamond" in their filename (located in the deployments folder), periphery contracts may have empty string values for their addresses. This indicates that the contract is not deployed on that particular chain and should not be flagged as an issue during code reviews.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.jsondeployments/_deployments_log_file.json
📚 Learning: 2025-09-16T01:39:54.086Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.086Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.jsonscript/tasks/diamondUpdateFacet.sh
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.jsondeployments/_deployments_log_file.json
📚 Learning: 2024-09-27T07:10:15.586Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#812
File: deployments/polygon.diamond.json:4-11
Timestamp: 2024-09-27T07:10:15.586Z
Learning: In `deployments/polygon.diamond.json`, it's acceptable for certain facets to have empty names and versions when specified by the developer.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.jsondeployments/_deployments_log_file.jsonscript/deploy/_targetState.json
📚 Learning: 2024-11-01T11:53:57.162Z
Learnt from: ezynda3
PR: lifinance/contracts#846
File: deployments/cronos.diamond.json:28-31
Timestamp: 2024-11-01T11:53:57.162Z
Learning: In `deployments/cronos.diamond.json`, both `GenericSwapFacet` and `GenericSwapFacetV3` are distinct facets that should be included together, as they serve different purposes.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.json
📚 Learning: 2024-11-26T01:01:18.499Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/arbitrum.diamond.json:148-150
Timestamp: 2024-11-26T01:01:18.499Z
Learning: In `deployments/*.diamond.json` configuration files, facets that are part of an open PR and not yet merged into the main branch may have missing `Name` and `Version` fields.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.jsondeployments/_deployments_log_file.json
📚 Learning: 2024-11-25T09:05:43.045Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/base.diamond.json:148-148
Timestamp: 2024-11-25T09:05:43.045Z
Learning: In deployment configuration files (e.g., `deployments/base.diamond.json`), empty addresses for contracts like `Permit2Proxy` may be placeholders and will be updated after approvals are released from the multisig safe.
Applied to files:
deployments/hemi.jsonconfig/permit2Proxy.json
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#819
File: deployments/scroll.diamond.json:82-82
Timestamp: 2024-10-09T03:47:21.269Z
Learning: In the `deployments/*.diamond.json` and `deployments/*.json` files, the `LiFiDEXAggregator` contract may intentionally have the same contract address across multiple networks. This is acceptable and should not be flagged as an issue in future code reviews.
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.json
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : Facet contracts must import/use: ILiFi, LibAsset, LibSwap, LibAllowList; and use security mixins ReentrancyGuard, SwapperV2, Validatable; ECDSA optional
Applied to files:
deployments/hemi.jsondeployments/hemi.diamond.json
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : Facet contracts must implement: internal _startBridge, swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Applied to files:
deployments/hemi.json
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : Facet contracts must be located in src/Facets and include "Facet" in the contract name
Applied to files:
deployments/hemi.jsonscript/helperFunctions.sh
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
deployments/hemi.json
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to script/deploy/targetState.json : Maintain targetState.json to track expected versions per network/environment and ensure deployment version consistency
Applied to files:
script/deploy/deployPeripheryContracts.shscript/deploy/_targetState.json
📚 Learning: 2025-07-15T05:05:28.134Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1140
File: config/permit2Proxy.json:3-3
Timestamp: 2025-07-15T05:05:28.134Z
Learning: In config/permit2Proxy.json, the addresses represent the Permit2 contract addresses on each chain, NOT the Permit2Proxy contract addresses. These Permit2 addresses are used as constructor arguments when deploying the Permit2Proxy contract. The Permit2Proxy acts as a wrapper/proxy that interacts with the underlying Permit2 contract on each network.
Applied to files:
config/permit2Proxy.json
📚 Learning: 2024-11-26T01:16:48.020Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/aurora.diamond.json:75-75
Timestamp: 2024-11-26T01:16:48.020Z
Learning: On networks where the `Permit2Proxy` contract is not deployed, the `Permit2Proxy` address is intentionally left empty in the configuration files.
Applied to files:
config/permit2Proxy.json
📚 Learning: 2024-11-26T01:17:13.212Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/cronos.diamond.json:67-67
Timestamp: 2024-11-26T01:17:13.212Z
Learning: In the `deployments/cronos.diamond.json` file, the `Permit2Proxy` address is intentionally left empty because `Permit2Proxy` is not deployed on the Cronos network.
Applied to files:
config/permit2Proxy.json
📚 Learning: 2025-06-25T06:27:38.873Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1140
File: deployments/fantom.diamond.json:97-105
Timestamp: 2025-06-25T06:27:38.873Z
Learning: When contracts are redeployed, they receive new addresses. Permit2Proxy addresses in deployment files reflect the actual deployed contract addresses for each network, not a standardized address across all networks.
Applied to files:
config/permit2Proxy.json
📚 Learning: 2024-11-26T01:04:16.637Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/fantom.diamond.json:92-94
Timestamp: 2024-11-26T01:04:16.637Z
Learning: In `deployments/*.diamond.json` files, it's acceptable for facets to have empty `Name` and `Version` fields, and these should not be flagged as issues.
Applied to files:
deployments/hemi.diamond.json
📚 Learning: 2024-11-26T01:14:58.163Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/worldchain.diamond.json:5-63
Timestamp: 2024-11-26T01:14:58.163Z
Learning: When facet versions are set to empty strings in `deployments/*.diamond.json` files, do not report this issue, as it is already known and will be addressed separately.
Applied to files:
deployments/hemi.diamond.jsonscript/tasks/diamondUpdateFacet.sh
📚 Learning: 2025-01-28T14:30:06.911Z
Learnt from: ezynda3
PR: lifinance/contracts#924
File: deployments/abstract.json:2-4
Timestamp: 2025-01-28T14:30:06.911Z
Learning: In the LiFi contract architecture, "LiFiDiamond" is the diamond proxy contract that connects all facets into a cohesive system. The facets are individual contracts that provide specific functionality, and the diamond proxy delegates calls to these facets.
Applied to files:
deployments/hemi.diamond.json
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#819
File: deployments/fuse.json:24-25
Timestamp: 2024-10-09T03:47:21.269Z
Learning: The `LiFiDEXAggregator.sol` contract is located in `src/Periphery/`.
Applied to files:
deployments/hemi.diamond.jsondeployments/_deployments_log_file.json
📚 Learning: 2025-06-13T08:30:26.220Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1207
File: deployments/_deployments_log_file.json:34037-34080
Timestamp: 2025-06-13T08:30:26.220Z
Learning: LiFiDiamond contains generic withdrawal logic, so individual facet contracts (e.g., PioneerFacet) do not need their own Ether-withdraw functions.
Applied to files:
deployments/hemi.diamond.json
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: ezynda3
PR: lifinance/contracts#806
File: deployments/_deployments_log_file.json:5780-5793
Timestamp: 2024-10-09T03:47:21.269Z
Learning: The owner address `0x11f11121df7256c40339393b0fb045321022ce44` and the `DiamondCutFacet` address `0xd5cf40a2a18b633cfd6a1ae16d1771596498cf83` in the `LiFiDiamond` deployment on `xlayer` are correct and should not be flagged as issues, even if they are not referenced in the Solidity files.
Applied to files:
deployments/hemi.diamond.json
📚 Learning: 2025-06-24T01:27:58.260Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1231
File: script/helperFunctions.sh:3641-3646
Timestamp: 2025-06-24T01:27:58.260Z
Learning: In the script/helperFunctions.sh codebase, the scripts have existing validation mechanisms that ensure the ENVIRONMENT parameter is always either "production" or "staging" before reaching functions like updateDiamondLogs, making additional validation within individual functions redundant.
Applied to files:
script/tasks/diamondUpdateFacet.shscript/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2025-01-28T14:29:00.823Z
Learnt from: ezynda3
PR: lifinance/contracts#924
File: script/deploy/zksync/utils/UpdateScriptBase.sol:112-178
Timestamp: 2025-01-28T14:29:00.823Z
Learning: The suggestion to modify `buildDiamondCut` function in `UpdateScriptBase.sol` to handle selectors from multiple old facets differently was deemed unnecessary by the maintainer.
Applied to files:
script/tasks/diamondUpdateFacet.sh
📚 Learning: 2025-01-29T06:28:56.328Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#961
File: script/tasks/diamondUpdateFacet.sh:119-119
Timestamp: 2025-01-29T06:28:56.328Z
Learning: In shell scripts, environment variables can be safely assigned inline with commands (e.g., `VAR=value command`) when they are only needed for that specific command execution, as demonstrated in the `diamondUpdateFacet.sh` script.
Applied to files:
script/tasks/diamondUpdateFacet.sh
📚 Learning: 2025-04-23T05:20:45.831Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1102
File: script/tasks/diamondSyncSigs_FAST.sh:14-18
Timestamp: 2025-04-23T05:20:45.831Z
Learning: In the lifinance/contracts repository, the `diamondSyncSigs_FAST` function in script/tasks/diamondSyncSigs_FAST.sh doesn't need explicit parameter validation (like for the ENVIRONMENT parameter) because it's called from a master script that ensures these parameters are available.
Applied to files:
script/tasks/diamondUpdateFacet.sh
📚 Learning: 2025-08-27T08:45:59.606Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Applied to files:
script/tasks/diamondUpdateFacet.shscript/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#819
File: script/tasks/diamondUpdatePeriphery.sh:194-194
Timestamp: 2024-10-09T03:47:21.269Z
Learning: In `script/tasks/diamondUpdatePeriphery.sh`, the team prefers to use `echo` statements without escaping inner quotes, even if Shellcheck warns about SC2027 regarding unintentional unquoting.
Applied to files:
script/tasks/diamondUpdateFacet.shscript/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2024-11-21T08:39:29.530Z
Learnt from: ezynda3
PR: lifinance/contracts#861
File: config/dexs.json:748-752
Timestamp: 2024-11-21T08:39:29.530Z
Learning: In the 'worldchain' network, the addresses `0x50D5a8aCFAe13Dceb217E9a071F6c6Bd5bDB4155`, `0x8f023b4193a6b18C227B4a755f8e28B3D30Ef9a1`, and `0x603a538477d44064eA5A5d8C345b4Ff6fca1142a` are used as DEXs and should be included in `config/dexs.json`.
Applied to files:
config/dexs.json
📚 Learning: 2025-08-28T02:02:56.965Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1136
File: config/dexs.json:0-0
Timestamp: 2025-08-28T02:02:56.965Z
Learning: Different networks in config/dexs.json naturally and appropriately have different DEX addresses. Do not flag different addresses across different networks as issues - only flag if the same network's addresses appear to have changed unintentionally between branches.
Applied to files:
config/dexs.json
📚 Learning: 2024-11-04T02:25:07.478Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#846
File: config/dexs.json:296-300
Timestamp: 2024-11-04T02:25:07.478Z
Learning: In `config/dexs.json`, it's expected that some addresses appear multiple times across different networks.
Applied to files:
config/dexs.json
📚 Learning: 2024-10-21T01:27:47.808Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#836
File: config/dexs.json:396-397
Timestamp: 2024-10-21T01:27:47.808Z
Learning: In `config/dexs.json`, it's acceptable for a whitelisted DEX address to be the same across multiple networks.
Applied to files:
config/dexs.json
📚 Learning: 2025-05-28T17:33:33.959Z
Learnt from: mirooon
PR: lifinance/contracts#1170
File: deployments/_deployments_log_file.json:28060-28072
Timestamp: 2025-05-28T17:33:33.959Z
Learning: Deployment log files like `deployments/_deployments_log_file.json` are historical records that document the actual versions deployed at specific times. They should not be updated to match current contract versions - they must accurately reflect what was deployed when the deployment occurred.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#812
File: deployments/_deployments_log_file.json:1914-1927
Timestamp: 2024-10-09T03:47:21.269Z
Learning: Duplicate entries in `deployments/_deployments_log_file.json` that are outdated do not require changes.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-10-09T07:06:25.731Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#829
File: deployments/optimism.diamond.staging.json:4-7
Timestamp: 2024-10-09T07:06:25.731Z
Learning: In `src/Facets/EmergencyPauseFacet.sol`, the `emergencyPause` and `emergencyUnpause` functions are correctly implemented and present.
Applied to files:
deployments/_deployments_log_file.json
📚 Learning: 2024-11-26T01:50:51.809Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#874
File: script/tasks/diamondSyncSigs.sh:88-88
Timestamp: 2024-11-26T01:50:51.809Z
Learning: In the 'lifinance/contracts' repository, scripts like 'script/tasks/diamondSyncSigs.sh' are executed only on local machines where private keys are securely managed, so additional private key security improvements are not required.
Applied to files:
script/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2025-06-06T16:09:58.692Z
Learnt from: mirooon
PR: lifinance/contracts#1193
File: script/tasks/diamondSyncWhitelistedAddresses.sh:128-128
Timestamp: 2025-06-06T16:09:58.692Z
Learning: In script/tasks/diamondSyncWhitelistedAddresses.sh at line 128, there is an unquoted command substitution `$(getPrivateKey "$NETWORK" "$ENVIRONMENT")` that should be quoted as `"$(getPrivateKey "$NETWORK" "$ENVIRONMENT")"` to prevent word splitting issues. The user mirooon wants to be reminded about this issue in future PRs when this file is touched.
Applied to files:
script/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2024-10-14T00:49:45.265Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/polygon.json:53-55
Timestamp: 2024-10-14T00:49:45.265Z
Learning: Use 'cast code ADDRESS --rpc-url <RPC-URL>' from the Foundry framework to check if a contract has bytecode deployed, to avoid false negatives.
Applied to files:
script/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#812
File: deployments/celo.json:26-26
Timestamp: 2024-10-09T03:47:21.269Z
Learning: When verifying contract deployments on the Celo network, ensure that the method used to check for contract code at an address does not produce false positives.
Applied to files:
script/tasks/diamondUpdatePeriphery.sh
📚 Learning: 2024-11-21T08:34:30.300Z
Learnt from: ezynda3
PR: lifinance/contracts#861
File: config/global.json:146-146
Timestamp: 2024-11-21T08:34:30.300Z
Learning: The project is deprecating `safeAddresses` and `safeApiUrls` in `global.json` and moving these configurations to `config/networks.json` for network configurations.
Applied to files:
config/networks.json
📚 Learning: 2024-11-21T08:24:53.059Z
Learnt from: ezynda3
PR: lifinance/contracts#861
File: script/deploy/_targetState.json:1453-1483
Timestamp: 2024-11-21T08:24:53.059Z
Learning: In `script/deploy/_targetState.json`, for the `abstract` network configuration, `ReceiverStargateV2` is correctly set to version `1.0.1`.
Applied to files:
script/deploy/_targetState.json
📚 Learning: 2024-11-21T08:25:26.214Z
Learnt from: ezynda3
PR: lifinance/contracts#861
File: script/deploy/_targetState.json:1364-1390
Timestamp: 2024-11-21T08:25:26.214Z
Learning: For the Cronos network configuration in `script/deploy/_targetState.json`, the absence of bridge facets such as `StargateFacet`, `AcrossFacet`, `HopFacet`, and `SymbiosisFacet` is acceptable and expected.
Applied to files:
script/deploy/_targetState.json
📚 Learning: 2025-05-27T12:36:26.987Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1168
File: script/deploy/_targetState.json:1564-1589
Timestamp: 2025-05-27T12:36:26.987Z
Learning: When reviewing deployment PRs in the lifinance/contracts repository, target state configuration files (like script/deploy/_targetState.json) may be updated for multiple networks even when the PR is focused on deploying to a specific network. The scope should be determined by the PR title and description, not just by all configuration changes present in the files.
Applied to files:
script/deploy/_targetState.json
📚 Learning: 2025-02-13T03:04:19.306Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#992
File: script/deploy/_targetState.json:25-32
Timestamp: 2025-02-13T03:04:19.306Z
Learning: Duplicate contract names (e.g., ERC20Proxy, Executor, etc.) in _targetState.json are expected when they appear across different network configurations, as they represent the same contract type deployed on multiple networks.
Applied to files:
script/deploy/_targetState.json
📚 Learning: 2025-09-01T09:35:29.886Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1299
File: deployments/arbitrum.staging.json:59-61
Timestamp: 2025-09-01T09:35:29.886Z
Learning: 0xDEnYO has repeatedly emphasized that the target state file (script/deploy/_targetState.json) should NEVER be reviewed, verified, or mentioned in deployment PRs. This instruction has been given multiple times and must be strictly followed without exception.
Applied to files:
script/deploy/_targetState.json
🧬 Code graph analysis (3)
script/deploy/deployAllContracts.sh (1)
script/helperFunctions.sh (1)
updateDiamondLogForNetwork(4425-4518)
script/deploy/deployPeripheryContracts.sh (1)
script/helperFunctions.sh (1)
getIncludedSecurityContractsArray(2895-2912)
script/tasks/diamondUpdatePeriphery.sh (1)
script/helperFunctions.sh (4)
getIncludedPeripheryContractsArray(2876-2893)getIncludedSecurityContractsArray(2895-2912)getRPCUrl(3620-3629)checkFailure(3047-3057)
🪛 Gitleaks (8.27.2)
deployments/hemi.json
[high] 2-2: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 21-21: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
deployments/hemi.diamond.json
[high] 70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Shellcheck (0.10.0)
script/helperFunctions.sh
[warning] 1463-1463: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 1466-1466: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 1466-1466: Variable was used as an array but is now assigned a string.
(SC2178)
[warning] 1469-1469: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 1472-1472: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 2901-2901: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 3182-3182: Declare and assign separately to avoid masking return values.
(SC2155)
script/tasks/diamondUpdatePeriphery.sh
[warning] 62-62: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
[warning] 156-156: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (32)
foundry.toml (2)
121-121: Etherscan/verifier entry — OK; add BLOCKSCOUT_API_KEY to .env.example.
foundry.toml entry matches config/networks.json (chain 43111, https://explorer.hemi.xyz/api); .env.example does not contain BLOCKSCOUT_API_KEY — add that variable to .env.example.
52-52: Document new RPC env var in .env.examplefoundry.toml references ETH_NODE_URI_HEMI (foundry.toml:52). Add ETH_NODE_URI_HEMI to .env.example (e.g. ETH_NODE_URI_HEMI="").
config/stargateV2.json (2)
76-76: Confirmed — Stargate TokenMessaging address for Hemi (chainId 43111)
'hemi' in config/stargateV2.json matches upstream Stargate v2 TokenMessaging: 0xAf5191B0De278C7286d6C7CC6ab6BB8A73bA2Cd6.
24-24: Confirmed: LayerZero Endpoint V2 for Hemi is correct.
0x6F475642a6e85809B1c36Fa62763669b1b48DD5B matches LayerZero's published Endpoint V2 for chainId 43111 (Hemi).config/gaszip.json (1)
27-27: Confirm hemi GasZip chain id is correct for production (networks.json = 397)Repo verification:
- config/networks.json → hemi.gasZipChainId = 397
- config/gaszip.json → gasZipRouters.hemi = 0x2a37D63EAdFe4b4682a3c28C1c2cD4F109Cc2762
Confirm 397 is the intended production chain id; if not, update config/networks.json accordingly.
script/tasks/diamondUpdateFacet.sh (1)
5-6: Don't re‑source .env — good. Confirm callers export SEND_PROPOSALS_DIRECTLY_TO_DIAMONDOnly the function definition exists at script/tasks/diamondUpdateFacet.sh:3 — no callers found under script/; verify whether callers live outside the repo or are invoked dynamically and ensure they export SEND_PROPOSALS_DIRECTLY_TO_DIAMOND before invoking.
config/dexs.json (1)
592-597: Approve — hemi DEX allowlist added; order correct. Add a short comment/source link for these four addresses in the PR description for audit traceability (config/dexs.json lines 592–597).config/networks.json (1)
444-465: Hemi network block — verified; confirm remaining items.
- ChainId/explorer: chainId 43111 and explorerApiUrl (https://explorer.hemi.xyz/api) match foundry.toml (line ~121) and config/networks.json — OK.
- RPC: networks.json lists https://rpc.hemi.network/rpc but foundry.toml uses ETH_NODE_URI_HEMI (line ~52). Confirm ETH_NODE_URI_HEMI points to a reliable RPC for forge/broadcast.
- GasZip: networks.json gasZipChainId = 397 and config/gaszip.json has gasZipRouters.hemi = 0x2a37D63EAdFe4b4682a3c28C1c2cD4F109Cc2762. Confirm GasZip backend maps short/ID 397 to hemi.
- Safe: networks.json safeAddress = 0xAd6912d8c99B43D9602a31146b5B236593B152BA — confirm this is the intended production SAFE.
script/helperFunctions.sh (1)
3181-3186: Propagation of Security contracts into addNewNetworkWithAllIncludedContractsInLatestVersions – LGTMContracts now include Security; aligns with the new helper. No issues.
script/tasks/diamondUpdatePeriphery.sh (4)
8-10: Avoid re-sourcing .env in child task – LGTMKeeping .env sourcing in the parent prevents env overrides. Good.
70-74: UPDATE_ALL path includes Security – LGTMCombining Periphery and Security via helpers is correct.
156-156: Quote command substitution already applied – LGTMRPC URL retrieval is now safely quoted.
172-176: codesize check fixed to numeric compare – LGTMUsing -eq on the numeric output of cast codesize is correct.
config/permit2Proxy.json (1)
24-24: Confirm hemi Permit2 addressAdded hemi -> 0x0000…BA3. Please confirm hemi’s Permit2 is deployed at this canonical address for its chainId; this file feeds Permit2Proxy constructor args.
deployments/hemi.diamond.json (1)
1-73: New hemi diamond manifest – cross-check consistencyLooks consistent. Please verify:
- Each Periphery address matches deployments/hemi.json.
- Facet Names/Versions align with script/deploy/_targetState.json for hemi.production.
script/deploy/deployAllContracts.sh (1)
372-375: Auto-refresh diamond logs post run – LGTMUnconditionally updating logs at the end is helpful and harmless in prod (read-only chain calls; writes local files).
deployments/hemi.json (1)
1-25: New hemi deployments map – sanity checkAddresses look well-formed. Please confirm LiFiDiamond here is the one used by deployments/hemi.diamond.json and that Permit2Proxy matches config/permit2Proxy.json usage.
deployments/_deployments_log_file.json (14)
2132-2147: DiamondLoupeFacet (hemi) entry OK.Fields present; version pinned at 1.0.0 with empty constructor as expected. Covered by the verification script.
3205-3220: OwnershipFacet (hemi) entry OK.No ctor args; matches typical pattern. Covered by the verification script.
4580-4595: DexManagerFacet 1.0.2 (hemi) recorded.Version matches AI summary. Please ensure the same version is targeted in script/deploy/_targetState.json for hemi.
5663-5678: AccessManagerFacet (hemi) entry OK.Schema/fields look consistent. Covered by the verification script.
6727-6742: WithdrawFacet (hemi) entry OK.Empty ctor args expected. Covered by the verification script.
7800-7815: PeripheryRegistryFacet (hemi) entry OK.No issues spotted. Covered by the verification script.
10958-10973: LiFiDiamondImmutable (hemi) entry OK.No ctor args; VERIFIED=true. Covered by the verification script.
16044-16059: Executor 2.1.0 (hemi): cross‑refs look consistent.Ctor args reference 0xEF3D… (proxy) and 0x156c… (receiver/admin). Please confirm both correspond to the hemi entries added in this PR.
25367-25382: Receiver (hemi) entry OK.No ctor args; consistent with schema. Covered by the verification script.
29686-29701: GasRebateDistributor 1.0.2 (hemi): zeroed extra arg OK.Args show a zeroed bytes32; expected if no Merkle root at deploy. Covered by the verification script.
32251-32266: LiFiDEXAggregator 1.12.0 (hemi): references look plausible.Ctor includes owner 0x156c…, deployer 0xD387…, and config blob. Ensure 0xD387… is the intended ops/deployer for hemi.
35828-35843: AcrossFacetV3 1.0.4 (hemi): Permit2 ref appears correct.Ctor contains 0x…22d47303… (Uniswap Permit2). Verify the other addresses (diamond and filler) match hemi config.
37849-37864: GasZipPeriphery 1.0.1 (hemi): cross‑refs to router/aggregator.Ctor args include router 0x2a37d6…, aggregator 0x588c00… (LiFiDEXAggregator), and 0xAD6912… (likely WETH). Verify each matches hemi.
1058-1072: Add hemi block: verify cross-file consistencyRun the script below from the repository root; it compares deployments/hemi.json with deployments/_deployments_log_file.json and reports DIFFs and non-VERIFIED entries.
#!/usr/bin/env python3 import json, sys, os LOG = "deployments/_deployments_log_file.json" HEMI = "deployments/hemi.json" if not os.path.isfile(LOG): print(f"MISSING: {LOG}"); sys.exit(3) if not os.path.isfile(HEMI): print(f"MISSING: {HEMI}"); sys.exit(3) def load(path): with open(path, "r") as f: return json.load(f) def extract_addr(entry): if isinstance(entry, str): return entry.lower() if not isinstance(entry, dict): return None for k in ("ADDRESS", "address", "addr"): v = entry.get(k) if v: return str(v).lower() return None log = load(LOG) hemi = load(HEMI) # build latest production addresses from log log_addrs = {} for name, val in log.items(): if not isinstance(val, dict): continue hemi_obj = val.get("hemi") if not isinstance(hemi_obj, dict): continue prod = hemi_obj.get("production") if not isinstance(prod, dict): continue keys = sorted(prod.keys()) if not keys: continue last = prod[keys[-1]] entry = None if isinstance(last, list) and last: entry = last[0] elif isinstance(last, dict): entry = last addr = extract_addr(entry) if addr: log_addrs[name] = addr # build addresses from deployments/hemi.json hemi_addrs = {} for k, v in hemi.items(): if isinstance(v, str): hemi_addrs[k] = v.lower() elif isinstance(v, dict): for key in ("address", "ADDRESS", "addr"): if key in v and v[key]: hemi_addrs[k] = str(v[key]).lower() break # compare mismatches = [] for name in sorted(set(list(hemi_addrs.keys()) + list(log_addrs.keys()))): a = hemi_addrs.get(name, "-") b = log_addrs.get(name, "-") if a != b: mismatches.append((name, a, b)) # check VERIFIED flags (accept boolean true or string "true") non_verified = [] for name, val in log.items(): if not isinstance(val, dict): continue hemi_obj = val.get("hemi") if not isinstance(hemi_obj, dict): continue prod = hemi_obj.get("production") if not isinstance(prod, dict): continue for ver, arr in prod.items(): rec = None if isinstance(arr, list) and arr: rec = arr[0] elif isinstance(arr, dict): rec = arr if not isinstance(rec, dict): non_verified.append(f"{name}@{ver} VERIFIED=<missing>") continue v = rec.get("VERIFIED", rec.get("verified", None)) if not (v is True or v == "true"): non_verified.append(f"{name}@{ver} VERIFIED={v}") print("== Address mismatches (hemi.json vs _deployments_log_file.json) ==") if not mismatches: print("(none)") else: for m in mismatches: print("DIFF\t{}\t{}\t{}".format(m[0], m[1], m[2])) print("== Non-VERIFIED hemi entries (should be empty) ==") if not non_verified: print("(none)") else: for n in non_verified: print(n) if mismatches or non_verified: sys.exit(4) print("OK: hemi addresses match and all hemi production entries are VERIFIED=true")script/deploy/_targetState.json (1)
724-752: Per maintainer policy, skipping review of _targetState.json.Following retrieved learnings from 0xDEnYO, this file should not be reviewed or discussed in PRs. No content feedback provided.
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script/deploy/deployPeripheryContracts.sh (1)
63-64: Restore exit-status handling for deploySingleContract.The previous post-deploy check was removed; failures now risk being silently ignored (unless set -e is globally active). Guard and propagate errors.
Apply:
- deploySingleContract "$CONTRACT" "$NETWORK" "$ENVIRONMENT" "$CURRENT_VERSION" + if ! deploySingleContract "$CONTRACT" "$NETWORK" "$ENVIRONMENT" "$CURRENT_VERSION"; then + checkFailure 1 "deployment failed for $CONTRACT on $NETWORK/$ENVIRONMENT (version $CURRENT_VERSION)" + return 1 + fi
🧹 Nitpick comments (2)
script/deploy/deployPeripheryContracts.sh (2)
36-44: Merging security+periphery is fine; switch to real arrays to avoid word-splitting/globbing pitfalls.Current whitespace-joined list can misbehave under set -f or unusual names. Use arrays and iterate safely.
Apply:
- # get names of all security contracts (that are not excluded in config) - SECURITY_CONTRACTS=$(getIncludedSecurityContractsArray) - - # combine periphery and security contracts - AUXILIARY_CONTRACTS="$PERIPHERY_CONTRACTS $SECURITY_CONTRACTS" - - # loop through all contracts - for CONTRACT in $AUXILIARY_CONTRACTS; do + # get names of all security contracts (that are not excluded in config) + read -r -a SECURITY_CONTRACTS <<< "$(getIncludedSecurityContractsArray)" + read -r -a PERIPHERY_CONTRACTS <<< "$(getIncludedPeripheryContractsArray)" + + # combine periphery and security contracts + local -a AUXILIARY_CONTRACTS=("${PERIPHERY_CONTRACTS[@]}" "${SECURITY_CONTRACTS[@]}") + + # loop through all contracts + for CONTRACT in "${AUXILIARY_CONTRACTS[@]}"; doNote: Nice improvement on naming (AUXILIARY_CONTRACTS) to avoid confusion with repo-wide ALL_CONTRACTS.
66-69: Good early-continue; consider tracking and summarizing skipped contracts.Helps operators quickly see what didn’t deploy and why.
Apply:
- # loop through all contracts - for CONTRACT in $AUXILIARY_CONTRACTS; do + # loop through all contracts + local -a SKIPPED_CONTRACTS=() + for CONTRACT in $AUXILIARY_CONTRACTS; do @@ - warning "target state version does not match with current version (contract=$CONTRACT, target_version=$TARGET_VERSION, current_version=$CURRENT_VERSION) >> contract will not be deployed" + warning "target state version does not match with current version (contract=$CONTRACT, target_version=$TARGET_VERSION, current_version=$CURRENT_VERSION) >> contract will not be deployed" + SKIPPED_CONTRACTS+=("$CONTRACT") continue @@ - echo "[info] <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< periphery contracts deployed (please check for warnings)" + if ((${#SKIPPED_CONTRACTS[@]})); then + echo "[info] skipped (${#SKIPPED_CONTRACTS[@]}): ${SKIPPED_CONTRACTS[*]}" + fi + echo "[info] <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< periphery contracts deployed (please check for warnings)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
script/deploy/deployPeripheryContracts.sh(2 hunks)script/tasks/diamondUpdatePeriphery.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- script/tasks/diamondUpdatePeriphery.sh
🧰 Additional context used
📓 Path-based instructions (2)
script/**/*.sh
📄 CodeRabbit inference engine (conventions.md)
script/**/*.sh: Bash scripts must start with #!/bin/bash, be modular with sections (Logging, Error handling and logging, Deployment functions), follow DRY via helpers, and organize core operations into functions
Bash style: consistent indentation/naming, comments, usage instructions, and TODO/limitations documented
Bash error handling and logging: use helper logging functions, validate inputs early, check exit status with checkFailure, and use set -e where appropriate
Files:
script/deploy/deployPeripheryContracts.sh
{script/**/*.sh,preinstall.sh}
📄 CodeRabbit inference engine (conventions.md)
Environment: load from .env or config.sh, declare globals in config, update .env.example, validate env vars early; add system packages to preinstall.sh
Files:
script/deploy/deployPeripheryContracts.sh
🧠 Learnings (8)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1168
File: script/deploy/_targetState.json:1564-1589
Timestamp: 2025-05-27T12:36:26.987Z
Learning: When reviewing deployment PRs in the lifinance/contracts repository, target state configuration files (like script/deploy/_targetState.json) may be updated for multiple networks even when the PR is focused on deploying to a specific network. The scope should be determined by the PR title and description, not just by all configuration changes present in the files.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/deploy/deployPeripheryContracts.sh
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
PR: lifinance/contracts#1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/deploy/deployPeripheryContracts.sh
📚 Learning: 2025-06-05T10:00:01.583Z
Learnt from: mirooon
PR: lifinance/contracts#1145
File: script/tasks/diamondSyncWhitelistedAddresses.sh:208-209
Timestamp: 2025-06-05T10:00:01.583Z
Learning: Task scripts in script/tasks/ directory (like diamondSyncWhitelistedAddresses.sh) define functions that are sourced by deployAllContracts.sh and called from there, rather than executing directly. They don't need to call their own functions at the end of the file.
Applied to files:
script/deploy/deployPeripheryContracts.sh
📚 Learning: 2024-11-26T01:50:51.809Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#874
File: script/tasks/diamondSyncSigs.sh:88-88
Timestamp: 2024-11-26T01:50:51.809Z
Learning: In the 'lifinance/contracts' repository, scripts like 'script/tasks/diamondSyncSigs.sh' are executed only on local machines where private keys are securely managed, so additional private key security improvements are not required.
Applied to files:
script/deploy/deployPeripheryContracts.sh
📚 Learning: 2025-08-07T06:34:07.709Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1316
File: script/deploy/deployAllContracts.sh:346-350
Timestamp: 2025-08-07T06:34:07.709Z
Learning: In script/deploy/deployAllContracts.sh, 0xDEnYO prefers not to add automated retry mechanisms for cast send commands in deployment scripts, preferring manual re-execution if network issues occur. This prioritizes simplicity over automated error handling.
Applied to files:
script/deploy/deployPeripheryContracts.sh
📚 Learning: 2024-11-26T01:18:52.125Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: script/deploy/deploySingleContract.sh:231-231
Timestamp: 2024-11-26T01:18:52.125Z
Learning: In the `deploySingleContract.sh` script, environment variable assignments preceding the `forge script` command (e.g., `DEPLOYSALT=$DEPLOYSALT ... forge script ...`) are acceptable and function correctly, even when variable expansions like `$DIAMOND_TYPE` and `$NETWORK` are used later in the same command.
Applied to files:
script/deploy/deployPeripheryContracts.sh
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to script/deploy/targetState.json : Maintain targetState.json to track expected versions per network/environment and ensure deployment version consistency
Applied to files:
script/deploy/deployPeripheryContracts.sh
🧬 Code graph analysis (1)
script/deploy/deployPeripheryContracts.sh (1)
script/helperFunctions.sh (1)
getIncludedSecurityContractsArray(2895-2912)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (1)
script/deploy/deployPeripheryContracts.sh (1)
10-11: OK to avoid re-sourcing .env — add a guard or ensure callers source it.Search of script/ found only the function definition (script/deploy/deployPeripheryContracts.sh:5–10); no call sites located. Add a lightweight guard to source .env when required env vars are unset, or document/ensure every caller sources .env before invoking this function.
Which Jira task belongs to this PR?
https://lifi.atlassian.net/browse/LF-14914
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)