Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates network configuration and RPC endpoints, refactors etherscan API key handling in foundry.toml to use explicit environment variables, introduces MongoDB-backed Safe proposal verification and querying capabilities, and adds a comprehensive Periphery contract registration checker across networks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~27 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@config/networks.json`:
- Around line 292-294: Update the Cronos explorer references: in the JSON object
containing "verificationType": "blockscout" replace the deprecated "explorerUrl"
value "https://cronoscan.com" with "https://explorer.cronos.org" and also change
"explorerApiUrl" from "https://cronos.org/explorer/api" to the current official
API base at "https://explorer.cronos.org/api" (ensure keys verificationType,
explorerUrl, and explorerApiUrl are updated together for consistency).
In `@deployments/opbnb.diamond.json`:
- Line 70: The Periphery object currently uses one composite key string which
breaks scripts expecting Periphery[contractName]; replace that single composite
key with individual keys for each contract (e.g., "ERC20Proxy", "Executor",
"FeeCollector", "FeeForwarder", "GasZipPeriphery", "LiFiDEXAggregator",
"LidoWrapper", "Patcher", "Permit2Proxy", "ReceiverAcrossV3",
"ReceiverAcrossV4", "ReceiverChainflip", "ReceiverOIF", "ReceiverStargateV2",
"TokenWrapper") and set each value to an empty string to match the structure
used by other deployment files so scripts like script/deploy/tron/utils.ts and
script/deploy/updateDiamondLog.ts can access Periphery[name] correctly.
In `@foundry.toml`:
- Line 31: The getZkSolcVersion function in script/helperFunctions.sh currently
greps for the old header "[profile.default.zksync]" and thus misses the new
inline table format; update getZkSolcVersion to parse foundry.toml for either an
inline table like "zksync = { zksolc = '1.5.15' }" (under [profile] or
[profile.zksync]) or the legacy header, and extract the zksolc value into
ZK_SOLC_VERSION (e.g., use a regex or toml-aware awk/grep that matches zksolc =
'...' or zksolc = "..." and assigns the captured version to the ZK_SOLC_VERSION
variable), ensuring the function still falls back to previous behavior if only
the old format exists.
- Line 106: The verifier endpoint URLs for the chain entries named "abstract",
"lens", and "sophon" are incorrect and must be updated: in the "abstract" entry
(abstract = { ... }) replace the url value
"https://abscan.org/contract_verification" with "https://api.abscan.org/api"; in
the "lens" entry (lens = { ... }) replace
"https://api-explorer-verify.lens.matterhosted.dev/contract_verification" with
"https://verify.lens.xyz/contract_verification"; and in the "sophon" entry
(sophon = { ... }) replace "https://explorer.sophon.xyz/contract_verification"
with "https://verification-explorer.sophon.xyz/contract_verification". Ensure
you only change the url fields for those named entries and do not modify the
"plasma" entry.
In `@GasZipPeriphery_standard_json.json`:
- Around line 1-39: The standard JSON has evmVersion set to "cancun" which
doesn't match the deployment compiler settings used for GasZipPeriphery
(deployed with "london"), causing verification to fail; regenerate the
standard-json for the contract (GasZipPeriphery) with evmVersion set to "london"
to match config/networks.json (or produce two separate standard_json artifacts
grouping contracts by evmVersion: one with "london" and one with "cancun") and
replace the current evmVersion field in the artifact accordingly so
bytecode/metadata match the deployed contracts.
In `@generate_complete_standard_json.sh`:
- Around line 4-60: The script currently hardcodes evmVersion ("cancun") and
optimizer runs (1000000) in the produced JSON; update
generate_complete_standard_json.sh to read foundry.toml and derive the actual
settings instead: parse foundry.toml (respecting the default profile) to extract
evm_version and optimizer.runs (falling back to sensible defaults if keys are
missing), then pass those extracted values into the jq invocation that builds
the JSON so evmVersion and settings.optimizer.runs come from the foundry.toml
values rather than being hardcoded; keep using the existing variables (CONTRACT,
OUTPUT, FLATTENED) and ensure error handling if foundry.toml cannot be read.
In `@script/helperFunctions.sh`:
- Around line 1189-1195: Validation rejects networks that intentionally set
NO_ETHERSCAN_API_KEY_REQUIRED because the allowlist check for optional keys in
the block that reads BLOCKEXPLORER_API_KEY uses KEY_VAR whitelist but doesn't
include NO_ETHERSCAN_API_KEY_REQUIRED; update that conditional in the same block
(the if checking [[ -z "$BLOCKEXPLORER_API_KEY" ]] && [[ "$KEY_VAR" !=
"BLOCKSCOUT_API_KEY" ]] && [[ "$KEY_VAR" != "ZKSYNC_NATIVE_VERIFIER" ]] && [[
"$KEY_VAR" != "VERIFY_CONTRACT_API_KEY" ]]) to also allow KEY_VAR ==
"NO_ETHERSCAN_API_KEY_REQUIRED" so networks using that placeholder pass
validation.
In `@script/multiNetworkExecution.sh`:
- Line 2591: The NETWORKS array is defined with commas so elements include
trailing commas; update the NETWORKS declaration (variable name NETWORKS) to use
space-separated elements instead of commas so the entries become "avalanche",
"nibiru", and "viction" as separate items (e.g., replace the comma-separated
form with a space-separated array literal).
In `@script/tasks/checkExecutorAndReceiver.sh`:
- Around line 58-76: The script currently only records failures when
RAW_RETURN_DATA matches the empty logs/returns pattern; when executeAndParse
fails but RAW_RETURN_DATA doesn't match that exact pattern the failure is
silent—update the check around executeAndParse/RAW_RETURN_DATA/RETURN_CODE to
append a failure entry to FAILED_RESULTS and emit an error for any
non-successful outcome that doesn't match the expected JSON pattern: inside the
same conditional block that inspects RAW_RETURN_DATA and RETURN_CODE
(referencing executeAndParse, RAW_RETURN_DATA, RETURN_CODE, and FAILED_RESULTS),
add an else branch that logs a descriptive error (including RAW_RETURN_DATA and
RETURN_CODE) and appends "[info] Failed to check on network: $NETWORK" to
FAILED_RESULTS so all unexpected/failed executions are recorded.
🧹 Nitpick comments (2)
generate_complete_standard_json.sh (1)
1-13: Align the script with repo bash conventions.
Consider sourcingscript/helperFunctions.sh/script/playgroundHelpers.sh, wrapping logic in amain()function, and using[[ ... ]]checks for consistency. As per coding guidelines, keep bash scripts structured and reuse helper utilities.script/tasks/checkExecutorAndReceiver.sh (1)
58-61: Source helper functions locally for defensive compatibility.The function uses
executeAndParse,error, andgetIncludedNetworksArray, which are only available when called viascriptMaster.sh. Adding local sourcing of helpers ensures the function works if called from any context. This pattern is already followed bydiamondSyncWhitelist.shin the same directory.Suggested change
checkExecutorAndReceiver() { # load env variables source .env + source script/config.sh + source script/helperFunctions.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@script/deploy/safe/query-safe-proposals.ts`:
- Around line 54-79: The plaintext substring check against hex calldata fails;
update the matching in the block that handles periphery contracts (the logic
that checks `contract && !contract.includes('Facet')`) to first verify the
function selector for registerPeripheryContract is present in `calldata` and
then hex-encode the contract name (UTF-8 -> hex, padded as ABI string would be)
before searching for it in `calldata`; use the existing `calldata` and
`contract` variables and only return `proposal` when both the selector and the
encoded contract name are found to avoid returning an unverified proposal.
In `@script/deploy/safe/verify-proposal-exists.sh`:
- Around line 67-90: The script uses the bash-only `local` keyword at top-level
which fails under set -euo pipefail; remove `local` from the three top-level
declarations so they become normal variables: change `local ERROR_MSG=$(echo
"$MONGO_RESULT" | jq -r '.error // "Unknown error"')`, `local
SAFE_TX_HASH=$(echo "$MONGO_RESULT" | jq -r '.safeTxHash // "unknown"')`, and
`local TIMESTAMP=$(echo "$MONGO_RESULT" | jq -r '.timestamp // "unknown"')` to
omit `local` while keeping the same variable names and parsing logic (these
appear alongside MONGO_EXIT, MONGO_RESULT, and FOUND handling).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@script/tasks/checkContractRegistrationPerNetwork.ts`:
- Around line 293-296: The RPC override in networkConfig.rpcUrl is taking
precedence over environment overrides because getViemChainForNetworkName already
resolves ETH_NODE_URI_*; to fix, prefer the chain RPC URL returned by
getViemChainForNetworkName (chain.rpcUrls.default.http[0]) first and fall back
to networkConfig.rpcUrl (or simply use chain.rpcUrls directly) when computing
rpcUrl so environment ETH_NODE_URI_* values are honored; update the assignment
that defines rpcUrl to use chain.rpcUrls.default.http[0] || networkConfig.rpcUrl
(or remove networkConfig.rpcUrl usage and rely solely on chain).
- Around line 353-375: Replace the top-level async main() with a citty command
using defineCommand and runMain (follow pattern from
updateWhitelistPeriphery.ts) and validate CLI args: ensure environment is one of
'production' or 'staging' (reject/exit on invalid strings) and validate
contractName (no path separators or traversal chars, e.g., allow strict
/^[A-Za-z0-9_]+$/) before using it in artifact paths like
`_artifacts/${contractName}.sol` and `_artifacts/${contractName}.json`; update
code paths that reference main() to call runMain(command) and return/exit on
invalid inputs.
- Around line 57-98: The function getFacetSelectors currently shells out to
`cast sig` and builds signatures manually (vulnerable and incorrect for
tuples/arrays); replace that logic by importing viem's toFunctionSelector and
use it on the ABI entry itself (e.g., call toFunctionSelector(item as unknown as
AbiFunction) instead of execSync and slicing) to compute the selector, remove
the child_process execSync usage and the manual signature construction, and push
the returned selector into selectors; ensure the ABI item is typed as a function
ABI when passed to toFunctionSelector and remove any fragile string
slicing/casting logic.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@script/tasks/temp/checkContractRegistrationPerNetwork.ts`:
- Around line 131-133: The current check uses readFileSync(abiPath, 'utf8')
which will throw if the file is missing; change the logic to first call
existsSync(abiPath) (existsSync is already imported) and if it returns false
throw the Error(`ABI file not found: ${abiPath}`), then only call
readFileSync(abiPath, 'utf8') to read the ABI; update the code paths around
abiPath, readFileSync, and existsSync accordingly.
🧹 Nitpick comments (2)
script/deploy/safe/confirm-safe-tx.ts (1)
838-840: Consider using existingtx.canExecuteproperty instead of recomputing.
canExecuteNowduplicates the computation already performed at line 690 wheretx.canExecuteis set using the samehasEnoughSignatures(safeTransaction, threshold)call. Usingtx.canExecutedirectly would be cleaner.Suggested simplification
- const canExecuteNow = hasEnoughSignatures(tx.safeTransaction, tx.threshold) + const canExecuteNow = tx.canExecute const isDeployerCurrentSigner = !useLedger && privKeyType === PrivateKeyTypeEnum.DEPLOYERscript/tasks/temp/checkContractRegistrationPerNetwork.ts (1)
531-534: No-op filter:name in networksis always true.The filter condition
name in networksalways evaluates to true sincevalidNetworkNamesis derived fromObject.keys(networks). This type guard doesn't provide value.♻️ Suggested simplification
- // Filter to only valid SupportedChain networks - const validNetworkNames = Object.keys(networks).filter( - (name): name is SupportedChain => name in networks - ) + // Get all network names as SupportedChain + const validNetworkNames = Object.keys(networks) as SupportedChain[]Or if type safety is the goal, use the
SupportedChaintype directly from imports.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script/deploy/safe/confirm-safe-tx.ts (1)
1079-1085: Critical logic bug: Missing braces cause "Execute with Deployer" to bypass safety checks.The
ifstatement on lines 1079-1082 lacks braces, so onlyoptions.push('Execute')is conditional. Line 1084's statement executes unconditionally within the else block, offering "Execute with Deployer" even when:
safetyBlockeris present- Transaction doesn't have enough signatures
🐛 Proposed fix
if ( !safetyBlocker && hasEnoughSignatures(tx.safeTransaction, tx.threshold) - ) + ) { options.push('Execute') if (!isDeployerCurrentSigner) options.push('Execute with Deployer') }
🤖 Fix all issues with AI agents
In `@script/deploy/safe/confirm-safe-tx.ts`:
- Around line 1050-1053: The SAFE_SIGNER branch currently pushes 'Execute' and
'Execute with Deployer' into options when canExecuteNow is true but does not
verify safetyBlocker; update the SAFE_SIGNER branch that contains the
options.push('Execute') and options.push('Execute with Deployer') to require
both canExecuteNow && !safetyBlocker before adding these options so blocked
transactions cannot be executed by SAFE_SIGNER, mirroring the DEPLOYER branch's
behavior.
🧹 Nitpick comments (1)
script/deploy/safe/confirm-safe-tx.ts (1)
1216-1233: LGTM - New handler follows established patterns.The "Execute with Deployer" action handler correctly initializes the deployer wallet and executes the transaction. Error handling is consistent with other action handlers in the file.
Consider extracting the deployer wallet initialization (lines 1220-1229) into a helper function to reduce duplication with lines 1154-1164 in the "Sign and Execute With Deployer" handler.
…ts into deploy-gaszipperiphery
…oy-gaszipperiphery
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@config/whitelist.json`:
- Around line 5353-5355: Revert the 61 GasZipPeriphery entries you added to
config/whitelist.json on this feature branch (the object with "name":
"GasZipPeriphery" and its "address"/"selectors" updates) and remove them from
this PR, then create a separate PR targeted at the main branch containing only
the whitelist modifications (the GasZipPeriphery address updates verified
against deployments/_deployments_log_file.json); ensure the feature-branch PR
contains no whitelist.json changes before merging here and that the new
main-targeted PR includes the same entries and a brief note referencing the
deployment manifest.
🧹 Nitpick comments (5)
script/helperFunctions.sh (5)
1878-1913: Redundant duplicate exports ofNO_ETHERSCAN_API_KEY_REQUIRED.Lines 1883-1884, 1894-1896, and 1900-1902 all check and export
NO_ETHERSCAN_API_KEY_REQUIRED=""but this was already handled at lines 1873-1875. Since the export at 1873-1875 runs unconditionally whenAPI_KEY = "NO_ETHERSCAN_API_KEY_REQUIRED", the subsequent checks inside the conditional branches are redundant.♻️ Consider removing redundant exports
if isZkEvmNetwork "$NETWORK"; then VERIFY_CMD+=("--verifier" "zksync") - # For zkSync native verifier, API key is optional. Export empty string if not set to avoid Foundry errors - if [[ "$API_KEY" = "NO_ETHERSCAN_API_KEY_REQUIRED" ]] && [[ -z "${NO_ETHERSCAN_API_KEY_REQUIRED:-}" ]]; then - export NO_ETHERSCAN_API_KEY_REQUIRED="" - fi else # Check verificationType from networks.json for Blockscout local VERIFICATION_TYPE VERIFICATION_TYPE=$(jq -r --arg network "$NETWORK" '.[$network].verificationType // empty' "$NETWORKS_JSON_FILE_PATH" 2>/dev/null) if [[ "$VERIFICATION_TYPE" = "blockscout" ]]; then VERIFY_CMD+=("--verifier" "blockscout") - # For Blockscout, API key is optional. Export empty string if not set to avoid Foundry errors - if [[ "$API_KEY" = "NO_ETHERSCAN_API_KEY_REQUIRED" ]] && [[ -z "${NO_ETHERSCAN_API_KEY_REQUIRED:-}" ]]; then - export NO_ETHERSCAN_API_KEY_REQUIRED="" - fi elif [[ "$VERIFICATION_TYPE" = "sourcify" ]]; then VERIFY_CMD+=("--verifier" "sourcify") - # For Sourcify, API key is optional. Export empty string if not set to avoid Foundry errors - if [[ "$API_KEY" = "NO_ETHERSCAN_API_KEY_REQUIRED" ]] && [[ -z "${NO_ETHERSCAN_API_KEY_REQUIRED:-}" ]]; then - export NO_ETHERSCAN_API_KEY_REQUIRED="" - fi elif [ "$API_KEY" = "MAINNET_ETHERSCAN_API_KEY" ]; then
1916-1938: Shellcheck SC2155: Declare and assign separately to avoid masking return values.Lines 1926-1927 combine declaration and assignment, which masks the return value of the command substitution.
♻️ Separate declaration and assignment
while IFS= read -r flag_entry; do - local flag_name=$(echo "$flag_entry" | jq -r '.key') - local flag_value=$(echo "$flag_entry" | jq -r '.value') + local flag_name + local flag_value + flag_name=$(echo "$flag_entry" | jq -r '.key') + flag_value=$(echo "$flag_entry" | jq -r '.value') if [[ -n "$flag_name" ]] && [[ "$flag_name" != "null" ]]; then
1995-2000: Shellcheck SC2155: Declare and assign separately.Lines 1996-1997 combine declaration and assignment.
♻️ Separate declaration and assignment
- local RESPONSE=$(echo "$VERIFY_OUTPUT" | grep -i "Response:" | tail -1 | sed -E 's/.*Response:[[:space:]]*([^[:space:]]+).*/\1/' | tr -d '`' | tr -d "'") - local DETAILS=$(echo "$VERIFY_OUTPUT" | grep -i "Details:" | tail -1 | sed -E "s/.*Details:[[:space:]]*['\`]?([^'\`]+)['\`]?.*/\1/" | head -1) + local RESPONSE + local DETAILS + RESPONSE=$(echo "$VERIFY_OUTPUT" | grep -i "Response:" | tail -1 | sed -E 's/.*Response:[[:space:]]*([^[:space:]]+).*/\1/' | tr -d '`' | tr -d "'") + DETAILS=$(echo "$VERIFY_OUTPUT" | grep -i "Details:" | tail -1 | sed -E "s/.*Details:[[:space:]]*['\`]?([^'\`]+)['\`]?.*/\1/" | head -1)
2008-2046: Good improvement: Always verify final status with verify-check.The addition of
verify-checkafter initial verification prevents false positives where the API returns OK but the contract isn't actually verified. This is a valuable reliability improvement.However, lines 2025, 2042-2043 have SC2155 warnings (declare and assign separately).
♻️ Separate declaration and assignment for SC2155 compliance
- local CHECK_OUTPUT=$(FOUNDRY_LOG=trace "${CHECK_CMD[@]}" 2>&1) || CHECK_EXIT_CODE=$? + local CHECK_OUTPUT + CHECK_OUTPUT=$(FOUNDRY_LOG=trace "${CHECK_CMD[@]}" 2>&1) || CHECK_EXIT_CODE=$?And for lines 2042-2043:
- local FINAL_RESPONSE=$(echo "$CHECK_OUTPUT" | grep -i "Response:" | tail -1 | sed -E 's/.*Response:[[:space:]]*([^[:space:]]+).*/\1/' | tr -d '`' | tr -d "'") - local FINAL_DETAILS=$(echo "$CHECK_OUTPUT" | grep -i "Details:" | tail -1 | sed -E "s/.*Details:[[:space:]]*['\`]?([^'\`]+)['\`]?.*/\1/" | head -1) + local FINAL_RESPONSE + local FINAL_DETAILS + FINAL_RESPONSE=$(echo "$CHECK_OUTPUT" | grep -i "Response:" | tail -1 | sed -E 's/.*Response:[[:space:]]*([^[:space:]]+).*/\1/' | tr -d '`' | tr -d "'") + FINAL_DETAILS=$(echo "$CHECK_OUTPUT" | grep -i "Details:" | tail -1 | sed -E "s/.*Details:[[:space:]]*['\`]?([^'\`]+)['\`]?.*/\1/" | head -1)
2049-2091: LGTM - Robust verification status handling.The comprehensive handling of different verification states (Pass, Verified, Success, Pending, Fail) with appropriate retries improves reliability. The additional 30-second wait for pending status before re-checking is appropriate.
Lines 2076-2077 have SC2155 warnings that should be addressed with the same pattern (separate declaration and assignment).
♻️ Separate declaration and assignment for lines 2076-2077
- local RETRY_RESPONSE=$(echo "$CHECK_OUTPUT" | grep -i "Response:" | tail -1 | sed -E 's/.*Response:[[:space:]]*([^[:space:]]+).*/\1/' | tr -d '`' | tr -d "'") - local RETRY_DETAILS=$(echo "$CHECK_OUTPUT" | grep -i "Details:" | tail -1 | sed -E "s/.*Details:[[:space:]]*['\`]?([^'\`]+)['\`]?.*/\1/" | head -1) + local RETRY_RESPONSE + local RETRY_DETAILS + RETRY_RESPONSE=$(echo "$CHECK_OUTPUT" | grep -i "Response:" | tail -1 | sed -E 's/.*Response:[[:space:]]*([^[:space:]]+).*/\1/' | tr -d '`' | tr -d "'") + RETRY_DETAILS=$(echo "$CHECK_OUTPUT" | grep -i "Details:" | tail -1 | sed -E "s/.*Details:[[:space:]]*['\`]?([^'\`]+)['\`]?.*/\1/" | head -1)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@script/tasks/temp/checkContractRegistrationPerNetwork.ts`:
- Around line 167-200: The current try/catch around the whitelist checks
swallows all errors and returns false, conflating RPC/ABI failures with a
genuine “not whitelisted” result; remove the catch or rethrow errors so
exceptions from publicClient.readContract (calls to
isContractSelectorWhitelisted and getWhitelistedSelectorsForContract) propagate
to the caller and can be handled as unknown (null) instead of false; ensure the
logic in the block that references whitelistPeripheryFunctions, selector
handling, and the whitelistedSelectors return stays the same but does not
catch-and-return-false.
🧹 Nitpick comments (2)
script/multiNetworkExecution.sh (1)
223-254: Consider validatingbundependency.The function depends on
bunto execute the TypeScript script, but unlike other tools validated inrequireTools(),bunavailability is not checked. This could lead to confusing errors ifbunis not installed.💡 Optional: Add bun dependency check
verifyProposalExistsInMongo() { local NETWORK="${1:-}" local ENVIRONMENT="${2:-}" local CONTRACT="${3:-}" + # Check if bun is available + if ! command -v bun >/dev/null 2>&1; then + error "verifyProposalExistsInMongo: 'bun' is required but not installed" + return 1 + fi + if [[ -z "$NETWORK" ]]; then error "verifyProposalExistsInMongo: Network parameter is required" return 1 fiAlternatively, add
bunto therequireTools()function if it's needed globally.script/tasks/temp/checkContractRegistrationPerNetwork.ts (1)
281-287: Avoid falling back tonetworks.jsonfor RPC URLs.Prefer env-provided RPCs (or the viem chain’s resolved URL) and skip the networks.json fallback to align with repo expectations.
♻️ Proposed fix
- const rpcUrl = - process.env[rpcEnvVarName] || - networkConfig.rpcUrl || - chain.rpcUrls.default.http[0] + const rpcUrl = + process.env[rpcEnvVarName] || + chain.rpcUrls.default.http[0]Based on learnings: In the LiFi contracts repository, RPC URLs should be strictly sourced from environment variables without fallback to networks.json configuration.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@foundry.toml`:
- Around line 120-168: The PR adds new environment variables referenced in
foundry.toml (see keys used by bob/unichain/flow/etc. referencing
BLOCKSCOUT_API_KEY, botanix/metis/plasma referencing VERIFY_CONTRACT_API_KEY,
lens referencing LENS_ETHERSCAN_API_KEY, ronin referencing SOURCIFY_API_KEY, and
corn referencing CORN_ETHERSCAN_API_KEY) but .env.example wasn't updated; add
these five vars to the "Mainnet Explorer API Keys" section of .env.example with
brief placeholder values and comments matching existing style so developers know
to supply real API keys.
🧹 Nitpick comments (1)
script/multiNetworkExecution.sh (1)
217-292: Well-structured MongoDB verification helper.The function has good input validation, proper
set +e/set -eguarding, structured JSON parsing, and clear error/success messaging using existing helpers.One minor note:
verifyProposalExistsInMongois not included in theexport -flist (lines 3147–3164), unlike all other public helpers. If this function is ever called from abash -ccontext or an independently spawned shell, it won't be available.Suggested addition to exports
export -f isActionAlreadyCompleted +export -f verifyProposalExistsInMongo
Comment on lines
+120
to
+168
| bob = { key = "${BLOCKSCOUT_API_KEY}", url = "https://explorer.gobob.xyz/api", verifier = "blockscout", chain = "60808" } | ||
| boba = { key = "${BOBA_ETHERSCAN_API_KEY}", url = "https://api.routescan.io/v2/network/mainnet/evm/288/etherscan/api", chain = "288" } | ||
| botanix = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://api.routescan.io/v2/network/mainnet/evm/3637/etherscan", chain = "3637" } | ||
| botanix = { key = "${VERIFY_CONTRACT_API_KEY}", url = "https://api.routescan.io/v2/network/mainnet/evm/3637/etherscan", chain = "3637" } | ||
| bsc = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=56", chain = "56" } | ||
| celo = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=42220", chain = "42220" } | ||
| corn = { key = "${CORN_ETHERSCAN_API_KEY}", url = "https://api.routescan.io/v2/network/mainnet/evm/21000000/etherscan", chain = "21000000" } | ||
| cronos = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://cronos.org/explorer/api", verifier = "blockscout", chain = "25" } | ||
| cronos = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=25", chain = "25" } | ||
| flare = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://api.routescan.io/v2/network/mainnet/evm/14/etherscan", chain = "14" } | ||
| flow = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://evm.flowscan.io/api", chain = "747" , verifier = "blockscout"} | ||
| flow = { key = "${BLOCKSCOUT_API_KEY}", url = "https://evm.flowscan.io/api", chain = "747" , verifier = "blockscout"} | ||
| fraxtal = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=252", chain = "252" } | ||
| fuse = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.fuse.io/api?", chain = "122" , verifier = "blockscout"} | ||
| fuse = { key = "${BLOCKSCOUT_API_KEY}", url = "https://explorer.fuse.io/api?", chain = "122" , verifier = "blockscout"} | ||
| gnosis = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=100", chain = "100" } | ||
| gravity = { key = "${GRAVITY_ETHERSCAN_API_KEY}", url = "https://www.oklink.com/api/v5/explorer/contract/verify-source-code-plugin/GRAVITY", chain = "1625", verifier = "oklink" } | ||
| hemi = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.hemi.xyz/api", chain = "43111", verifier = "blockscout" } | ||
| hemi = { key = "${BLOCKSCOUT_API_KEY}", url = "https://explorer.hemi.xyz/api", chain = "43111", verifier = "blockscout" } | ||
| hyperevm = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=999", chain = "999" } | ||
| etherlink = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.etherlink.com/api", chain = "42793", verifier = "blockscout" } | ||
| ink = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.inkonchain.com/api", verifier = "blockscout", chain = "57073"} | ||
| jovay = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.jovay.io/api", chain = "5734951"} | ||
| immutablezkevm = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.immutable.com/api", chain = "13371", verifier = "blockscout" } | ||
| kaia = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://www.oklink.com/api/v5/explorer/contract/verify-source-code-plugin/kaia", chain = "8217", verifier = "oklink" } | ||
| katana = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=747474", chain = "747474" } | ||
| lens = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://api-explorer-verify.lens.matterhosted.dev/contract_verification", chain = "232", verifier = "zksync" } | ||
| lens = { key = "${LENS_ETHERSCAN_API_KEY}", url = "https://api-explorer-verify.lens.matterhosted.dev/contract_verification", chain = "232", verifier = "zksync" } | ||
| linea = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=59144", chain = "59144" } | ||
| lisk = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://blockscout.lisk.com/api", chain = "1135", verifier = "blockscout" } | ||
| lisk = { key = "${BLOCKSCOUT_API_KEY}", url = "https://blockscout.lisk.com/api", chain = "1135", verifier = "blockscout" } | ||
| mantle = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=5000", chain = "5000" } | ||
| megaeth = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://megaeth.blockscout.com/api", chain = "4326", verifier = "blockscout" } | ||
| metis = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://api.routescan.io/v2/network/mainnet/evm/1088/etherscan", chain = "1088" } | ||
| megaeth = { key = "${BLOCKSCOUT_API_KEY}", url = "https://megaeth.blockscout.com/api", chain = "4326", verifier = "blockscout" } | ||
| metis = { key = "${VERIFY_CONTRACT_API_KEY}", url = "https://api.routescan.io/v2/network/mainnet/evm/1088/etherscan", chain = "1088" } | ||
| mode = { key = "${MODE_ETHERSCAN_API_KEY}", url = "https://api.routescan.io/v2/network/mainnet/evm/34443/etherscan", chain = "34443" } | ||
| monad = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=143", verifier = "custom", chain = "143" } | ||
| moonbeam = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=1284", chain = "1284" } | ||
| nibiru = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://api.routescan.io/v2/network/mainnet/evm/6900/etherscan", chain = "6900" } | ||
| optimism = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=10", chain = "10" } | ||
| opbnb = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=204", chain = "204" } | ||
| plasma = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://api.routescan.io/v2/network/mainnet/evm/9745/etherscan", chain = "9745" } | ||
| plume = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer-plume-mainnet-1.t.conduit.xyz/api", verifier = "blockscout", chain = "98866" } | ||
| plasma = { key = "${VERIFY_CONTRACT_API_KEY}", url = "https://api.routescan.io/v2/network/mainnet/evm/9745/etherscan", chain = "9745" } | ||
| plume = { key = "${BLOCKSCOUT_API_KEY}", url = "https://explorer-plume-mainnet-1.t.conduit.xyz/api", verifier = "blockscout", chain = "98866" } | ||
| polygon = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=137", chain = "137" } | ||
| ronin= { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://sourcify.roninchain.com/server", chain = "2020", verifier = "sourcify" } | ||
| rootstock= { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://rootstock.blockscout.com/api", chain = "30", verifier = "blockscout" } | ||
| ronin= { key = "${SOURCIFY_API_KEY}", url = "https://sourcify.roninchain.com/server", chain = "2020", verifier = "sourcify" } | ||
| rootstock= { key = "${BLOCKSCOUT_API_KEY}", url = "https://rootstock.blockscout.com/api", chain = "30", verifier = "blockscout" } | ||
| scroll = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=534352", chain = "534352" } | ||
| sei = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=1329", chain = "1329" } | ||
| soneium = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://soneium.blockscout.com/api", verifier = "blockscout", chain = "1868" } | ||
| soneium = { key = "${BLOCKSCOUT_API_KEY}", url = "https://soneium.blockscout.com/api", verifier = "blockscout", chain = "1868" } | ||
| sonic = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=146", chain = "146" } | ||
| sophon = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer.sophon.xyz/contract_verification", chain = "50104", verifier = "zksync" } | ||
| sophon = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=50104", chain = "50104" } | ||
| stable = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=988", chain = "988"} | ||
| superposition = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://explorer-superposition-1v9rjalnat.t.conduit.xyz/api/", chain = "55244", verifier = "blockscout" } | ||
| superposition = { key = "${BLOCKSCOUT_API_KEY}", url = "https://explorer-superposition-1v9rjalnat.t.conduit.xyz/api/", chain = "55244", verifier = "blockscout" } | ||
| swellchain = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=1923", chain = "1923" } | ||
| taiko = { key = "${MAINNET_ETHERSCAN_API_KEY}", url = "https://api.etherscan.io/v2/api?chainid=167000", chain = "167000" } | ||
| unichain = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://unichain.blockscout.com/api?", chain = "130" , verifier = "blockscout" } | ||
| vana = { key = "${NO_ETHERSCAN_API_KEY_REQUIRED}", url = "https://vanascan.io/api", chain = "1480", verifier = "blockscout" } | ||
| unichain = { key = "${BLOCKSCOUT_API_KEY}", url = "https://unichain.blockscout.com/api?", chain = "130" , verifier = "blockscout" } | ||
| vana = { key = "${BLOCKSCOUT_API_KEY}", url = "https://vanascan.io/api", chain = "1480", verifier = "blockscout" } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display .env.example
echo "=== Looking for .env.example ==="
fd -t f '.env.example'
# If found, show the full contents to verify
echo ""
echo "=== Contents of .env.example ==="
fd -t f '.env.example' -exec cat {} \;Repository: lifinance/contracts
Length of output: 347
🏁 Script executed:
#!/bin/bash
# Read .env.example directly
echo "=== Contents of .env.example ==="
cat .env.example
echo ""
echo "=== Checking for new API key variables ==="
for var in BLOCKSCOUT_API_KEY VERIFY_CONTRACT_API_KEY LENS_ETHERSCAN_API_KEY SOURCIFY_API_KEY BOBA_ETHERSCAN_API_KEY GRAVITY_ETHERSCAN_API_KEY CORN_ETHERSCAN_API_KEY MODE_ETHERSCAN_API_KEY; do
if grep -q "$var" .env.example; then
echo "✅ $var found"
else
echo "❌ $var missing"
fi
doneRepository: lifinance/contracts
Length of output: 2364
Update .env.example with the new API key environment variables.
Five new environment variables are introduced but missing from .env.example:
BLOCKSCOUT_API_KEY(used by 12+ chains)VERIFY_CONTRACT_API_KEY(used by botanix, metis, plasma)LENS_ETHERSCAN_API_KEY(used by lens)SOURCIFY_API_KEY(used by ronin)CORN_ETHERSCAN_API_KEY(used by corn)
Add these to the "Mainnet Explorer API Keys" section per the coding guideline: "Update .env.example when adding new environment variables."
🤖 Prompt for AI Agents
In `@foundry.toml` around lines 120 - 168, The PR adds new environment variables
referenced in foundry.toml (see keys used by bob/unichain/flow/etc. referencing
BLOCKSCOUT_API_KEY, botanix/metis/plasma referencing VERIFY_CONTRACT_API_KEY,
lens referencing LENS_ETHERSCAN_API_KEY, ronin referencing SOURCIFY_API_KEY, and
corn referencing CORN_ETHERSCAN_API_KEY) but .env.example wasn't updated; add
these five vars to the "Mainnet Explorer API Keys" section of .env.example with
brief placeholder values and comments matching existing style so developers know
to supply real API keys.
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.
Which Jira task belongs to this PR?
https://lifi.atlassian.net/browse/LF-13168?atlOrigin=eyJpIjoiZWU1YzI1MWJjYzA2NDM3ODg1MWU3OGMyNWI3YjJiNTIiLCJwIjoiaiJ9
Why did I implement it this way?
verifyProposalExistsInMongoto fail if a proposal does not exist. I ran into an issue where, after callingcreateMultisigProposalForContract, the proposal was not created. I decided to keep this check and investigate the root cause later. it might be due to a race condition in the database.Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)