Pioneer Throughput Testing Script #1353
Conversation
WalkthroughAdds a Forge script contract Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 10
🧹 Nitpick comments (7)
script/pioneer/PioneerQA.sol (2)
31-47: Minor loop/style nitsInitialize i explicitly and consider unchecked increment to save a bit of gas (non-critical in scripts).
- for (uint256 i; i < numParams; ++i) { + for (uint256 i = 0; i < numParams; ) { Params calldata param = params[i]; ... - } + unchecked { ++i; } + }
21-27: Validate inputs early (diamond/refundAddress/params)Add basic requires to fail fast on obvious misconfigurations.
function run( address diamond, address payable refundAddress, Params[] calldata params ) public { - if (!LibAsset.isContract(diamond)) revert DiamondNotContract(); + if (!LibAsset.isContract(diamond)) revert DiamondNotContract(); + require(refundAddress != address(0), "refundAddress=0"); + require(params.length > 0, "empty params");script/pioneer/pioneer-qa.sh (5)
14-17: Quote expansions; normalize onceSafer lower/upper casing with quotes.
-# Convert network to lowercase -NETWORK=$(echo $NETWORK | tr '[:upper:]' '[:lower:]') -UPPER_NETWORK=$(echo $NETWORK | tr '[:lower:]' '[:upper:]') +NETWORK=$(echo "$NETWORK" | tr '[:upper:]' '[:lower:]') +UPPER_NETWORK=$(echo "$NETWORK" | tr '[:lower:]' '[:upper:]')
79-86: Prefer data-urlencode; make endpoint configurableSafer query construction; allow overriding endpoint via env.
-# Set the Pioneer endpoint (replace with actual endpoint if needed) -PIONEER_ENDPOINT="https://solver-dev.li.fi" +PIONEER_ENDPOINT="${PIONEER_ENDPOINT:-https://solver-dev.li.fi}" ... -RESPONSE=$(curl -s -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Content-Type: application/json") +RESPONSE=$(curl -sS --fail --get \ + --data-urlencode "fromChain=$FROM_CHAIN" \ + --data-urlencode "toChain=$TO_CHAIN" \ + --data-urlencode "fromToken=$FROM_TOKEN" \ + --data-urlencode "toToken=$TO_TOKEN" \ + --data-urlencode "toAddress=$TO_ADDRESS" \ + --data-urlencode "fromAmount=$FROM_AMOUNT" \ + --data-urlencode "slippage=$SLIPPAGE" \ + --data-urlencode "externalId=$EXTERNAL_ID" \ + "$PIONEER_ENDPOINT/quote")
93-97: Add retries and fail-fast on HTTP errorsCurrent check exits on first error without retry/backoff.
-if [ -z "$RESPONSE" ] || echo "$RESPONSE" | grep -q '"error"'; then - echo "Quote request failed: $RESPONSE" - exit 1 -fi +if [ -z "$RESPONSE" ] || echo "$RESPONSE" | jq -e '.error?' > /dev/null; then + echo "Quote request failed: ${RESPONSE:-<empty>}" + exit 1 +fiOptionally wrap curl in a small retry loop with backoff.
112-116: More robust tuple formattingPrevent trailing comma/space issues; avoid extra whitespace.
-FLATTENED_RESPONSES=$(printf "(%s)," "${RESPONSES[@]}") -FLATTENED_RESPONSES="[${FLATTENED_RESPONSES%,}]" +printf -v FLATTENED_RESPONSES '(%s),' "${RESPONSES[@]}" +FLATTENED_RESPONSES="[${FLATTENED_RESPONSES%,}]"
118-118: Quote sensitive args in forge callQuote PRIVATE_KEY/RPC_URL; keep verbosity configurable.
-forge script PioneerQA --sig "run(address,address,(bytes32,address,address,uint256,uint256,uint256)[])" "$DIAMOND" "$USER_ADDRESS" "$FLATTENED_RESPONSES" --private-key $PRIVATE_KEY --rpc-url $RPC_URL --broadcast -vvvv +forge script PioneerQA --sig "run(address,address,(bytes32,address,address,uint256,uint256,uint256)[])" \ + "$DIAMOND" "$USER_ADDRESS" "$FLATTENED_RESPONSES" \ + --private-key "$PRIVATE_KEY" --rpc-url "$RPC_URL" --broadcast -vvvv
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
script/pioneer/PioneerQA.sol(1 hunks)script/pioneer/pioneer-qa-full.sh(1 hunks)script/pioneer/pioneer-qa.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (conventions.md)
**/*.sh: Bash scripts must start with #!/bin/bash and be organized into modular functions (Logging; Error handling and logging; Deployment functions), applying DRY via helpers
Load environment from .env or config.sh, declare globals in config, update .env.example, and validate env vars early
Use helper logging functions (echoDebug, error, warning, success), validate inputs early, check exit status with checkFailure, and use set -e where appropriate
Provide automated retry for RPC issues, wrap Foundry deployment, validate requirements/dependencies, and implement robust error recovery
Provide clear prompts (e.g., gum choose), feedback, progress indicators, usage instructions, examples, and maintain helpful help text/errors
Keep scripts modular, consistent in naming, compatible with tooling, and tested; update documentation with changes
Files:
script/pioneer/pioneer-qa-full.shscript/pioneer/pioneer-qa.sh
🧠 Learnings (1)
📓 Common learnings
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.
🪛 Shellcheck (0.10.0)
script/pioneer/pioneer-qa-full.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
script/pioneer/pioneer-qa.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 32-32: Quote this to prevent word splitting.
(SC2046)
[warning] 32-32: Quote this to prevent word splitting.
(SC2046)
[warning] 33-33: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
🔇 Additional comments (2)
script/pioneer/PioneerQA.sol (1)
33-44: BridgeData initializer matches struct; noamountfield exists.script/pioneer/pioneer-qa.sh (1)
37-38: Fix cast wallet syntaxcast expects the --private-key flag.
-USER_ADDRESS=$(cast wallet address $PRIVATE_KEY) +USER_ADDRESS=$(cast wallet address --private-key "$PRIVATE_KEY")⛔ Skipped due to learnings
Learnt from: maxklenk PR: lifinance/contracts#782 File: script/demoScripts/demoPermit2.ts:100-107 Timestamp: 2024-11-05T17:15:33.833Z Learning: In demo scripts using testing wallets, passing private keys via command-line arguments is acceptable.
| # For each network provided, run the qa script in parallel and then wait for all to finish. | ||
| for RUN in $(seq 1 $NUM_RUNS); | ||
| do | ||
| echo "Starting run $RUN of $NUM_RUNS" | ||
| for NETWORK in $(echo $NETWORK | tr -d '[]"' | tr ',' '\n'); | ||
| do | ||
| echo "Starting QA for network: $NETWORK" | ||
| bash script/pioneer/pioneer-qa.sh "$NETWORK" "$TO_NETWORKS" "$TOKEN" "$NUM_TRANSACTIONS" & | ||
| done | ||
| # Wait for all background processes to finish | ||
| wait | ||
| done; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid variable shadowing; quote expansions; capture failures from background jobs
Reuse of NETWORK as loop var shadows input; also no failure aggregation.
-# For each network provided, run the qa script in parallel and then wait for all to finish.
-for RUN in $(seq 1 $NUM_RUNS);
+# For each run, spawn per-network jobs and wait.
+for RUN in $(seq 1 "$NUM_RUNS");
do
echo "Starting run $RUN of $NUM_RUNS"
- for NETWORK in $(echo $NETWORK | tr -d '[]"' | tr ',' '\n');
+ pids=()
+ for NET in $(echo "$SRC_NETWORKS" | tr -d '[]"' | tr ',' '\n');
do
- echo "Starting QA for network: $NETWORK"
- bash script/pioneer/pioneer-qa.sh "$NETWORK" "$TO_NETWORKS" "$TOKEN" "$NUM_TRANSACTIONS" &
+ echo "Starting QA for network: $NET"
+ bash script/pioneer/pioneer-qa.sh "$NET" "$TO_NETWORKS" "$TOKEN" "$NUM_TRANSACTIONS" &
+ pids+=($!)
done
- # Wait for all background processes to finish
- wait
+ # Wait for all background processes to finish and propagate failures
+ status=0
+ for pid in "${pids[@]}"; do
+ if ! wait "$pid"; then status=1; fi
+ done
+ if [ "$status" -ne 0 ]; then
+ echo "One or more QA jobs failed in run $RUN"
+ exit 1
+ fi
done;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # For each network provided, run the qa script in parallel and then wait for all to finish. | |
| for RUN in $(seq 1 $NUM_RUNS); | |
| do | |
| echo "Starting run $RUN of $NUM_RUNS" | |
| for NETWORK in $(echo $NETWORK | tr -d '[]"' | tr ',' '\n'); | |
| do | |
| echo "Starting QA for network: $NETWORK" | |
| bash script/pioneer/pioneer-qa.sh "$NETWORK" "$TO_NETWORKS" "$TOKEN" "$NUM_TRANSACTIONS" & | |
| done | |
| # Wait for all background processes to finish | |
| wait | |
| done; | |
| # For each run, spawn per-network jobs and wait. | |
| for RUN in $(seq 1 "$NUM_RUNS"); | |
| do | |
| echo "Starting run $RUN of $NUM_RUNS" | |
| pids=() | |
| for NET in $(echo "$SRC_NETWORKS" | tr -d '[]"' | tr ',' '\n'); | |
| do | |
| echo "Starting QA for network: $NET" | |
| bash script/pioneer/pioneer-qa.sh "$NET" "$TO_NETWORKS" "$TOKEN" "$NUM_TRANSACTIONS" & | |
| pids+=($!) | |
| done | |
| # Wait for all background processes to finish and propagate failures | |
| status=0 | |
| for pid in "${pids[@]}"; do | |
| if ! wait "$pid"; then | |
| status=1 | |
| fi | |
| done | |
| if [ "$status" -ne 0 ]; then | |
| echo "One or more QA jobs failed in run $RUN" | |
| exit 1 | |
| fi | |
| done; |
| LibAsset.approveERC20( | ||
| IERC20(param.sendingAssetId), | ||
| diamond, | ||
| param.inputAmount, | ||
| param.inputAmount | ||
| ); |
There was a problem hiding this comment.
approveERC20 call has wrong arity; likely compile-time error
LibAsset.approveERC20 typically takes 3 args (token, spender, amount). You’re passing 4.
Apply:
- LibAsset.approveERC20(
- IERC20(param.sendingAssetId),
- diamond,
- param.inputAmount,
- param.inputAmount
- );
+ LibAsset.approveERC20(
+ IERC20(param.sendingAssetId),
+ diamond,
+ param.inputAmount
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LibAsset.approveERC20( | |
| IERC20(param.sendingAssetId), | |
| diamond, | |
| param.inputAmount, | |
| param.inputAmount | |
| ); | |
| LibAsset.approveERC20( | |
| IERC20(param.sendingAssetId), | |
| diamond, | |
| param.inputAmount | |
| ); |
🤖 Prompt for AI Agents
In script/pioneer/PioneerQA.sol around lines 54 to 59, the LibAsset.approveERC20
call uses four arguments which mismatches the library's 3-argument signature
(token, spender, amount); remove the duplicate amount so the call is
LibAsset.approveERC20(IERC20(param.sendingAssetId), diamond, param.inputAmount)
to match the expected arity and compile correctly.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
script/pioneer/pioneer-qa.sh (6)
1-3: Add Bash shebang and strict mode per repo guidelinesEnsure predictable behavior and satisfy ShellCheck SC2148.
+#!/bin/bash +set -Eeuo pipefail +IFS=$'\n\t'
18-21: Quote deployment path; check file exists and non-empty LiFiDiamondAvoids jq failures and empty addresses.
-DIAMOND=$(jq -r '.LiFiDiamond' ./deployments/$NETWORK.staging.json) -echo "Using LiFi Diamond at $DIAMOND" +DEPLOY_FILE="./deployments/${NETWORK}.staging.json" +[ -f "$DEPLOY_FILE" ] || { echo "Missing $DEPLOY_FILE" >&2; exit 1; } +DIAMOND=$(jq -er '.LiFiDiamond' "$DEPLOY_FILE") +[ -n "$DIAMOND" ] || { echo "Empty .LiFiDiamond in $DEPLOY_FILE" >&2; exit 1; } +echo "Using LiFi Diamond at $DIAMOND"
22-27: Validate RPC env var and quote usagesPrevents empty RPC and word-splitting.
-RPC_URL=$(eval echo \$ETH_NODE_URI_$UPPER_NETWORK) +RPC_ENV="ETH_NODE_URI_${UPPER_NETWORK}" +RPC_URL=$(eval "echo \${$RPC_ENV:-}") +[ -n "$RPC_URL" ] || { echo "Missing $RPC_ENV" >&2; exit 1; } -# Get from network id. -FROM_NETWORK_ID=$(cast chain-id --rpc-url $RPC_URL) +# Get from network id. +FROM_NETWORK_ID=$(cast chain-id --rpc-url "$RPC_URL")
46-57: Guard against zero operating amountAvoids 0-value txs.
echo "Operating Amount: $OP_AMOUNT" +[ "$OP_AMOUNT" -gt 0 ] || { echo "OP_AMOUNT computed as 0" >&2; exit 1; }
103-103: Bug: using OP_AMOUNT instead of per-tx FROM_AMOUNT; remove spacesEach Params.inputAmount must match the per-transaction amount and avoid space-splitting.
- RESPONSES+=("$(echo "$TRANSACTION_ID, $TOKEN, $USER_ADDRESS, $OP_AMOUNT, $TO_AMOUNT_MIN, $TO_CHAIN_ID")") + RESPONSES+=("${TRANSACTION_ID},${TOKEN},${USER_ADDRESS},${FROM_AMOUNT},${TO_AMOUNT_MIN},${TO_CHAIN_ID}")
28-35: Harden TO_NETWORKS parsing; trim spaces; validate per-network RPCAvoids SC2046/SC2206, supports ["a","b"] input, and respects env-only RPC policy.
-TO_NETWORKS_IDS=() -for NET in $(echo $TO_NETWORKS | tr -d '[]"' | tr ',' '\n'); -do - NET_ID=$(cast chain-id --rpc-url $(eval echo \$ETH_NODE_URI_$(echo $NET | tr '[:lower:]' '[:upper:]'))) - TO_NETWORKS_IDS+=($NET_ID) -done +TO_NETWORKS_IDS=() +SANITIZED=$(printf "%s" "$TO_NETWORKS" | tr -d '[]" ') +for NET in ${SANITIZED//,/ }; do + UPPER_NET=${NET^^} + NET_RPC_ENV="ETH_NODE_URI_${UPPER_NET}" + NET_RPC=$(eval "echo \${$NET_RPC_ENV:-}") + [ -n "$NET_RPC" ] || { echo "Missing $NET_RPC_ENV" >&2; exit 1; } + NET_ID=$(cast chain-id --rpc-url "$NET_RPC") + TO_NETWORKS_IDS+=("$NET_ID") +done
🧹 Nitpick comments (3)
script/pioneer/pioneer-qa.sh (3)
15-17: Prefer Bash parameter expansion for case conversion; quote varsSimpler and avoids subshells.
-NETWORK=$(echo $NETWORK | tr '[:upper:]' '[:lower:]') -UPPER_NETWORK=$(echo $NETWORK | tr '[:lower:]' '[:upper:]') +NETWORK=${NETWORK,,} +UPPER_NETWORK=${NETWORK^^}
88-89: Make Pioneer endpoint configurableAllows switching envs without editing the script.
- PIONEER_ENDPOINT="https://solver-dev.li.fi" + PIONEER_ENDPOINT="${PIONEER_ENDPOINT:-https://solver-dev.li.fi}"
1-114: Modularize with helpers and preflight checks (logging, deps, retries)Align with repo shell guidelines: add log helpers (echoDebug/error/success), a checkFailure trap, and preflight dependency checks (jq, curl, cast, forge, openssl); wrap network calls with a reusable retry helper. I can provide a small logging/retry module if you want it in this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
script/pioneer/pioneer-qa.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (conventions.md)
**/*.sh: Bash scripts must start with #!/bin/bash and be organized into modular functions (Logging; Error handling and logging; Deployment functions), applying DRY via helpers
Load environment from .env or config.sh, declare globals in config, update .env.example, and validate env vars early
Use helper logging functions (echoDebug, error, warning, success), validate inputs early, check exit status with checkFailure, and use set -e where appropriate
Provide automated retry for RPC issues, wrap Foundry deployment, validate requirements/dependencies, and implement robust error recovery
Provide clear prompts (e.g., gum choose), feedback, progress indicators, usage instructions, examples, and maintain helpful help text/errors
Keep scripts modular, consistent in naming, compatible with tooling, and tested; update documentation with changes
Files:
script/pioneer/pioneer-qa.sh
🧠 Learnings (13)
📓 Common learnings
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.
📚 Learning: 2025-08-29T05:16:40.025Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.025Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-29T09:56:59.784Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.784Z
Learning: Applies to **/*.sh : Bash scripts must start with #!/bin/bash and be organized into modular functions (Logging; Error handling and logging; Deployment functions), applying DRY via helpers
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-29T09:56:59.784Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.784Z
Learning: Applies to **/*.sh : Keep scripts modular, consistent in naming, compatible with tooling, and tested; update documentation with changes
Applied to files:
script/pioneer/pioneer-qa.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/pioneer/pioneer-qa.sh
📚 Learning: 2024-11-26T01:03:43.597Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/scroll.diamond.json:111-111
Timestamp: 2024-11-26T01:03:43.597Z
Learning: When reviewing pull requests, only point out missing contract addresses in `deployments/<network>.diamond.json` if the contract's address is present in `deployments/<network>.json` but missing in `deployments/<network>.diamond.json`. If the contract is not deployed on a network (i.e., not present in `deployments/<network>.json`), then no action is needed.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2024-09-23T01:42:03.075Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#807
File: script/deploy/facets/DeployGasZipFacet.s.sol:22-35
Timestamp: 2024-09-23T01:42:03.075Z
Learning: In deployment scripts like `DeployGasZipFacet.s.sol`, do not report issues suggesting adding error handling for missing configuration files or unexpected JSON structures, as the script will fail if the file is missing.
Applied to files:
script/pioneer/pioneer-qa.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:
script/pioneer/pioneer-qa.sh
📚 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:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-29T09:56:59.784Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.784Z
Learning: Applies to script/deploy/resources/deployRequirements.json : Maintain deployRequirements.json to define dependencies, zero-address restrictions, required external configs; consumed by helperFunctions.sh for validation
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-19T05:10:12.922Z
Learnt from: ezynda3
PR: lifinance/contracts#1303
File: script/utils/network.ts:27-32
Timestamp: 2025-08-19T05:10:12.922Z
Learning: In the lifinance/contracts repository, the team decided to standardize RPC environment variable naming by converting all hyphens to underscores in network names. For example, "tron-shasta" becomes "ETH_NODE_URI_TRON_SHASTA". The getRPCEnvVarName() helper in script/utils/network.ts implements this standard and should be used consistently across the codebase instead of direct string manipulation.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-01-24T14:53:20.703Z
Learnt from: mirooon
PR: lifinance/contracts#950
File: script/demoScripts/utils/demoScriptChainConfig.ts:17-20
Timestamp: 2025-01-24T14:53:20.703Z
Learning: In the LiFi contracts repository, RPC URLs should be strictly sourced from environment variables without fallback to networks.json configuration.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-04-17T02:40:15.827Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1102
File: script/tasks/diamondSyncSigs_FAST.sh:112-117
Timestamp: 2025-04-17T02:40:15.827Z
Learning: Do not suggest using `mapfile` or `read -a` for array assignments in the lifinance/contracts repository as these features may not be available in the user's environment. The scripts are designed with the knowledge that network names will only ever consist of one word, making word splitting concerns less relevant.
Applied to files:
script/pioneer/pioneer-qa.sh
🪛 Shellcheck (0.10.0)
script/pioneer/pioneer-qa.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 32-32: Quote this to prevent word splitting.
(SC2046)
[warning] 32-32: Quote this to prevent word splitting.
(SC2046)
[warning] 33-33: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
| # Get from network. | ||
| NETWORK=$1 # "optimism" | ||
| # Get to networks | ||
| TO_NETWORKS=$2 # ["optimism", "arbitrum", "polygon"] | ||
| # Get the token | ||
| TOKEN=$3 # 0x... | ||
| NUM_TRANSACTIONS=$4 # 10 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add usage/help and validate required args early
Prevents divide-by-zero and empty inputs; improves UX.
+# Usage / args
+usage() {
+ echo "Usage: $(basename "$0") <NETWORK> <TO_NETWORKS_JSON> <TOKEN_ADDRESS> <NUM_TRANSACTIONS>" >&2
+ echo "Example: $(basename "$0") optimism '[\"arbitrum\",\"polygon\"]' 0x0000000000000000000000000000000000000000 100" >&2
+ exit 1
+}
+[ $# -ge 4 ] || usage
+[[ "$4" =~ ^[0-9]+$ ]] || { echo "NUM_TRANSACTIONS must be an integer" >&2; exit 1; }
+[[ "$4" -gt 0 ]] || { echo "NUM_TRANSACTIONS must be > 0" >&2; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Get from network. | |
| NETWORK=$1 # "optimism" | |
| # Get to networks | |
| TO_NETWORKS=$2 # ["optimism", "arbitrum", "polygon"] | |
| # Get the token | |
| TOKEN=$3 # 0x... | |
| NUM_TRANSACTIONS=$4 # 10 | |
| # Usage / args | |
| usage() { | |
| echo "Usage: $(basename "$0") <NETWORK> <TO_NETWORKS_JSON> <TOKEN_ADDRESS> <NUM_TRANSACTIONS>" >&2 | |
| echo "Example: $(basename "$0") optimism '[\"arbitrum\",\"polygon\"]' 0x0000000000000000000000000000000000000000 100" >&2 | |
| exit 1 | |
| } | |
| [ $# -ge 4 ] || usage | |
| [[ "$4" =~ ^[0-9]+$ ]] || { echo "NUM_TRANSACTIONS must be an integer" >&2; exit 1; } | |
| [[ "$4" -gt 0 ]] || { echo "NUM_TRANSACTIONS must be > 0" >&2; exit 1; } | |
| # Get from network. | |
| NETWORK=$1 # "optimism" | |
| # Get to networks | |
| TO_NETWORKS=$2 # ["optimism", "arbitrum", "polygon"] | |
| # Get the token | |
| TOKEN=$3 # 0x... | |
| NUM_TRANSACTIONS=$4 # 10 |
🤖 Prompt for AI Agents
In script/pioneer/pioneer-qa.sh around lines 6 to 13, the script currently
assigns positional args without validation which can lead to divide-by-zero and
empty-input errors; add an early argument check that verifies required args are
present, NUM_TRANSACTIONS is a positive integer (>0) and TO_NETWORKS is
non-empty (and if expected as a list, validate basic format), and print a
concise usage/help message (showing required arguments and example) then exit
with a non-zero status when validation fails; implement these checks immediately
after the existing assignments so downstream code never runs with invalid or
missing inputs.
| # Fetch quote from Pioneer | ||
| RESPONSE=$(curl -s -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Content-Type: application/json") | ||
|
|
||
| # # Check if the response is valid | ||
| if [ -z "$RESPONSE" ] || echo "$RESPONSE" | grep -q '"error"'; then | ||
| echo "Quote request failed: $RESPONSE" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add retries and robust response validation
Use curl retries and jq validation instead of grep.
- RESPONSE=$(curl -s -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Content-Type: application/json")
-
- # # Check if the response is valid
- if [ -z "$RESPONSE" ] || echo "$RESPONSE" | grep -q '"error"'; then
- echo "Quote request failed: $RESPONSE"
- exit 1
- fi
+ RESPONSE=$(curl -sS --retry 3 --retry-delay 2 --max-time 20 -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Accept: application/json")
+ if ! echo "$RESPONSE" | jq -e 'has("error")|not and (.toChainId and .toAmountMin)' >/dev/null; then
+ echo "Quote request failed/invalid: $RESPONSE" >&2
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Fetch quote from Pioneer | |
| RESPONSE=$(curl -s -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Content-Type: application/json") | |
| # # Check if the response is valid | |
| if [ -z "$RESPONSE" ] || echo "$RESPONSE" | grep -q '"error"'; then | |
| echo "Quote request failed: $RESPONSE" | |
| exit 1 | |
| fi | |
| # Fetch quote from Pioneer | |
| RESPONSE=$(curl -sS --retry 3 --retry-delay 2 --max-time 20 \ | |
| -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" \ | |
| -H "Accept: application/json") | |
| if ! echo "$RESPONSE" | jq -e 'has("error") | not and (.toChainId and .toAmountMin)' >/dev/null; then | |
| echo "Quote request failed/invalid: $RESPONSE" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In script/pioneer/pioneer-qa.sh around lines 90-97, the current curl call and
grep-based validation are fragile; replace with a curl invocation that enables
retries and failure on HTTP errors (e.g. --retry, --retry-delay,
--fail/--show-error, and a reasonable --max-time), capture both HTTP status and
response body separately, then validate the body with jq to ensure it is valid
JSON and does not contain an "error" field (or is empty). If jq fails to parse
or finds .error (or the HTTP status is non-2xx), print the response and exit
non-zero. Ensure you handle transient curl errors via retries and only proceed
when jq confirms a valid, non-error JSON quote.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
script/pioneer/PioneerQA.sol (1)
53-58: Fix: LibAsset.approveERC20 arity is wrong (4 args passed, expects 3)This will not compile. Remove the duplicate amount argument.
LibAsset.approveERC20( IERC20(param.sendingAssetId), diamond, - param.minAmount, - param.minAmount + param.minAmount );script/pioneer/pioneer-qa.sh (5)
1-4: Enable strict mode earlyAdd bash safety flags before sourcing env.
#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t' ## Load env source .env
14-15: Add usage/help and validate argsPrevents empty inputs and divide-by-zero.
NUM_TRANSACTIONS=$4 # 10 +# Usage / args +usage() { + echo "Usage: $(basename "$0") <NETWORK> <TO_NETWORKS_JSON> <TOKEN_ADDRESS> <NUM_TRANSACTIONS>" >&2 + echo "Example: $(basename "$0") optimism '[\"arbitrum\",\"polygon\"]' 0x0000000000000000000000000000000000000000 100" >&2 + exit 1 +} +[ $# -ge 4 ] || usage +[[ "$NUM_TRANSACTIONS" =~ ^[0-9]+$ ]] || { echo "NUM_TRANSACTIONS must be an integer" >&2; exit 1; } +[[ "$NUM_TRANSACTIONS" -gt 0 ]] || { echo "NUM_TRANSACTIONS must be > 0" >&2; exit 1; }
46-49: Fix cast usage; require PRIVATE_KEYPass key via flag and quote.
[[ -n "${PRIVATE_KEY:-}" ]] || { echo "Missing PRIVATE_KEY in env" >&2; exit 1; } # Compute the user's address -USER_ADDRESS=$(cast wallet address $PRIVATE_KEY) +USER_ADDRESS=$(cast wallet address --private-key "$PRIVATE_KEY")
100-107: Harden quote request: retries + JSON validationReduces flakiness; check structured fields instead of grepping.
# Set the Pioneer endpoint PIONEER_ENDPOINT="https://solver-dev.li.fi" # Fetch quote from Pioneer - RESPONSE=$(curl -s -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Content-Type: application/json") + RESPONSE=$(curl -sS --retry 3 --retry-delay 2 --max-time 20 -G "$PIONEER_ENDPOINT/quote?$QUERY_STRING" -H "Accept: application/json") echo "$PIONEER_ENDPOINT/quote?$QUERY_STRING" echo "Response: $RESPONSE" # # Check if the response is valid - if [ -z "$RESPONSE" ] || echo "$RESPONSE" | grep -q '"error"'; then - echo "Quote request failed: $RESPONSE" - exit 1 - fi + if ! echo "$RESPONSE" | jq -e 'has("error")|not and (.toChainId and .toAmountMin)' >/dev/null; then + echo "Quote request failed/invalid: $RESPONSE" >&2 + exit 1 + fiAlso applies to: 109-113
118-118: Drop spaces in tuple; keep fields tightAvoids downstream parsing issues.
- RESPONSES+=("$(echo "$TRANSACTION_ID, $TOKEN, $USER_ADDRESS, $FROM_AMOUNT, $TO_CHAIN_ID")") + RESPONSES+=("$(echo "$TRANSACTION_ID,$TOKEN,$USER_ADDRESS,$FROM_AMOUNT,$TO_CHAIN_ID")")
🧹 Nitpick comments (6)
script/pioneer/PioneerQA.sol (2)
32-36: Use a meaningful integrator tag“ACME Devs” looks placeholder. Helps analytics/attribution to set a stable identifier (e.g., PioneerQA).
- integrator: "ACME Devs", + integrator: "PioneerQA",
52-58: Optional: favor max approval to cut txs under loadGiven throughput goals, consider a one-time max approval to avoid repeat approvals per tx (team already accepts this pattern).
- LibAsset.approveERC20( + LibAsset.maxApproveERC20( IERC20(param.sendingAssetId), diamond, - param.minAmount + type(uint256).max );script/pioneer/pioneer-qa.sh (4)
16-19: Quote expansions when normalizing NETWORKMinor safety/readability.
-NETWORK=$(echo $NETWORK | tr '[:upper:]' '[:lower:]') -UPPER_NETWORK=$(echo $NETWORK | tr '[:lower:]' '[:upper:]') +NETWORK=$(echo "$NETWORK" | tr '[:upper:]' '[:lower:]') +UPPER_NETWORK=$(echo "$NETWORK" | tr '[:lower:]' '[:upper:]')
35-44: Safer TO_NETWORKS parsing without mapfile/read -aAvoid subshell array loss and keep to repository constraints.
-TO_NETWORKS_IDS=() -for NET in $(echo $TO_NETWORKS | tr -d '[]"' | tr ',' '\n'); -do +TO_NETWORKS_IDS=() +TO_NETS_CLEAN=$(printf "%s" "$TO_NETWORKS" | tr -d '[]"' | tr ',' '\n') +while read -r NET; do UPPER_NET=$(echo "$NET" | tr '[:lower:]' '[:upper:]') NET_RPC_ENV="ETH_NODE_URI_${UPPER_NET}" NET_RPC=$(eval "echo \${$NET_RPC_ENV:-}") [ -n "$NET_RPC" ] || { echo "Missing $NET_RPC_ENV"; exit 1; } NET_ID=$(cast chain-id --rpc-url "$NET_RPC") TO_NETWORKS_IDS+=("$NET_ID") -done +done <<< "$TO_NETS_CLEAN" +[ "${#TO_NETWORKS_IDS[@]}" -gt 0 ] || { echo "TO_NETWORKS resolved to empty set"; exit 1; }
58-61: Quote balance queryAvoid word-splitting.
-USER_BALANCE=$(cast balance $USER_ADDRESS --rpc-url $RPC_URL) +USER_BALANCE=$(cast balance "$USER_ADDRESS" --rpc-url "$RPC_URL")
124-129: Consider batching to avoid ARG_MAX at 1k–10k tx scaleLarge arrays can exceed command-line limits. Chunk RESPONSES (e.g., groups of 200–500) and invoke forge per chunk, or write the array to a temp file and read it in the script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
script/pioneer/PioneerQA.sol(1 hunks)script/pioneer/pioneer-qa-full.sh(1 hunks)script/pioneer/pioneer-qa.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- script/pioneer/pioneer-qa-full.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (conventions.md)
**/*.sh: Bash scripts must start with #!/bin/bash and be organized into modular functions (Logging; Error handling and logging; Deployment functions), applying DRY via helpers
Load environment from .env or config.sh, declare globals in config, update .env.example, and validate env vars early
Use helper logging functions (echoDebug, error, warning, success), validate inputs early, check exit status with checkFailure, and use set -e where appropriate
Provide automated retry for RPC issues, wrap Foundry deployment, validate requirements/dependencies, and implement robust error recovery
Provide clear prompts (e.g., gum choose), feedback, progress indicators, usage instructions, examples, and maintain helpful help text/errors
Keep scripts modular, consistent in naming, compatible with tooling, and tested; update documentation with changes
Files:
script/pioneer/pioneer-qa.sh
🧠 Learnings (18)
📚 Learning: 2024-09-23T02:04:16.323Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#807
File: src/Periphery/GasZipPeriphery.sol:49-53
Timestamp: 2024-09-23T02:04:16.323Z
Learning: When `LibAsset.maxApproveERC20` is used with `type(uint256).max`, the team acknowledges and accepts the associated security risks. In future reviews, avoid flagging this as a concern.
Applied to files:
script/pioneer/PioneerQA.sol
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
PR: lifinance/contracts#1328
File: test/solidity/Periphery/GasZipPeriphery.t.sol:223-233
Timestamp: 2025-08-27T23:36:40.773Z
Learning: In bridge facet swap flows with requiresDeposit=false, tokens are already held by the diamond contract from previous swaps. When subsequent contracts like GasZipPeriphery call LibAsset.depositAsset, they pull tokens from msg.sender (the diamond) to themselves via transferFrom, requiring no additional approvals since the diamond already owns the tokens.
Applied to files:
script/pioneer/PioneerQA.sol
📚 Learning: 2025-08-29T11:53:38.549Z
Learnt from: mirooon
PR: lifinance/contracts#1328
File: test/solidity/Periphery/LDA/BaseCoreRouteTest.t.sol:379-388
Timestamp: 2025-08-29T11:53:38.549Z
Learning: In test/solidity/Periphery/LDA/BaseCoreRouteTest.t.sol, for the revert-testing helper function _executeAndVerifySwap, only the aggregator branch (CommandType.DistributeSelfERC20) should use amountIn-1 to underfund and trigger insufficient balance errors, while user-funded branches should use the full amountIn to test other error conditions.
Applied to files:
script/pioneer/PioneerQA.sol
📚 Learning: 2025-08-29T05:16:40.067Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-29T09:56:59.816Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.816Z
Learning: Applies to **/*.sh : Bash scripts must start with #!/bin/bash and be organized into modular functions (Logging; Error handling and logging; Deployment functions), applying DRY via helpers
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-29T09:56:59.816Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.816Z
Learning: Applies to **/*.sh : Keep scripts modular, consistent in naming, compatible with tooling, and tested; update documentation with changes
Applied to files:
script/pioneer/pioneer-qa.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/pioneer/pioneer-qa.sh
📚 Learning: 2024-11-26T01:03:43.597Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: deployments/scroll.diamond.json:111-111
Timestamp: 2024-11-26T01:03:43.597Z
Learning: When reviewing pull requests, only point out missing contract addresses in `deployments/<network>.diamond.json` if the contract's address is present in `deployments/<network>.json` but missing in `deployments/<network>.diamond.json`. If the contract is not deployed on a network (i.e., not present in `deployments/<network>.json`), then no action is needed.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2024-09-23T01:42:03.075Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#807
File: script/deploy/facets/DeployGasZipFacet.s.sol:22-35
Timestamp: 2024-09-23T01:42:03.075Z
Learning: In deployment scripts like `DeployGasZipFacet.s.sol`, do not report issues suggesting adding error handling for missing configuration files or unexpected JSON structures, as the script will fail if the file is missing.
Applied to files:
script/pioneer/pioneer-qa.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:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-29T09:56:59.816Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.816Z
Learning: Applies to script/deploy/resources/deployRequirements.json : Maintain deployRequirements.json to define dependencies, zero-address restrictions, required external configs; consumed by helperFunctions.sh for validation
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-08-19T05:10:12.922Z
Learnt from: ezynda3
PR: lifinance/contracts#1303
File: script/utils/network.ts:27-32
Timestamp: 2025-08-19T05:10:12.922Z
Learning: In the lifinance/contracts repository, the team decided to standardize RPC environment variable naming by converting all hyphens to underscores in network names. For example, "tron-shasta" becomes "ETH_NODE_URI_TRON_SHASTA". The getRPCEnvVarName() helper in script/utils/network.ts implements this standard and should be used consistently across the codebase instead of direct string manipulation.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-01-24T14:53:20.703Z
Learnt from: mirooon
PR: lifinance/contracts#950
File: script/demoScripts/utils/demoScriptChainConfig.ts:17-20
Timestamp: 2025-01-24T14:53:20.703Z
Learning: In the LiFi contracts repository, RPC URLs should be strictly sourced from environment variables without fallback to networks.json configuration.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-04-17T02:40:15.827Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1102
File: script/tasks/diamondSyncSigs_FAST.sh:112-117
Timestamp: 2025-04-17T02:40:15.827Z
Learning: Do not suggest using `mapfile` or `read -a` for array assignments in the lifinance/contracts repository as these features may not be available in the user's environment. The scripts are designed with the knowledge that network names will only ever consist of one word, making word splitting concerns less relevant.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2024-11-05T17:15:33.833Z
Learnt from: maxklenk
PR: lifinance/contracts#782
File: script/demoScripts/demoPermit2.ts:100-107
Timestamp: 2024-11-05T17:15:33.833Z
Learning: In demo scripts using testing wallets, passing private keys via command-line arguments is acceptable.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-06-06T16:10:04.674Z
Learnt from: mirooon
PR: lifinance/contracts#1193
File: script/tasks/diamondSyncWhitelistedAddresses.sh:97-97
Timestamp: 2025-06-06T16:10:04.674Z
Learning: In script/tasks/diamondSyncWhitelistedAddresses.sh (lines 97 and 133), there are shell array assignments using command substitution that should be fixed using mapfile instead: `WHITELISTED_ADDRESSES=($(getApprovedWhitelistedAddresses))` should be `mapfile -t WHITELISTED_ADDRESSES < <(getApprovedWhitelistedAddresses)` and similarly for `WHITELISTED_ADDRESSES_UPDATED`. User mirooon requested to be reminded of this issue in future PRs when this file is modified.
Applied to files:
script/pioneer/pioneer-qa.sh
📚 Learning: 2025-06-15T11:05:21.990Z
Learnt from: mirooon
PR: lifinance/contracts#1193
File: script/tasks/diamondSyncSelectors.sh:112-117
Timestamp: 2025-06-15T11:05:21.990Z
Learning: In script/tasks/diamondSyncSelectors.sh (line 112), there is a shell array assignment using command substitution that should be fixed using mapfile instead: `CFG_SELECTORS=($(jq -r '.[] | sh' "./config/whitelistedSelectors.json" | tr -d \' | tr '[:upper:]' '[:lower:]'))` should be `mapfile -t CFG_SELECTORS < <(jq -r '.[]' "./config/whitelistedSelectors.json" | tr '[:upper:]' '[:lower:]')`. User mirooon requested to be reminded of this issue in future PRs when this file is modified.
Applied to files:
script/pioneer/pioneer-qa.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/pioneer/pioneer-qa.sh
🔇 Additional comments (4)
script/pioneer/PioneerQA.sol (2)
25-25: Good: upfront diamond type checkEarly revert on non-contract target avoids wasted gas and misconfig pitfalls.
47-51: Confirm broadcasted value semanticsUnder Foundry broadcasting, this call should send ETH from the broadcaster EOA, not this contract. Just verify locally that value forwarding works as expected for your setup.
script/pioneer/pioneer-qa.sh (2)
86-89: Good: guard against per-tx zero amountPrevents accidental 0-value txs at high split counts.
115-118: Verify response chainId matches requestExtra safety against server-side mismatches.
# Extract necessary fields from the response TO_CHAIN_ID=$(echo "$RESPONSE" | jq -r '.toChainId') + if [ "$TO_CHAIN_ID" != "$TO_CHAIN" ]; then + echo "Mismatch: requested toChain=$TO_CHAIN, response toChainId=$TO_CHAIN_ID" >&2 + exit 1 + fi
| DEPLOY_FILE="./deployments/${NETWORK}.staging.json" | ||
| test -f "$DEPLOY_FILE" || { echo "Missing $DEPLOY_FILE"; exit 1; } | ||
| DIAMOND=$(jq -r '.LiFiDiamond' "$DEPLOY_FILE") | ||
| echo "Using LiFi Diamond at $DIAMOND" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate diamond address after jq
Catch missing/null address early.
DIAMOND=$(jq -r '.LiFiDiamond' "$DEPLOY_FILE")
echo "Using LiFi Diamond at $DIAMOND"
+[ -n "$DIAMOND" ] && [ "$DIAMOND" != "null" ] || { echo "LiFiDiamond is missing in $DEPLOY_FILE"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DEPLOY_FILE="./deployments/${NETWORK}.staging.json" | |
| test -f "$DEPLOY_FILE" || { echo "Missing $DEPLOY_FILE"; exit 1; } | |
| DIAMOND=$(jq -r '.LiFiDiamond' "$DEPLOY_FILE") | |
| echo "Using LiFi Diamond at $DIAMOND" | |
| DEPLOY_FILE="./deployments/${NETWORK}.staging.json" | |
| test -f "$DEPLOY_FILE" || { echo "Missing $DEPLOY_FILE"; exit 1; } | |
| DIAMOND=$(jq -r '.LiFiDiamond' "$DEPLOY_FILE") | |
| echo "Using LiFi Diamond at $DIAMOND" | |
| [ -n "$DIAMOND" ] && [ "$DIAMOND" != "null" ] || { echo "LiFiDiamond is missing in $DEPLOY_FILE"; exit 1; } |
🤖 Prompt for AI Agents
In script/pioneer/pioneer-qa.sh around lines 21 to 25, after extracting DIAMOND
with jq, add a validation that DIAMOND is non-empty and not the literal "null";
if the value is missing/empty/null print a clear error like "Missing LiFiDiamond
in $DEPLOY_FILE" and exit non‑zero so the script fails early. Ensure the check
handles both empty string and the "null" string returned by jq and runs
immediately after the echo assignment.
Which Jira task belongs to this PR?
https://lifi.atlassian.net/browse/LII-99?atlOrigin=eyJpIjoiOWU5MzYwZGVlZjdhNGI1M2EzMTA0MjlhZWNlNjhmNzUiLCJwIjoiaiJ9
Why did I implement it this way?
We need a way to reliable to do throughput testing of Pioneer and the wallet service. These scripts allows for throughput testing on the order of thousands of transactions in a short period of time.
The scripts are composed of 3 files:
--slow).The idea is to execute upwards of 1k to 10k transactions in a short period of time from multiple different origin to multiple different origins.
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)