feat(hyperlane): integrate hyperlane-cosmos v1.1.0 bridge modules#301
Merged
Conversation
- Add Hyperlane Core and Warp modules to enable cross-chain messaging and token bridging between BitSong and EVM chains - Register hyperlane and warp store keys, module accounts, and keepers - Wire WarpKeeper with CoreKeeper dependency for mailbox access - Add begin/end/init blocker ordering after IBC modules - Grant Minter+Burner permissions to warp module for synthetic tokens - Add hyperlane-cosmos v1.1.0 dependency
|
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:
📝 WalkthroughWalkthroughIntegrates Hyperlane and Warp modules into the Bitsong app by registering them in module basics, configuring their block execution order, implementing a v024 chain upgrade handler, and updating dependencies to support the new modules; extends .gitignore for development artifacts. ChangesCore Module Integration
Build Configuration
Sequence Diagram(s)sequenceDiagram
participant App as Bitsong App Init
participant ModuleRegistry as Module Registry
participant HyperlaneModule as Hyperlane Module
participant WarpModule as Warp Module
participant Blockchain as Blockchain Runtime
rect rgba(0, 100, 150, 0.5)
Note over App,ModuleRegistry: Application Startup
App->>ModuleRegistry: Register AppModuleBasics
ModuleRegistry->>HyperlaneModule: Register basics
ModuleRegistry->>WarpModule: Register basics
App->>ModuleRegistry: Instantiate AppModules
ModuleRegistry->>HyperlaneModule: Create module instance
ModuleRegistry->>WarpModule: Create module instance
end
rect rgba(150, 100, 0, 0.5)
Note over Blockchain: Block Execution
Blockchain->>HyperlaneModule: BeginBlock
Blockchain->>WarpModule: BeginBlock
Blockchain->>HyperlaneModule: EndBlock
Blockchain->>WarpModule: EndBlock
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
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)
app/modules.go (1)
188-222:⚠️ Potential issue | 🔴 CriticalRemove duplicate module names in
orderInitBlockers—they cause doubleInitGenesiscalls.
authtypes.ModuleNameappears at lines 191 and 206, andibcwasmtypes.ModuleNameappears at lines 207 and 216. In Cosmos SDK'smodule.Manager, duplicate names in the initialization order list cause that module'sInitGenesisto be called twice, which violates the intended initialization contract and can corrupt chain state. These duplicates are introduced in this PR and must be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/modules.go` around lines 188 - 222, In orderInitBlockers(), remove the duplicate module names so each ModuleName appears only once: drop the second occurrence of authtypes.ModuleName and the second occurrence of ibcwasmtypes.ModuleName to prevent double InitGenesis calls; update the returned slice in orderInitBlockers() so every module constant (e.g., capabilitytypes.ModuleName, authtypes.ModuleName, ibcwasmtypes.ModuleName, etc.) is unique while preserving the intended initialization order.
🧹 Nitpick comments (1)
.gitignore (1)
66-68: Consider a broader.env*pattern to guard against accidental secret commits.The specific paths
scripts/hyperlane/.envandinfra/.env.secretsare good, but any new.envfile added elsewhere won't be caught. A catch-all pattern like**/.env*(with explicit exceptions via!if needed) would be more defensive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 66 - 68, Replace the two explicit secret-file entries with a broader catch-all pattern to prevent accidental commits of any environment files: remove or supersede the specific lines "scripts/hyperlane/.env" and "infra/.env.secrets" and add a recursive pattern like "**/.env*" to .gitignore (optionally add explicit negate patterns with "!" if you need to allow particular committed example env files).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 111: Your go.mod references an indirect dependency
github.com/ethereum/go-ethereum v1.14.12 which has a HIGH-severity
vulnerability; update to a patched release by either updating the upstream
consumer (hyperlane-cosmos v1.1.0) to a version that depends on go-ethereum
v1.14.13 or v1.16.7+, or add a go.mod replace directive to pin
github.com/ethereum/go-ethereum to v1.14.13 (or v1.16.7+) to override the
indirect version; after upgrading, ensure any Ethereum node key is rotated
(delete <datadir>/geth/nodekey before restart) as recommended.
---
Outside diff comments:
In `@app/modules.go`:
- Around line 188-222: In orderInitBlockers(), remove the duplicate module names
so each ModuleName appears only once: drop the second occurrence of
authtypes.ModuleName and the second occurrence of ibcwasmtypes.ModuleName to
prevent double InitGenesis calls; update the returned slice in
orderInitBlockers() so every module constant (e.g., capabilitytypes.ModuleName,
authtypes.ModuleName, ibcwasmtypes.ModuleName, etc.) is unique while preserving
the intended initialization order.
---
Nitpick comments:
In @.gitignore:
- Around line 66-68: Replace the two explicit secret-file entries with a broader
catch-all pattern to prevent accidental commits of any environment files: remove
or supersede the specific lines "scripts/hyperlane/.env" and
"infra/.env.secrets" and add a recursive pattern like "**/.env*" to .gitignore
(optionally add explicit negate patterns with "!" if you need to allow
particular committed example env files).
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (15)
.github/workflows/push-docker-image.yml (2)
73-75: Quote$GITHUB_OUTPUTto satisfy SC2086.Static analysis flags unquoted
$GITHUB_OUTPUTon all three lines. While the GHA-provided path is unlikely to contain spaces, quoting is a good defensive habit.Proposed fix
- echo "IMAGE_TAGS<<EOF" >> $GITHUB_OUTPUT - echo "$IMAGE_TAGS" >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT + { + echo "IMAGE_TAGS<<EOF" + echo "$IMAGE_TAGS" + echo "EOF" + } >> "$GITHUB_OUTPUT"This also addresses the SC2129 style hint by consolidating the three redirects into a single grouped redirect.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push-docker-image.yml around lines 73 - 75, The three echo lines that append IMAGE_TAGS to the GHA output use an unquoted $GITHUB_OUTPUT and multiple redirects; change them to a single grouped append that redirects once to the quoted "$GITHUB_OUTPUT" and echo the IMAGE_TAGS content (use the IMAGE_TAGS variable quoted as "$IMAGE_TAGS" inside the group) so the path is quoted (satisfying SC2086) and you avoid repeated redirects (addresses SC2129).
28-29: Hardcoded branch name will keep triggering builds after merge.Once this PR merges to
main, thefeat-hyperlanebranch filter will either become a no-op (if the branch is deleted) or keep building stale images. Consider replacing with a wildcard pattern or removing before merge if branch-based images are only needed during development.branches: - - 'feat-hyperlane' + - 'main'Or, if multiple feature branches should produce images:
branches: - - 'feat-hyperlane' + - 'feat-*'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push-docker-image.yml around lines 28 - 29, The workflow currently hardcodes a branch filter value 'feat-hyperlane' under the branches key which will keep triggering builds after merge; update the branches filter (in the push trigger section where branches: ['feat-hyperlane'] is defined) to either remove the branches filter entirely if you want builds on all branches, replace the literal with a wildcard pattern such as 'feat-*' to capture multiple feature branches, or parameterize it via a repository secret or environment variable so it won't become stale after merging.scripts/hyperlane/03-evm-deploy.sh (1)
61-116: Implicit global variablesDEPLOYED_EVMandTOKEN_IDcouple functions.
DEPLOYED_EVM(Line 64) andTOKEN_ID(Line 123) are set withoutlocal, making them implicitly global.enroll_cosmos_sideandenroll_evm_sidedepend on these globals being set by prior function calls. This works but is fragile — consider either documenting the contract or passing values via state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/03-evm-deploy.sh` around lines 61 - 116, deploy_hyp_erc20 and the token enrollment functions rely on implicitly global variables (DEPLOYED_EVM, TOKEN_ID); make these locals and/or use the existing save_state/load_state to avoid fragile globals: declare DEPLOYED_EVM as local inside deploy_hyp_erc20 (it’s already saved with save_state "evm_hyp_erc20"), declare TOKEN_ID local where it’s assigned, and update enroll_cosmos_side and enroll_evm_side to read values via load_state ("evm_hyp_erc20" and the token state key) or accept them as parameters instead of depending on globals; ensure references to evm_to_bytes32, save_state, and load_state remain consistent.scripts/hyperlane/04-agents.sh (2)
444-453: State cleanup runsjqper key — consolidate into a single call.The loop runs 7 separate jq read-write cycles on the state file. A single
jqinvocation deleting all keys is more efficient and atomic.Proposed fix
- for key in validator_addr bitsong_multisig_ism_id basesepolia_multisig_ism \ - validator_bitsong_announced validator_basesepolia_announced \ - ism_updated_cosmos_side ism_updated_evm_side; do - [[ -f "$STATE_FILE" ]] && jq --arg k "$key" 'del(.[$k])' "$STATE_FILE" > "${STATE_FILE}.tmp" \ - && mv "${STATE_FILE}.tmp" "$STATE_FILE" - done + if [[ -f "$STATE_FILE" ]]; then + jq 'del(.validator_addr, .bitsong_multisig_ism_id, .basesepolia_multisig_ism, + .validator_bitsong_announced, .validator_basesepolia_announced, + .ism_updated_cosmos_side, .ism_updated_evm_side)' \ + "$STATE_FILE" > "${STATE_FILE}.tmp" && mv "${STATE_FILE}.tmp" "$STATE_FILE" + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/04-agents.sh` around lines 444 - 453, The loop that runs jq once per key is inefficient; replace it with a single jq invocation that deletes all seven keys at once to be atomic and faster: check [[ -f "$STATE_FILE" ]] and then run jq with a del() call that removes .validator_addr, .bitsong_multisig_ism_id, .basesepolia_multisig_ism, .validator_bitsong_announced, .validator_basesepolia_announced, .ism_updated_cosmos_side, and .ism_updated_evm_side in one expression, writing to "${STATE_FILE}.tmp" and mv-ing it back (preserve the existing temp-file atomic write pattern used elsewhere).
70-124: Inline Python bech32 encoder lacks dependency pre-check and leaks key via process args.The
ecdsapackage is required but only mentioned in the error message (Line 107). A pre-check (python3 -c "import ecdsa") would fail fast with a clear message. Also,$COSMOS_SIGNER_KEYis interpolated into the Python command string (Line 103), making it visible in/proc/PID/cmdline. For a devnet script this is acceptable, but consider piping via stdin for better hygiene.Proposed improvement
fund_cosmos_signer() { log_step "Step 1b: Fund Cosmos Signer" + python3 -c "import ecdsa" 2>/dev/null || { log_err "Python ecdsa package missing: pip3 install ecdsa"; return 1; } + local cosmos_signer_addr - cosmos_signer_addr=$(python3 -c " + cosmos_signer_addr=$(python3 - "$COSMOS_SIGNER_KEY" << 'PYEOF' +import sys from ecdsa import SigningKey, SECP256k1 import hashlib def to_bech32(hex_key, prefix='bitsong'): pk = bytes.fromhex(hex_key.replace('0x','')) ... -print(to_bech32('$COSMOS_SIGNER_KEY')) -" 2>/dev/null) +print(to_bech32(sys.argv[1])) +PYEOF +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/04-agents.sh` around lines 70 - 124, The embedded Python call in fund_cosmos_signer both omits a pre-check for the ecdsa dependency and leaks COSMOS_SIGNER_KEY on the process command line; add an upfront check (e.g., run a short python3 -c "import ecdsa" check and fail with a clear log_err if it raises) and change the python invocation that derives the bech32 address to read the private key from stdin or an environment-safe source instead of interpolating $COSMOS_SIGNER_KEY into the command string (update the python3 call used in fund_cosmos_signer and related logging to reflect this safer input method and surface dependency failures clearly).scripts/hyperlane/status.sh (1)
34-43:date -dis GNU-only — will fail on macOS.
date -d "$started"(Line 36) is a GNU coreutils extension not available on macOS's BSDdate. If cross-platform use is intended, considerdateportability or document the Linux requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/status.sh` around lines 34 - 43, The script uses GNU-only date parsing (date -d "$started") inside the block handling status/started (variables status, started, start_epoch), which fails on macOS; replace this with a portable approach: detect GNU date vs BSD date (e.g., check if date -d works) and if unavailable parse the timestamp using a macOS-compatible command (date -j -f or a small portable fallback using python/perl) to compute start_epoch, preserving the rest of the uptime calculation logic; ensure the change keeps the same variable names (start_epoch, now_epoch, diff, uptime) and behavior for Linux while providing the BSD/date-compatible branch for macOS.scripts/hyperlane/run-all.sh (1)
58-77: Unquoted$CLEAN_FLAGis intentional but fragile.The unquoted
$CLEAN_FLAGon Lines 61, 63, 64, 73 relies on word-splitting to pass zero arguments when empty. This works but is unconventional underset -euo pipefail. Using an array would be more robust and idiomatic.Alternative using array
-CLEAN_FLAG="" -[[ "$CLEAN" == "true" ]] && CLEAN_FLAG="--clean" +CLEAN_ARGS=() +[[ "$CLEAN" == "true" ]] && CLEAN_ARGS=(--clean) -run_phase 1 "01-chain.sh" $CLEAN_FLAG +run_phase 1 "01-chain.sh" "${CLEAN_ARGS[@]+"${CLEAN_ARGS[@]}"}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/run-all.sh` around lines 58 - 77, The script currently uses a string CLEAN_FLAG and an unquoted expansion which relies on word-splitting; replace this with a shell array (e.g. CLEAN_ARGS) that is empty by default and set to contain --clean when CLEAN is true, then pass that array to run_phase invocations instead of CLEAN_FLAG (update the calls that reference CLEAN_FLAG and the run_phase invocations for 01-chain.sh, 03-evm-deploy.sh, 04-agents.sh, 06-fantoken-route.sh to expand the array), ensuring the array is expanded properly so no spurious empty arguments are passed.scripts/hyperlane/06-fantoken-route.sh (5)
608-617:--cleanloop runsjq | mvper key — N sequential file rewrites.For a small fixed set of suffixes this is fine, but it could be a single
jqcall with multipledel()operations. Minor efficiency nit.♻️ Single jq pass to delete all keys
if [[ "$CLEAN" == "true" ]]; then log "Cleaning state for symbol '$FT_KEY'..." - for suffix in denom minted token_id evm_hyp_erc20 evm_hyp_erc20_bytes32 \ - router_enrolled evm_router_enrolled evm_ism_set relayer_restarted; do - jq --arg k "ft_${FT_KEY}_${suffix}" 'del(.[$k])' "$STATE_FILE" > "${STATE_FILE}.tmp" \ - && mv "${STATE_FILE}.tmp" "$STATE_FILE" - done + local prefix="ft_${FT_KEY}_" + jq --arg p "$prefix" 'with_entries(select(.key | startswith($p) | not))' \ + "$STATE_FILE" > "${STATE_FILE}.tmp" && mv "${STATE_FILE}.tmp" "$STATE_FILE" unregister_route log_ok "State cleaned for '$FT_KEY'" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/06-fantoken-route.sh` around lines 608 - 617, The CLEAN branch currently rewrites STATE_FILE once per suffix; instead build the list of suffixes (same set used now) and perform a single jq invocation that deletes all "ft_${FT_KEY}_${suffix}" keys in one pass (use jq del with an array or chained del() calls generated from the suffix list) and then mv the single temporary file back to STATE_FILE; keep the surrounding log calls and the unregister_route call and reference CLEAN, FT_KEY, STATE_FILE and unregister_route so you only perform one jq | mv instead of N sequential rewrites.
86-95:unregister_routeusesgrep -vwith unescaped regex fromFT_KEY.
grep -v "^${FT_KEY}$"treatsFT_KEYas a regex. If a symbol ever contains.,*, or other metacharacters, it could unintentionally remove other routes. Usegrep -Fxvfor a fixed-string exact-line match.♻️ Use fixed-string matching
- new_list=$(echo "$list" | tr ',' '\n' | grep -v "^${FT_KEY}$" | paste -sd ',' -) || true + new_list=$(echo "$list" | tr ',' '\n' | grep -Fxv "$FT_KEY" | paste -sd ',' -) || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/06-fantoken-route.sh` around lines 86 - 95, unregister_route currently uses grep -v with an unescaped regex ("grep -v \"^${FT_KEY}$\"") so metas in FT_KEY can match unexpectedly; update the pipeline in unregister_route (which calls load_state and save_state) to use fixed-string exact-line matching (e.g., replace the grep invocation with grep -Fxv or equivalent) so the removal compares FT_KEY as a literal exact line and won’t treat characters like . or * as regex metacharacters.
465-481: Private keys passed as CLI arguments todocker runare visible viadocker inspectandps.Lines 477 and 480 pass
COSMOS_SIGNER_KEYandEVM_RELAYER_KEYdirectly on the command line. Any user on the host can read these viaps auxordocker inspect. For a local devnet script this is acceptable, but if this pattern is reused for testnet/mainnet, secrets should be passed via Docker secrets, a mounted file, or environment variables (-eflag, which is still visible indocker inspectbut not inps).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/06-fantoken-route.sh` around lines 465 - 481, The script exposes private keys by passing COSMOS_SIGNER_KEY and EVM_RELAYER_KEY directly on the docker run command line (used for --chains.bitsong.signer.key and --chains.basesepolia.signer.key); change this to load secrets from a safer source such as Docker secrets or mounted files and reference those inside the container (e.g., mount $BITSONG_HOME/cosmos_key:/run/secrets/cosmos_key:ro and use the file path inside the relayer config or set an env var reading that file), or at minimum export them as environment variables via -e (noting they remain visible in docker inspect) and update the CLI flags to read from the secret file paths or env vars rather than embedding the raw keys on the command line.
182-212:build_whitelistconstructs JSON via string concatenation — fragile.Hand-assembling a JSON array with string interpolation is error-prone (unescaped characters, stray commas, etc.). Consider using
jqto build the array incrementally, which inherently produces valid JSON.♻️ Use jq to build the whitelist
build_whitelist() { - local whitelist="[" + local whitelist="[]" # Always include ubtsg route local token_id evm_hyp_erc20 token_id=$(load_state "token_id") evm_hyp_erc20=$(load_state "evm_hyp_erc20") if [[ -n "$token_id" && -n "$evm_hyp_erc20" ]]; then - whitelist+="{\"senderAddress\":\"${token_id}\",\"destinationDomain\":\"${REMOTE_DOMAIN}\"}," - whitelist+="{\"senderAddress\":\"$(evm_to_bytes32 "$evm_hyp_erc20")\",\"destinationDomain\":\"${DOMAIN_ID}\"}," + whitelist=$(echo "$whitelist" | jq --arg sa "$token_id" --arg dd "$REMOTE_DOMAIN" \ + '. + [{"senderAddress": $sa, "destinationDomain": $dd}]') + whitelist=$(echo "$whitelist" | jq --arg sa "$(evm_to_bytes32 "$evm_hyp_erc20")" --arg dd "$DOMAIN_ID" \ + '. + [{"senderAddress": $sa, "destinationDomain": $dd}]') fi # ... same pattern for fantoken routes ... - whitelist="${whitelist%,}]" # Remove trailing comma echo "$whitelist" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/06-fantoken-route.sh` around lines 182 - 212, The build_whitelist function currently constructs JSON by concatenating strings which is fragile; change it to build the array using jq so items are appended as real JSON objects. Replace the manual string assembly in build_whitelist (including the blocks that add ubtsg using token_id/evm_hyp_erc20 and the loop that adds each fantoken using ft_${sym}_token_id and ft_${sym}_evm_hyp_erc20) with calls that pipe object literals to jq --arg to set senderAddress and destinationDomain and use jq -s to produce the final array; keep using evm_to_bytes32(token) and load_state(...) to read values but pass them via jq arguments so escaping is handled and you never manually trim trailing commas.
57-60:ft_save/ft_loadassumeFT_KEYis non-empty — no guard.If
ft_saveorft_loadis accidentally called beforeFT_KEYis set, the state key degenerates toft__<suffix>(double underscore), silently writing/reading the wrong key. The main flow does enforceFT_KEYis set, but a guard or assertion would make this more robust against future refactors.♻️ Add a guard
-ft_save() { save_state "ft_${FT_KEY}_$1" "$2"; } -ft_load() { load_state "ft_${FT_KEY}_$1"; } +ft_save() { [[ -n "$FT_KEY" ]] || { log_err "BUG: ft_save called with empty FT_KEY"; return 1; }; save_state "ft_${FT_KEY}_$1" "$2"; } +ft_load() { [[ -n "$FT_KEY" ]] || { log_err "BUG: ft_load called with empty FT_KEY"; return 1; }; load_state "ft_${FT_KEY}_$1"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/06-fantoken-route.sh` around lines 57 - 60, ft_save and ft_load currently assume FT_KEY is set and will silently use "ft__<suffix>" if empty; add a guard in both helper functions (ft_save and ft_load) that asserts FT_KEY is non-empty before calling save_state/load_state, and fail fast with an explanatory error message (and non-zero exit) if FT_KEY is empty so you don't read/write the wrong state key.scripts/hyperlane/07-fantoken-test.sh (2)
117-121: Inline bytes32 padding duplicatesevm_to_bytes32from lib.sh.Lines 120–121 replicate the same address-to-bytes32 conversion that
evm_to_bytes32already provides. Use the library function to stay DRY and avoid divergence.♻️ Use library helper
- local hex="${evm_signer_addr#0x}"; hex=$(echo "$hex" | tr '[:upper:]' '[:lower:]') - evm_signer_bytes32=$(printf "0x%064s" "$hex" | tr ' ' '0') + evm_signer_bytes32=$(evm_to_bytes32 "$evm_signer_addr")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/07-fantoken-test.sh` around lines 117 - 121, The inline bytes32 padding duplicates existing helper evm_to_bytes32; replace the manual conversion of evm_signer_addr -> evm_signer_bytes32 (the hex trimming/lowercasing and printf padding) with a call to evm_to_bytes32 from lib.sh, e.g. evm_signer_bytes32=$(evm_to_bytes32 "$evm_signer_addr"), keeping the preceding derivation and error check for evm_signer_addr intact and ensuring the resulting evm_signer_bytes32 variable is used as before.
214-221:totalSupplyis used as a proxy for signer balance — works only in single-user devnet.Line 216 checks
totalSupplyto determine if the EVM signer has tokens to burn viatransferRemote. If multiple users have minted tokens,totalSupply > 0doesn't guarantee the signer holds any. For the current single-user devnet flow this works, but abalanceOf(signer)check would be more correct.♻️ Check actual signer balance
- evm_balance=$(cast call "$ft_evm_hyp_erc20" "totalSupply()(uint256)" --rpc-url "$EVM_RPC" 2>/dev/null) || evm_balance="0" + local evm_signer_addr + evm_signer_addr=$(cast wallet address --private-key "$EVM_RELAYER_KEY" 2>/dev/null) + evm_balance=$(cast call "$ft_evm_hyp_erc20" "balanceOf(address)(uint256)" "$evm_signer_addr" --rpc-url "$EVM_RPC" 2>/dev/null) || evm_balance="0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/07-fantoken-test.sh` around lines 214 - 221, The script wrongly uses totalSupply() to infer the signer's token holdings; update the check to call balanceOf(<signer_address>) instead. Replace the cast call on ft_evm_hyp_erc20 from "totalSupply()(uint256)" to "balanceOf(address)(uint256)" and pass the runtime signer address variable used elsewhere in the script (e.g., the EVM signer variable like EVM_SIGNER or EVM_FROM) while keeping EVM_RPC; store the result back into evm_balance and keep the existing error logging path if the balance is "0". Ensure the balanceOf call targets the same contract variable ft_evm_hyp_erc20.scripts/hyperlane/lib.sh (1)
119-129: Concurrentsave_statecalls can corrupt the state file.If two scripts (or backgrounded subshells) call
save_stateconcurrently, the read-modify-write on the sameSTATE_FILEcan race: both read the same version, one overwrites the other's write. For a sequential orchestration script this is fine in practice, but the library is designed for general reuse. Consider usingflockaround the read-modify-write.♻️ Optional: wrap with flock
save_state() { local key="$1" value="$2" [[ ! -f "$STATE_FILE" ]] && echo '{}' > "$STATE_FILE" - local tmp="${STATE_FILE}.tmp" - jq --arg k "$key" --arg v "$value" '.[$k] = $v' "$STATE_FILE" > "$tmp" && mv "$tmp" "$STATE_FILE" + ( + flock -x 200 + local tmp="${STATE_FILE}.tmp" + jq --arg k "$key" --arg v "$value" '.[$k] = $v' "$STATE_FILE" > "$tmp" && mv "$tmp" "$STATE_FILE" + ) 200>"${STATE_FILE}.lock" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hyperlane/lib.sh` around lines 119 - 129, The save_state function is vulnerable to concurrent writes; wrap its read-modify-write against STATE_FILE with an exclusive flock to prevent races: acquire an exclusive lock on STATE_FILE (or a lockfd derived from STATE_FILE) before reading, running jq to update into a temp file, and mv the temp into STATE_FILE, then release the lock; also apply a shared flock in load_state when reading STATE_FILE to avoid torn reads. Update the save_state and load_state functions to use flock (fall back to a safe error if flock is unavailable) while keeping the existing behavior of creating STATE_FILE if missing and preserving atomic mv.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/push-docker-image.yml:
- Around line 22-29: The workflow currently uses both the release and push.tags
triggers (the YAML keys "release" and "push: tags: - 'v[0-9]+.[0-9]+.[0-9]+'")
which can fire for the same release; remove or disable one of these triggers to
avoid duplicate runs—e.g., delete the entire "push: tags:" block (or
alternatively remove the "release:" block) so only a single trigger (either
"release" or the tag-based "push") remains, keeping the branch filter "branches:
- 'feat-hyperlane'" untouched if branch-scoped pushes are required.
In `@app/keepers/keepers.go`:
- Around line 123-124: Add the hyperlane and warp module store keys to the v023
upgrade's StoreUpgrades so their KV stores are created during migration: update
the StoreUpgrades.Added slice (the v023 upgrade definition that contains
StoreUpgrades) to include hyperlanetypes.ModuleName and warptypes.ModuleName
(follow the same pattern used in v021/v022), leaving Deleted empty; ensure you
reference the existing StoreUpgrades variable/struct in the v023 upgrade block
so the stores are initialized at upgrade time.
In `@scripts/hyperlane/02-hyperlane.sh`:
- Around line 170-195: The update_enrollment function saves
evm_enrollment_updated but doesn't update the router_enrolled flag, so reruns
still think enrollment is pending; modify update_enrollment (the function that
calls save_state "evm_hyp_erc20" / "evm_hyp_erc20_bytes32" /
"evm_enrollment_updated") to also call save_state "router_enrolled" "true" after
a successful submit_tx (use the same success path where log_ok "Enrollment
updated" is invoked) so the existing setup_hyperlane check for router_enrolled
will recognize the enrollment has completed.
- Around line 199-208: The --update-enrollment case currently reads
UPDATE_EVM_ADDR="$2" and does shift 2 without validating $2; add a guard in the
case branch for --update-enrollment that checks whether $2 is present and not
another flag (e.g., [[ -z "$2" || "$2" =~ ^- ]] ), and if missing print a clear
error and exit; otherwise set UPDATE_EVM_ADDR="$2" and shift 2. This fix targets
the --update-enrollment branch handling of UPDATE_EVM_ADDR and the use of
$2/shift 2.
In `@scripts/hyperlane/03-evm-deploy.sh`:
- Around line 238-260: The script mixes "Phase 3" and "Phase 4" labels: update
the internal messages and state handling to match the orchestrator's phase
numbering (or vice versa) so they are consistent; specifically, change the text
strings and log messages that mention "Phase 4" (the jq cleanup log at the CLEAN
block, the banner call banner "Phase 4: Deploy Warp Route", and any other log_ok
messages referencing Phase 4) to "Phase 3" (or change the prerequisite check
that says "Phase 3 state not found" to "Phase 4 state not found" if you prefer
moving the whole script to Phase 4), and ensure STATE_FILE usage, DEPLOYER_ADDR
output (log_ok "EVM deployer: $DEPLOYER_ADDR"), and load_state mailbox_id
messages all use the same phase label; keep symbol names (STATE_FILE, CLEAN,
banner, log_ok, load_state) intact and only alter the human-facing strings so
the orchestrator and script labels align.
In `@scripts/hyperlane/05-test.sh`:
- Around line 296-301: The summary shows blank values because load_state returns
an empty string with exit code 0 for missing keys, so the "|| echo 'not run'"
fallback never runs; change the print logic to capture load_state's output (for
cosmos_to_evm_test_passed and evm_to_cosmos_test_passed) and explicitly test for
an empty string, printing "not run" when empty and the actual value otherwise
(or alter load_state to return non‑zero on missing keys and keep the existing ||
fallback). Target the echo lines that reference
load_state(cosmos_to_evm_test_passed) and load_state(evm_to_cosmos_test_passed).
In `@scripts/hyperlane/07-fantoken-test.sh`:
- Around line 236-238: The variable initial_height is assigned but never used
(ShellCheck SC2034); remove the unused assignment or use it for diagnostics.
Locate the block that sets initial_height (the call to "$BINARY" status --node
"$NODE" --home "$BITSONG_HOME" piped to jq) and either delete the initial_height
variable entirely or log it (e.g., echo or processLogger-style message) when
waiting for the relay so you can report the block range scanned; ensure any
downstream logic that referenced initial_height is updated accordingly.
- Around line 354-360: The script currently allows EVM_ONLY and COSMOS_ONLY to
both be true which silently skips all tests; add a mutual-exclusion check after
flag parsing that detects if both EVM_ONLY and COSMOS_ONLY are set and then
prints a clear error (e.g., "Cannot set both --evm-only and --cosmos-only") and
exits non-zero, or alternatively treat both-as-run-both by changing logic—modify
the flag handling around EVM_ONLY/COSMOS_ONLY so you either (A) validate and
exit when both variables are true, or (B) change the test invocation conditions
(test_ft_cosmos_to_evm and test_ft_evm_to_cosmos) to run when either flag is
unset rather than skip both; implement option A (exit on both) by adding an if
[[ "$EVM_ONLY" == "true" && "$COSMOS_ONLY" == "true" ]]; then echo "error..."
>&2; exit 1; fi immediately after flag parsing.
In `@scripts/hyperlane/lib.sh`:
- Around line 241-264: The Python fallback in bech32_to_bytes32 interpolates
$bech32_addr directly into a single-quoted Python string, enabling shell/Python
injection; instead pass the address via an environment variable and read it
inside Python to avoid interpolation. Update the call so the address is passed
as BECH32_ADDR="$bech32_addr" to python3 and change the Python snippet in the
bech32_to_bytes32 function to import os and use os.environ['BECH32_ADDR']
(remove the direct '$bech32_addr' literal), preserving the same decoding logic
and exit/status handling.
In `@scripts/hyperlane/run-all.sh`:
- Around line 5-6: Update the stale usage comment in run-all.sh: change the line
reading "# bash run-all.sh # Run phases 1-5" to reflect that
the script runs phases 1-7 (or list the actual phase range/run behavior). Edit
the top-of-file usage block in run-all.sh so the comment accurately documents
the executed phases (e.g., "# Run phases 1-7" or an explicit brief description
of phases).
- Around line 20-43: The --from case handling must guard against missing or
non-numeric values for FROM_PHASE to avoid unbound-variable and failed numeric
comparisons: in the case branch that sets FROM_PHASE, first ensure $2 is present
and non-empty, then validate it matches a positive-integer pattern (e.g., regex
^[0-9]+$) before assigning FROM_PHASE="$2" and shifting; on failure print a
clear usage/error and exit non-zero. Update the parsing logic around the
FROM_PHASE variable and ensure subsequent numeric comparisons (the -le check
later) operate on a guaranteed numeric value.
In `@scripts/hyperlane/stop.sh`:
- Around line 14-28: The script currently allows both CHAIN_ONLY and AGENTS_ONLY
to be true which results in no services being stopped; after argument parsing
check the combination and reject it: if both CHAIN_ONLY and AGENTS_ONLY are
true, print a clear error message and exit non‑zero. Reference the existing flag
variables CHAIN_ONLY and AGENTS_ONLY and the agent-stop and chain-stop blocks so
the check runs before those blocks execute; this enforces mutual exclusivity
rather than silently doing nothing.
---
Nitpick comments:
In @.github/workflows/push-docker-image.yml:
- Around line 73-75: The three echo lines that append IMAGE_TAGS to the GHA
output use an unquoted $GITHUB_OUTPUT and multiple redirects; change them to a
single grouped append that redirects once to the quoted "$GITHUB_OUTPUT" and
echo the IMAGE_TAGS content (use the IMAGE_TAGS variable quoted as "$IMAGE_TAGS"
inside the group) so the path is quoted (satisfying SC2086) and you avoid
repeated redirects (addresses SC2129).
- Around line 28-29: The workflow currently hardcodes a branch filter value
'feat-hyperlane' under the branches key which will keep triggering builds after
merge; update the branches filter (in the push trigger section where branches:
['feat-hyperlane'] is defined) to either remove the branches filter entirely if
you want builds on all branches, replace the literal with a wildcard pattern
such as 'feat-*' to capture multiple feature branches, or parameterize it via a
repository secret or environment variable so it won't become stale after
merging.
In `@scripts/hyperlane/03-evm-deploy.sh`:
- Around line 61-116: deploy_hyp_erc20 and the token enrollment functions rely
on implicitly global variables (DEPLOYED_EVM, TOKEN_ID); make these locals
and/or use the existing save_state/load_state to avoid fragile globals: declare
DEPLOYED_EVM as local inside deploy_hyp_erc20 (it’s already saved with
save_state "evm_hyp_erc20"), declare TOKEN_ID local where it’s assigned, and
update enroll_cosmos_side and enroll_evm_side to read values via load_state
("evm_hyp_erc20" and the token state key) or accept them as parameters instead
of depending on globals; ensure references to evm_to_bytes32, save_state, and
load_state remain consistent.
In `@scripts/hyperlane/04-agents.sh`:
- Around line 444-453: The loop that runs jq once per key is inefficient;
replace it with a single jq invocation that deletes all seven keys at once to be
atomic and faster: check [[ -f "$STATE_FILE" ]] and then run jq with a del()
call that removes .validator_addr, .bitsong_multisig_ism_id,
.basesepolia_multisig_ism, .validator_bitsong_announced,
.validator_basesepolia_announced, .ism_updated_cosmos_side, and
.ism_updated_evm_side in one expression, writing to "${STATE_FILE}.tmp" and
mv-ing it back (preserve the existing temp-file atomic write pattern used
elsewhere).
- Around line 70-124: The embedded Python call in fund_cosmos_signer both omits
a pre-check for the ecdsa dependency and leaks COSMOS_SIGNER_KEY on the process
command line; add an upfront check (e.g., run a short python3 -c "import ecdsa"
check and fail with a clear log_err if it raises) and change the python
invocation that derives the bech32 address to read the private key from stdin or
an environment-safe source instead of interpolating $COSMOS_SIGNER_KEY into the
command string (update the python3 call used in fund_cosmos_signer and related
logging to reflect this safer input method and surface dependency failures
clearly).
In `@scripts/hyperlane/06-fantoken-route.sh`:
- Around line 608-617: The CLEAN branch currently rewrites STATE_FILE once per
suffix; instead build the list of suffixes (same set used now) and perform a
single jq invocation that deletes all "ft_${FT_KEY}_${suffix}" keys in one pass
(use jq del with an array or chained del() calls generated from the suffix list)
and then mv the single temporary file back to STATE_FILE; keep the surrounding
log calls and the unregister_route call and reference CLEAN, FT_KEY, STATE_FILE
and unregister_route so you only perform one jq | mv instead of N sequential
rewrites.
- Around line 86-95: unregister_route currently uses grep -v with an unescaped
regex ("grep -v \"^${FT_KEY}$\"") so metas in FT_KEY can match unexpectedly;
update the pipeline in unregister_route (which calls load_state and save_state)
to use fixed-string exact-line matching (e.g., replace the grep invocation with
grep -Fxv or equivalent) so the removal compares FT_KEY as a literal exact line
and won’t treat characters like . or * as regex metacharacters.
- Around line 465-481: The script exposes private keys by passing
COSMOS_SIGNER_KEY and EVM_RELAYER_KEY directly on the docker run command line
(used for --chains.bitsong.signer.key and --chains.basesepolia.signer.key);
change this to load secrets from a safer source such as Docker secrets or
mounted files and reference those inside the container (e.g., mount
$BITSONG_HOME/cosmos_key:/run/secrets/cosmos_key:ro and use the file path inside
the relayer config or set an env var reading that file), or at minimum export
them as environment variables via -e (noting they remain visible in docker
inspect) and update the CLI flags to read from the secret file paths or env vars
rather than embedding the raw keys on the command line.
- Around line 182-212: The build_whitelist function currently constructs JSON by
concatenating strings which is fragile; change it to build the array using jq so
items are appended as real JSON objects. Replace the manual string assembly in
build_whitelist (including the blocks that add ubtsg using
token_id/evm_hyp_erc20 and the loop that adds each fantoken using
ft_${sym}_token_id and ft_${sym}_evm_hyp_erc20) with calls that pipe object
literals to jq --arg to set senderAddress and destinationDomain and use jq -s to
produce the final array; keep using evm_to_bytes32(token) and load_state(...) to
read values but pass them via jq arguments so escaping is handled and you never
manually trim trailing commas.
- Around line 57-60: ft_save and ft_load currently assume FT_KEY is set and will
silently use "ft__<suffix>" if empty; add a guard in both helper functions
(ft_save and ft_load) that asserts FT_KEY is non-empty before calling
save_state/load_state, and fail fast with an explanatory error message (and
non-zero exit) if FT_KEY is empty so you don't read/write the wrong state key.
In `@scripts/hyperlane/07-fantoken-test.sh`:
- Around line 117-121: The inline bytes32 padding duplicates existing helper
evm_to_bytes32; replace the manual conversion of evm_signer_addr ->
evm_signer_bytes32 (the hex trimming/lowercasing and printf padding) with a call
to evm_to_bytes32 from lib.sh, e.g. evm_signer_bytes32=$(evm_to_bytes32
"$evm_signer_addr"), keeping the preceding derivation and error check for
evm_signer_addr intact and ensuring the resulting evm_signer_bytes32 variable is
used as before.
- Around line 214-221: The script wrongly uses totalSupply() to infer the
signer's token holdings; update the check to call balanceOf(<signer_address>)
instead. Replace the cast call on ft_evm_hyp_erc20 from "totalSupply()(uint256)"
to "balanceOf(address)(uint256)" and pass the runtime signer address variable
used elsewhere in the script (e.g., the EVM signer variable like EVM_SIGNER or
EVM_FROM) while keeping EVM_RPC; store the result back into evm_balance and keep
the existing error logging path if the balance is "0". Ensure the balanceOf call
targets the same contract variable ft_evm_hyp_erc20.
In `@scripts/hyperlane/lib.sh`:
- Around line 119-129: The save_state function is vulnerable to concurrent
writes; wrap its read-modify-write against STATE_FILE with an exclusive flock to
prevent races: acquire an exclusive lock on STATE_FILE (or a lockfd derived from
STATE_FILE) before reading, running jq to update into a temp file, and mv the
temp into STATE_FILE, then release the lock; also apply a shared flock in
load_state when reading STATE_FILE to avoid torn reads. Update the save_state
and load_state functions to use flock (fall back to a safe error if flock is
unavailable) while keeping the existing behavior of creating STATE_FILE if
missing and preserving atomic mv.
In `@scripts/hyperlane/run-all.sh`:
- Around line 58-77: The script currently uses a string CLEAN_FLAG and an
unquoted expansion which relies on word-splitting; replace this with a shell
array (e.g. CLEAN_ARGS) that is empty by default and set to contain --clean when
CLEAN is true, then pass that array to run_phase invocations instead of
CLEAN_FLAG (update the calls that reference CLEAN_FLAG and the run_phase
invocations for 01-chain.sh, 03-evm-deploy.sh, 04-agents.sh,
06-fantoken-route.sh to expand the array), ensuring the array is expanded
properly so no spurious empty arguments are passed.
In `@scripts/hyperlane/status.sh`:
- Around line 34-43: The script uses GNU-only date parsing (date -d "$started")
inside the block handling status/started (variables status, started,
start_epoch), which fails on macOS; replace this with a portable approach:
detect GNU date vs BSD date (e.g., check if date -d works) and if unavailable
parse the timestamp using a macOS-compatible command (date -j -f or a small
portable fallback using python/perl) to compute start_epoch, preserving the rest
of the uptime calculation logic; ensure the change keeps the same variable names
(start_epoch, now_epoch, diff, uptime) and behavior for Linux while providing
the BSD/date-compatible branch for macOS.
Adds the hyperlane (x/core) and warp (x/warp) module stores and initializes them via RunMigrations. Materializes both module accounts at the upgrade height and removes duplicate init-genesis entries.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 117: Update the github.com/go-jose/go-jose/v4 dependency in go.mod from
v4.1.1 to v4.1.4 or higher to address the JWE decryption panic vulnerability.
Change the version number in the line containing "github.com/go-jose/go-jose/v4
v4.1.1 // indirect" to v4.1.4 or a later compatible version, then run "go mod
tidy" to update the go.sum file with the new dependency versions.
- Line 49: Update the google.golang.org/grpc dependency in the go.mod file from
version v1.75.0 to v1.79.3 or any newer stable version to address the
authorization bypass vulnerability (GHSA-p77j-4mvh-x3m3). After updating the
dependency version number for google.golang.org/grpc, run go mod tidy to ensure
all transitive dependencies are properly resolved. Additionally, verify that
authorization middleware is properly configured on all gRPC service definitions
throughout the codebase to prevent unauthorized access attempts.
- Line 20: The hyperlane-cosmos dependency at line 20 in go.mod transitively
pulls in a vulnerable version of go-ethereum (v1.14.12) that contains the RLPx
handshake vulnerability CVE-2026-26315. Add a replace directive in go.mod after
the existing module declarations to explicitly pin
github.com/ethereum/go-ethereum to version v1.16.9 or any v1.17.0 or later
version, which contains the security patch. After adding the replace directive
and running go mod tidy, you must also rotate the node key by deleting the
nodekey file from your data directory before restarting the service to prevent
potential private key exposure from the vulnerability.
- Around line 211-213: Update the versions of go.opentelemetry.io/otel and
go.opentelemetry.io/otel/sdk from v1.37.0 to v1.43.0 or later in the go.mod
file. This will address the HIGH-severity vulnerabilities including excessive
memory allocation via malformed baggage headers, arbitrary code execution via
PATH hijacking on BSD systems, and PATH hijacking vulnerabilities on
macOS/Darwin. Change both the go.opentelemetry.io/otel and
go.opentelemetry.io/otel/sdk module versions to the patched release to ensure
all security issues are resolved before production deployment.
- Line 191: The shamaton/msgpack/v2 dependency at version v2.2.2 contains a
denial-of-service vulnerability related to buffer validation for truncated
fixext data. Update the github.com/shamaton/msgpack/v2 dependency from v2.2.2 to
v2.4.1 or later in the go.mod file to fix the vulnerability. After updating the
version string, run go mod tidy to ensure the dependency tree is properly
resolved and validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f614696a-4d2b-4700-98b8-d872ed1c2bca
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
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
1 participant
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.
Summary by CodeRabbit
.env.exampleto help bootstrap required environment variables..gitignorefor local Hyperlane/dev artifacts and build outputs.