Deprecated networks file #1030
Deprecated networks file #1030
Conversation
WalkthroughThis pull request updates several scripts by replacing the previous method of retrieving network data from plain text files with a structured JSON approach. In multiple scripts, commands that used Changes
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
script/tasks/acceptOwnershipTransferPeriphery.sh (1)
48-50:⚠️ Potential issueCorrect Logging Variable in the For-Loop
Inside the loop over networks, the script echoes the selected network using the variable$NETWORKinstead of the loop variable$CURRENT_NETWORK. This can lead to incorrect or confusing log messages. Update the echo statement to use$CURRENT_NETWORKto ensure clarity.
Example diff:- echo "[info] now executing transfer ownership script in network: $NETWORK" + echo "[info] now executing transfer ownership script in network: $CURRENT_NETWORK"
🧹 Nitpick comments (16)
script/tasks/diamondSyncSigs.sh (2)
23-24: Use of JSON-Based Network Selection
Transitioning to ajq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH"call—piped intogum filter—instead of a static file read greatly improves flexibility and data structure. Please confirm that the environment variableNETWORKS_JSON_FILE_PATHis correctly set, that the JSON file is valid, and consider adding an error check if no networks are returned.
127-131: Clarify Overall Exit Code Management
Inside the retry loop, the script conditionally sets a variable namedRETURNon failure. Because this variable is used later in the function’s final exit check, its value might be overwritten across iterations. It would be more robust to initialize a status variable at the start of the function and accumulate errors from each network iteration. This would clarify the overall success state.script/tasks/diamondEMERGENCYPause.sh (1)
25-27: Consistent Network Selection from JSON
Replacing the plain-text file read with a JSON-based extraction usingjqandgum filteris a positive change. Ensure thatNETWORKS_JSON_FILE_PATHpoints to a valid JSON file and consider error handling if the JSON is empty or malformed.script/tasks/diamondUpdatePeriphery.sh (1)
21-24: Adoption of JSON-Based Network Selection
The new approach—usingjq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH"piped togum filter—effectively replaces the previous file read method for selecting networks. Just ensure thatNETWORKS_JSON_FILE_PATHis defined and that its JSON structure matches the expected format.script/tasks/updateFacetConfig.sh (1)
20-22: Improved Network Array Input Handling
UsingIFS=$'\n' read -r -d '' -a NETWORKS < <(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum choose --no-limit)to capture the selected network names into an array is an effective update. (Note that using
-d ''expects a null terminator. You might consider usingmapfilefor improved clarity and portability if the target shell supports it.)script/tasks/diamondSyncDEXs.sh (1)
22-24: Consistent JSON-Based Network Selection for DEX Syncing
Switching to retrieve network keys usingjq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH"withgum filtermaintains a consistent approach across all scripts. As with the other changes, please verify that the JSON file is correctly formatted and that there is adequate error handling when no networks are selected.script/deploy/deployFacetAndAddToDiamond.sh (1)
24-26: Refactor Local Variable Declaration for NETWORK
The command now reads the network from the JSON file usingjqand pipes it togum filter. However, ShellCheck (SC2155) warns about declaring and assigning the variable on the same line. Consider separating the declaration from assignment.
Example diff:- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 24-24: Declare and assign separately to avoid masking return values.
(SC2155)
script/utils/diamondEMERGENCYPauseGitHub.sh (1)
175-178: Adopt Structured JSON for Network Enumeration
The updated loop correctly reads network keys from the JSON file viajqand populates theNETWORKSarray. This not only improves maintainability but also standardizes network data access across the scripts. Consider adding error handling in case the JSON file is missing or thejqcommand fails, to further harden this code segment.archive/scripts/removeUnusableFunctionsForImmutable.sh (1)
20-21: Improve Declaration Style for Local NETWORK Variable
The function now usesjqto extract network keys from the JSON file. To satisfy ShellCheck recommendations and enhance readability, separate the declaration and assignment forNETWORK.
Example diff:- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 20-20: Declare and assign separately to avoid masking return values.
(SC2155)
script/tasks/acceptOwnershipTransferPeriphery.sh (1)
31-32: Separate Declaration from Assignment for NETWORK
Similar to other scripts, please split the declaration and assignment when reading the network from the JSON file to comply with best practices and address ShellCheck SC2155.
Example diff:- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 31-31: Declare and assign separately to avoid masking return values.
(SC2155)
archive/scripts/Tasks/diamondMakeImmutable.sh (1)
16-17: Enhance Local Variable Declaration for NETWORK
The updated network selection now usesjqto extract from the JSON file. To address ShellCheck (SC2155) concerns, separate the declaration and assignment ofNETWORKas shown below:- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")This adjustment improves clarity and prevents potential masking of return values.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 16-16: Declare and assign separately to avoid masking return values.
(SC2155)
script/tasks/checkExecutorAndReceiver.sh (1)
50-50: Declare and AssignNETWORKSeparately to Avoid Masking Return ValuesShellCheck suggests separating the declaration of local variables from their assignment. This can help avoid inadvertently masking return values. Consider refactoring as follows:
- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
script/tasks/diamondUpdateSgConfig.sh (1)
30-30: Scope theNETWORKVariable Locally to Prevent Global LeakageIn this changed segment, the
NETWORKvariable is assigned without an explicit local declaration. To ensure that its scope is limited to the function and to follow best practices, consider declaring it locally and separating the assignment:- NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network...") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network...")script/scriptMaster.sh (3)
121-121: Separate Declaration and Assignment forNETWORKVariableAt line 121, the
NETWORKvariable is declared and assigned in one statement. To avoid masking return values as per ShellCheck’s recommendation, consider splitting the declaration and assignment:- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 121-121: Declare and assign separately to avoid masking return values.
(SC2155)
248-248: Refactor Variable Declaration in the New Network Use CaseSimilarly, at line 248, the
NETWORKvariable is declared and assigned in a single line. Splitting these statements will help ensure that any potential return value from the command is not masked:- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 248-248: Declare and assign separately to avoid masking return values.
(SC2155)
503-503: Improve Variable Scoping forNETWORKin Diamond Log UpdateAt line 503, in the branch handling diamond log updates for a specific network, the same pattern is observed. Splitting the variable declaration from its assignment can enhance clarity and robustness:
- local NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network") + local NETWORK + NETWORK=$(jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 503-503: Declare and assign separately to avoid masking return values.
(SC2155)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
archive/scripts/Tasks/diamondMakeImmutable.sh(1 hunks)archive/scripts/removeUnusableFunctionsForImmutable.sh(1 hunks)script/config.example.sh(1 hunks)script/deploy/deployFacetAndAddToDiamond.sh(1 hunks)script/helperFunctions.sh(4 hunks)script/scriptMaster.sh(3 hunks)script/tasks/acceptOwnershipTransferPeriphery.sh(1 hunks)script/tasks/checkExecutorAndReceiver.sh(1 hunks)script/tasks/diamondEMERGENCYPause.sh(1 hunks)script/tasks/diamondSyncDEXs.sh(1 hunks)script/tasks/diamondSyncSigs.sh(1 hunks)script/tasks/diamondUpdatePeriphery.sh(1 hunks)script/tasks/diamondUpdateSgConfig.sh(1 hunks)script/tasks/updateFacetConfig.sh(1 hunks)script/utils/diamondEMERGENCYPauseGitHub.sh(1 hunks)script/utils/network.ts(0 hunks)
💤 Files with no reviewable changes (1)
- script/utils/network.ts
🧰 Additional context used
🧠 Learnings (2)
script/tasks/diamondEMERGENCYPause.sh (1)
Learnt from: 0xDEnYO
PR: lifinance/contracts#715
File: script/utils/diamondEMERGENCYPauseGitHub.sh:127-127
Timestamp: 2024-11-12T09:43:10.543Z
Learning: In `script/utils/diamondEMERGENCYPauseGitHub.sh`, temporary hardcoded network lists may be included during testing in pull requests, with the plan to remove them before merging into the main branch.
script/utils/diamondEMERGENCYPauseGitHub.sh (1)
Learnt from: 0xDEnYO
PR: lifinance/contracts#715
File: script/utils/diamondEMERGENCYPauseGitHub.sh:127-127
Timestamp: 2024-11-12T09:43:10.543Z
Learning: In `script/utils/diamondEMERGENCYPauseGitHub.sh`, temporary hardcoded network lists may be included during testing in pull requests, with the plan to remove them before merging into the main branch.
🪛 Shellcheck (0.10.0)
archive/scripts/Tasks/diamondMakeImmutable.sh
[warning] 16-16: Declare and assign separately to avoid masking return values.
(SC2155)
archive/scripts/removeUnusableFunctionsForImmutable.sh
[warning] 20-20: Declare and assign separately to avoid masking return values.
(SC2155)
script/deploy/deployFacetAndAddToDiamond.sh
[warning] 24-24: Declare and assign separately to avoid masking return values.
(SC2155)
script/config.example.sh
[warning] 68-68: NETWORKS_JSON_FILE_PATH appears unused. Verify use (or export if used externally).
(SC2034)
script/tasks/checkExecutorAndReceiver.sh
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
script/tasks/acceptOwnershipTransferPeriphery.sh
[warning] 31-31: Declare and assign separately to avoid masking return values.
(SC2155)
script/scriptMaster.sh
[warning] 121-121: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 248-248: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 503-503: Declare and assign separately to avoid masking return values.
(SC2155)
script/helperFunctions.sh
[warning] 2020-2020: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 2152-2152: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 2857-2857: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: run-unit-tests
- GitHub Check: generate-tag
🔇 Additional comments (7)
script/tasks/diamondEMERGENCYPause.sh (1)
162-167: Caution: Early Exit on Already Paused Diamond
Within thehandleNetworkfunction, the script callsexit 0when detecting that the diamond is already paused. This abrupt exit may bypass additional network processing if multiple networks were expected. Please verify that this behavior is intentional; if parallel processing is desired across networks, consider collecting and reporting statuses before exiting.script/tasks/diamondUpdatePeriphery.sh (1)
77-81: Verify Deployment Address File Path Formation
The concatenation that creates deployment files (e.g.ADDRS="deployments/$NETWORK.$FILE_SUFFIX""json" ```) may result in filenames such as `network.productionjson` if `FILE_SUFFIX` does not include a terminating dot. Verify that the `getFileSuffix` function returns the correct value or explicitly add a period before `json` if needed. </details> <details> <summary>script/config.example.sh (1)</summary> `67-69`: **Ensure Consistency and External Usage of the New JSON Network Variable** The variable `NETWORKS_JSON_FILE_PATH` now clearly points to the JSON file containing network details. Please confirm that all dependent scripts reference this variable correctly. If this variable is meant to be used externally (for example, by sourced scripts), consider exporting it. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Shellcheck (0.10.0)</summary> [warning] 68-68: NETWORKS_JSON_FILE_PATH appears unused. Verify use (or export if used externally). (SC2034) </details> </details> </details> <details> <summary>script/helperFunctions.sh (4)</summary> `1969-1977`: **Update to use structured JSON for network retrieval in `getAllNetworksArray`.** The function now sets ```bash local FILE="$NETWORKS_JSON_FILE_PATH"ensuring that network names are read from the intended JSON file rather than from a plain text file. This aligns with the PR objective of standardizing the network configuration format.
2016-2028: Consistent JSON-based filtering ingetIncludedNetworksArray.
The changes modify the variable assignment so that the function reads fromlocal FILE="$NETWORKS_JSON_FILE_PATH"and later uses the output of
jq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH"to populate the array. This ensures that the inclusion/exclusion logic properly uses the new JSON configuration.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2020-2020: Declare and assign separately to avoid masking return values.
(SC2155)
2152-2159: Modernized user network selection ingetUserSelectedNetwork.
Instead of reading from a plain text file, the function now retrieves the network names fromjq -r 'keys[]' "$NETWORKS_JSON_FILE_PATH" | gum filter --placeholder "Network..."This not only improves consistency with the rest of the scripts but also provides a better user experience by leveraging
gum filterfor interactive selection. Ensure thatNETWORKS_JSON_FILE_PATHis correctly set in your environment.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2152-2152: Declare and assign separately to avoid masking return values.
(SC2155)
2852-2862: Robust chain ID retrieval ingetChainIdusing the JSON file.
The function now checks for the existence of the JSON file viaif [[ ! -f "$NETWORKS_JSON_FILE_PATH" ]]; then echo "Error: JSON file '$NETWORKS_JSON_FILE_PATH' not found." >&2 return 1 fiand then retrieves the chain ID with:
local CHAIN_ID=$(jq -r --arg network "$NETWORK" '.[$network].chainId // empty' "$NETWORKS_JSON_FILE_PATH")This update solidifies the move to a structured JSON approach and helps prevent misconfiguration.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2857-2857: Declare and assign separately to avoid masking return values.
(SC2155)
Which Jira task belongs to this PR?
LF-10381
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)