YNU-140: rename infura_url var to blockchain_rpc#354
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRenames Infura-related env vars and struct field to BlockchainRPC across code, tests, charts, docs, and docker-compose. Adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Clearnode App
participant CFG as LoadConfig
participant Env as Env Vars
participant Net as NetworkConfig
participant Cust as NewCustody
participant ETH as ethclient
App->>CFG: LoadConfig()
CFG->>Env: Read {PREFIX}_BLOCKCHAIN_RPC, {PREFIX}_BLOCK_STEP
Note right of CFG #d6f5d6: BLOCK_STEP defaults to 10000
CFG-->>App: NetworkConfig (BlockchainRPC, BlockStep, ...)
App->>Cust: NewCustody(..., BlockchainRPC, BlockStep, ...)
Cust->>ETH: Dial(BlockchainRPC)
ETH-->>Cust: eth client
Cust-->>App: Custody instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
Summary of ChangesHello @dimast-x, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the codebase by standardizing the naming convention for blockchain RPC endpoint variables and configuration keys. The change replaces specific references to 'Infura URL' with the more generic 'Blockchain RPC' to enhance clarity and allow for broader compatibility with various blockchain providers beyond Infura. This improves the maintainability and flexibility of the system's network configuration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request successfully renames infura_url and related variables to the more generic blockchain_rpc across the codebase, which is a good improvement for clarity and provider agnosticism. My review focuses on a few minor inconsistencies in comments and configuration files where the old naming convention or provider-specific terms still linger. Addressing these will help fully align the code with the goal of this refactoring.
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)
clearnode/main.go (1)
88-95: Bug: variable shadowing empties the worker clients map.You redeclare custodyClients inside the block, iterate over the new empty map, and pass an empty set to the worker.
Apply this fix:
- if len(custodyClients) > 0 { - custodyClients := make(map[uint32]CustodyInterface, len(custodyClients)) - for chainID, client := range custodyClients { - custodyClients[chainID] = client - } - worker := NewBlockchainWorker(db, custodyClients, logger) + if len(custodyClients) > 0 { + workerClients := make(map[uint32]CustodyInterface, len(custodyClients)) + for chainID, client := range custodyClients { + workerClients[chainID] = client + } + worker := NewBlockchainWorker(db, workerClients, logger) go worker.Start(context.Background()) }
🧹 Nitpick comments (5)
clearnode/README.md (1)
141-148: Update leftover INFURA_URL mention to BLOCKCHAIN_RPC.The surrounding doc uses BLOCKCHAIN_RPC; this line still says INFURA_URL.
Apply this doc fix:
- Multiple networks can be added. For each supported network (POLYGON, ETH_SEPOLIA, CELO, BASE, WORLD_CHAIN, LOCALNET), you can specify the corresponding INFURA_URL, CUSTODY_CONTRACT_ADDRESS, ADJUDICATOR_ADDRESS, and BALANCE_CHECKER_ADDRESS environment variables. + Multiple networks can be added. For each supported network (POLYGON, ETH_SEPOLIA, CELO, BASE, WORLD_CHAIN, LOCALNET), you can specify the corresponding BLOCKCHAIN_RPC, CUSTODY_CONTRACT_ADDRESS, ADJUDICATOR_ADDRESS, and BALANCE_CHECKER_ADDRESS environment variables.clearnode/config.go (3)
14-15: Update comment to be provider‑agnosticReplace “Infura endpoint URL” with “RPC endpoint URL”.
Apply this diff:
-// - {PREFIX}_BLOCKCHAIN_RPC: The Infura endpoint URL for the network +// - {PREFIX}_BLOCKCHAIN_RPC: The RPC endpoint URL for the network
124-126: *Add legacy fallback for _INFURA_URL to avoid breaking existing envsSupport old envs during the transition (prefer BLOCKCHAIN_RPC when both exist).
Apply this diff:
- if strings.HasPrefix(key, network+"_BLOCKCHAIN_RPC") { - blockchainRPC = value + if strings.HasPrefix(key, network+"_BLOCKCHAIN_RPC") { + blockchainRPC = value + } else if strings.HasPrefix(key, network+"_INFURA_URL") && blockchainRPC == "" { + // Backward compatibility with legacy env name + blockchainRPC = value
142-152: Clarify validation and messageYou’re checking four required fields (RPC, custody, adjudicator, balance checker). Consider updating the nearby comment (“both required variables”) for accuracy. Otherwise, the guard and assignment look correct.
clearnode/custody.go (1)
50-52: Validate node chain ID against configured chain (or remove the param)The
chainparameter isn’t used; you derive chain ID from the node. Either removechainor verify it matches to prevent misconfiguration.Apply this diff (add mismatch check after obtaining
chainID):func NewCustody(signer *Signer, db *gorm.DB, wsNotifier *WSNotifier, blockchainRPC, custodyAddressStr, adjudicatorAddr, balanceCheckerAddr string, chain uint32, blockStep uint64, logger Logger) (*Custody, error) { client, err := ethclient.Dial(blockchainRPC) if err != nil { return nil, fmt.Errorf("failed to connect to blockchain node: %w", err) } chainID, err := client.ChainID(context.Background()) if err != nil { return nil, fmt.Errorf("failed to get chain ID: %w", err) } + if chain != 0 && chain != uint32(chainID.Int64()) { + return nil, fmt.Errorf("chain ID mismatch: config=%d node=%s", chain, chainID.String()) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
clearnode/README.md(2 hunks)clearnode/channel_service_test.go(1 hunks)clearnode/chart/config/prod/secrets.yaml(1 hunks)clearnode/chart/config/sandbox/secrets.yaml(1 hunks)clearnode/chart/config/uat/secrets.yaml(1 hunks)clearnode/config.go(5 hunks)clearnode/custody.go(1 hunks)clearnode/eth_listener_test.go(1 hunks)clearnode/main.go(1 hunks)clearnode/reconcile_cli.go(2 hunks)clearnode/rpc_router_public_test.go(1 hunks)clearnode/rpc_router_test.go(1 hunks)docker-compose.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
clearnode/main.go (1)
clearnode/custody.go (1)
NewCustody(50-96)
clearnode/custody.go (4)
clearnode/nitrolite/bindings.go (2)
NewCustody(147-153)Custody(88-92)clearnode/signer.go (1)
Signer(24-26)clearnode/notification.go (1)
WSNotifier(11-14)clearnode/log.go (1)
Logger(12-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and Publish (Clearnode)
- GitHub Check: Test (Integration) / Test Integration
- GitHub Check: Analyze (go)
🔇 Additional comments (12)
clearnode/README.md (1)
154-161: Example aligns with rename.Example env now uses POLYGON_BLOCKCHAIN_RPC and looks consistent with the code changes.
clearnode/chart/config/uat/secrets.yaml (1)
6-7: UAT secret key rename is consistent.Keys switched to *_BLOCKCHAIN_RPC while keeping same secret refs. Looks good.
clearnode/main.go (1)
76-78: LGTM: uses network.BlockchainRPC in NewCustody.Matches the config field rename and custody constructor.
clearnode/eth_listener_test.go (1)
16-20: LGTM: test now uses blockchainRPC.Rename is consistent with the rest of the codebase.
clearnode/rpc_router_test.go (1)
118-126: LGTM: NetworkConfig literals updated to BlockchainRPC.Field rename applied correctly in tests.
clearnode/reconcile_cli.go (2)
45-49: LGTM: dialing via network.BlockchainRPC.Consistent with the field rename.
60-71: LGTM: NewCustody wired with BlockchainRPC and BlockStep.Constructor call matches the new signature.
docker-compose.yml (1)
150-151: ANVIL_INFURA_URL not found — INFURA strings remain in tests/chartsNo exact "ANVIL_INFURA_URL" occurrences found. INFURA is still referenced in these files:
- clearnode/rpc_router_test.go
- clearnode/rpc_router_public_test.go
- clearnode/channel_service_test.go
- clearnode/config.go (comment)
- clearnode/chart/config/sandbox/secrets.yaml
- clearnode/chart/config/prod/secrets.yaml
- clearnode/chart/config/uat/secrets.yaml
- clearnode/README.md
docker-compose.yml contains ANVIL_BLOCKCHAIN_RPC: ws://anvil:8545
clearnode/chart/config/prod/secrets.yaml (1)
6-11: *Prod secret key renames OK — no _INFURA_URL consumers foundRepo-wide searches found no environment variables matching "*_INFURA_URL"; Helm charts/configs use *_BLOCKCHAIN_RPC as expected. README and tests still mention "Infura" or include example Infura URLs (informational only).
clearnode/rpc_router_public_test.go (1)
54-56: LGTM: test config uses BlockchainRPCField rename in NetworkConfig literals is correct and aligns with the refactor.
clearnode/channel_service_test.go (1)
83-83: LGTM: NetworkConfig literal updated to BlockchainRPCMatches the renamed field; no behavioral change in tests.
clearnode/config.go (1)
37-42: LGTM: Added BlockchainRPC and BlockStepThe struct changes look good and match the PR objective. Defaulting BlockStep to 10000 is sensible.
|
lets merge it after #346 |
27fa551 to
f145a6a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
clearnode/chart/config/prod/secrets.yaml (1)
6-11: Optional: add transitional aliases for one deploy.If there’s any risk of missed references, you can temporarily keep old keys pointing to the same secrets:
POLYGON_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-polygon-infura-url?version=latest + POLYGON_INFURA_URL: ref+gcpsecrets://ynet-stage/clearnet-prod-polygon-infura-url?version=latest WORLD_CHAIN_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-worldchain-infura-url?version=latest + WORLD_CHAIN_INFURA_URL: ref+gcpsecrets://ynet-stage/clearnet-prod-worldchain-infura-url?version=latest FLOW_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-flow-infura-url?version=latest + FLOW_INFURA_URL: ref+gcpsecrets://ynet-stage/clearnet-prod-flow-infura-url?version=latest BASE_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-base-infura-url?version=latest + BASE_INFURA_URL: ref+gcpsecrets://ynet-stage/clearnet-prod-base-infura-url?version=latest ETH_MAINNET_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-eth-mainnet-infura-url?version=latest + ETH_MAINNET_INFURA_URL: ref+gcpsecrets://ynet-stage/clearnet-prod-eth-mainnet-infura-url?version=latest LINEA_MAINNET_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-linea-mainnet-infura-url?version=latest + LINEA_MAINNET_INFURA_URL: ref+gcpsecrets://ynet-stage/clearnet-prod-linea-mainnet-infura-url?version=latestRemove the aliases in the next release.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
clearnode/README.md(2 hunks)clearnode/channel_service_test.go(1 hunks)clearnode/chart/config/prod/secrets.yaml(1 hunks)clearnode/chart/config/sandbox/secrets.yaml(1 hunks)clearnode/chart/config/uat/secrets.yaml(1 hunks)clearnode/config.go(5 hunks)clearnode/custody.go(1 hunks)clearnode/eth_listener_test.go(1 hunks)clearnode/main.go(1 hunks)clearnode/reconcile_cli.go(2 hunks)clearnode/rpc_router_public_test.go(1 hunks)clearnode/rpc_router_test.go(1 hunks)docker-compose.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- docker-compose.yml
- clearnode/channel_service_test.go
- clearnode/custody.go
- clearnode/rpc_router_public_test.go
- clearnode/chart/config/sandbox/secrets.yaml
- clearnode/main.go
- clearnode/chart/config/uat/secrets.yaml
- clearnode/rpc_router_test.go
- clearnode/eth_listener_test.go
- clearnode/config.go
- clearnode/README.md
- clearnode/reconcile_cli.go
🔇 Additional comments (3)
clearnode/chart/config/prod/secrets.yaml (3)
6-11: Rename looks good.Env var keys updated to *_BLOCKCHAIN_RPC consistently. No YAML issues spotted.
6-11: Align GCP secret names with new convention (follow-up).Underlying GCP Secret Manager names still use "-infura-url". For long-term consistency and clarity, consider creating new secrets with "-blockchain-rpc" and updating refs.
Example diff once new secrets exist:
- POLYGON_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-polygon-infura-url?version=latest + POLYGON_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-polygon-blockchain-rpc?version=latest - WORLD_CHAIN_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-worldchain-infura-url?version=latest + WORLD_CHAIN_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-worldchain-blockchain-rpc?version=latest - FLOW_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-flow-infura-url?version=latest + FLOW_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-flow-blockchain-rpc?version=latest - BASE_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-base-infura-url?version=latest + BASE_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-base-blockchain-rpc?version=latest - ETH_MAINNET_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-eth-mainnet-infura-url?version=latest + ETH_MAINNET_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-eth-mainnet-blockchain-rpc?version=latest - LINEA_MAINNET_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-linea-mainnet-infura-url?version=latest + LINEA_MAINNET_BLOCKCHAIN_RPC: ref+gcpsecrets://ynet-stage/clearnet-prod-linea-mainnet-blockchain-rpc?version=latest
6-11: Sanity-check: leftover INFURA_URL usageclearnode/chart/config/prod/secrets.yaml still references GCP secret names containing “-infura-url” for several *BLOCKCHAIN_RPC entries; my automated rg run didn’t scan files (ripgrep reported “No files were searched”). Run a repo-wide search for /\b[A-Z0-9]*INFURA_URL\b/ (e.g. rg -nP --hidden -uu '\b[A-Z0-9_]*INFURA_URL\b') and confirm there are no stale INFURA_URL env keys remaining.
f145a6a to
e10ee25
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
clearnode/config.go (3)
39-44: Typo in field name BalanceCHeckerAddressConsider correcting to BalanceCheckerAddress in a follow‑up to avoid confusion.
110-116: Default block stepDefaulting to 10000 is fine. Consider extracting to a const for discoverability (non‑blocking).
126-141: Match exact env keys instead of HasPrefixPrevents accidental matches like FOO_BLOCKCHAIN_RPC_BACKUP overriding values.
- if strings.HasPrefix(key, network+"_BLOCKCHAIN_RPC") { + if key == network+"_BLOCKCHAIN_RPC" { blockchainRPC = value - } else if strings.HasPrefix(key, network+"_CUSTODY_CONTRACT_ADDRESS") { + } else if key == network+"_CUSTODY_CONTRACT_ADDRESS" { custodyAddress = value - } else if strings.HasPrefix(key, network+"_ADJUDICATOR_ADDRESS") { + } else if key == network+"_ADJUDICATOR_ADDRESS" { adjudicatorAddress = value - } else if strings.HasPrefix(key, network+"_BALANCE_CHECKER_ADDRESS") { + } else if key == network+"_BALANCE_CHECKER_ADDRESS" { balanceCheckerAddress = value - } else if strings.HasPrefix(key, network+"_BLOCK_STEP") { + } else if key == network+"_BLOCK_STEP" { if step, err := strconv.ParseUint(value, 10, 64); err == nil && step > 0 { blockStep = step } else { logger.Warn("Invalid BLOCK_STEP value", "network", network, "value", value) } }clearnode/custody.go (1)
49-56: Use DialContext with timeout to avoid startup hangs on bad RPC URLsPrevents indefinite blocking if the endpoint is unreachable; this is an internal change to NewCustody (no caller updates required). Add context and time imports.
-// NewCustody initializes the Ethereum client and custody contract wrapper. -func NewCustody(signer *Signer, db *gorm.DB, wsNotifier *WSNotifier, blockchainRPC, custodyAddressStr, adjudicatorAddr, balanceCheckerAddr string, chain uint32, blockStep uint64, logger Logger) (*Custody, error) { - client, err := ethclient.Dial(blockchainRPC) +// NewCustody initializes the Ethereum client and custody contract wrapper. +func NewCustody(signer *Signer, db *gorm.DB, wsNotifier *WSNotifier, blockchainRPC, custodyAddressStr, adjudicatorAddr, balanceCheckerAddr string, chain uint32, blockStep uint64, logger Logger) (*Custody, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + client, err := ethclient.DialContext(ctx, blockchainRPC) if err != nil { return nil, fmt.Errorf("failed to connect to blockchain node: %w", err) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
clearnode/README.md(2 hunks)clearnode/channel_service_test.go(1 hunks)clearnode/chart/config/prod/secrets.yaml(1 hunks)clearnode/chart/config/sandbox/secrets.yaml(1 hunks)clearnode/chart/config/uat/secrets.yaml(1 hunks)clearnode/config.go(5 hunks)clearnode/custody.go(1 hunks)clearnode/eth_listener_test.go(1 hunks)clearnode/main.go(1 hunks)clearnode/reconcile_cli.go(2 hunks)clearnode/rpc_router_public_test.go(1 hunks)clearnode/rpc_router_test.go(1 hunks)docker-compose.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- clearnode/chart/config/uat/secrets.yaml
- clearnode/main.go
- clearnode/chart/config/sandbox/secrets.yaml
- clearnode/reconcile_cli.go
- clearnode/rpc_router_test.go
- clearnode/chart/config/prod/secrets.yaml
- clearnode/README.md
- clearnode/rpc_router_public_test.go
- clearnode/eth_listener_test.go
- clearnode/channel_service_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
clearnode/custody.go (4)
clearnode/nitrolite/bindings.go (2)
NewCustody(147-153)Custody(88-92)clearnode/signer.go (1)
Signer(24-26)clearnode/notification.go (1)
WSNotifier(11-14)clearnode/log.go (1)
Logger(12-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and Publish (Clearnode)
- GitHub Check: Test (Integration) / Test Integration
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
clearnode/config.go (2)
14-14: Doc now provider‑agnostic (BLOCKCHAIN_RPC) — resolved
143-155: Back‑compat: accept deprecated {PREFIX}_INFURA_URL as a fallbackSmooths migration for existing deployments.
- // Only add network if both required variables are present + // Backwards‑compat: accept deprecated INFURA_URL if BLOCKCHAIN_RPC is not set + if blockchainRPC == "" { + if v := os.Getenv(network + "_INFURA_URL"); v != "" { + blockchainRPC = v + logger.Warn("Using deprecated env var; migrate to BLOCKCHAIN_RPC", "env", network+"_INFURA_URL") + } + } + // Only add network if both required variables are present if blockchainRPC != "" && custodyAddress != "" && adjudicatorAddress != "" && balanceCheckerAddress != "" { networkLower := strings.ToLower(network) config.networks[chainID] = &NetworkConfig{ Name: networkLower, ChainID: chainID, BlockchainRPC: blockchainRPC, CustodyAddress: custodyAddress, AdjudicatorAddress: adjudicatorAddress, BalanceCHeckerAddress: balanceCheckerAddress, BlockStep: blockStep, } }rg search returned no matches for INFURA_URL — confirm CI/deploy/templates/docs for any remaining deprecated env usages before merging.
nksazonov
left a comment
There was a problem hiding this comment.
Generally LGTM with some nitpicks
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clearnode/config.go (2)
42-47: Typo breaks public API: BalanceCHeckerAddress should be BalanceCheckerAddress.
This likely causes compile errors where the field is referenced.Apply this diff:
type NetworkConfig struct { Name string ChainID uint32 BlockchainRPC string CustodyAddress string AdjudicatorAddress string - BalanceCHeckerAddress string // TODO: add balance checker method into our smart contract + BalanceCheckerAddress string // TODO: add balance checker method into our smart contract BlockStep uint64 }
147-158: Field name assignment must match the fixed struct field.
Update the struct literal to use BalanceCheckerAddress.Apply this diff:
config.networks[chainID] = &NetworkConfig{ Name: networkLower, ChainID: chainID, BlockchainRPC: blockchainRPC, CustodyAddress: custodyAddress, AdjudicatorAddress: adjudicatorAddress, - BalanceCHeckerAddress: balanceCheckerAddress, + BalanceCheckerAddress: balanceCheckerAddress, BlockStep: blockStep, }
🧹 Nitpick comments (3)
clearnode/config.go (3)
129-143: Use exact key matches instead of HasPrefix when reading env vars.
Prefix matching can unintentionally pick up similarly prefixed keys; exact matches are safer.Apply this diff:
- if strings.HasPrefix(key, network+"_BLOCKCHAIN_RPC") { + if key == network+"_BLOCKCHAIN_RPC" { blockchainRPC = value - } else if strings.HasPrefix(key, network+"_CUSTODY_CONTRACT_ADDRESS") { + } else if key == network+"_CUSTODY_CONTRACT_ADDRESS" { custodyAddress = value - } else if strings.HasPrefix(key, network+"_ADJUDICATOR_ADDRESS") { + } else if key == network+"_ADJUDICATOR_ADDRESS" { adjudicatorAddress = value - } else if strings.HasPrefix(key, network+"_BALANCE_CHECKER_ADDRESS") { + } else if key == network+"_BALANCE_CHECKER_ADDRESS" { balanceCheckerAddress = value - } else if strings.HasPrefix(key, network+"_BLOCK_STEP") { + } else if key == network+"_BLOCK_STEP" { if step, err := strconv.ParseUint(value, 10, 64); err == nil && step > 0 { blockStep = step } else { logger.Warn("Invalid BLOCK_STEP value", "network", network, "value", value) } }
147-148: Optional: log when a network is skipped due to missing vars.
Helps diagnose misconfigurations in envs.Example:
} else { logger.Debug("skipping network due to missing config", "network", network, "rpc", blockchainRPC != "", "custody", custodyAddress != "", "adjudicator", adjudicatorAddress != "", "balanceChecker", balanceCheckerAddress != "") }
1-163: Fix typo 'BalanceCHeckerAddress' and confirm Infura references are intentional
- Rename BalanceCHeckerAddress → BalanceCheckerAddress and update all usages (found in clearnode/config.go, clearnode/main.go, clearnode/reconcile_cli.go and related assignments).
- Infura endpoints appear only in tests/docs (clearnode/rpc_router_test.go, clearnode/rpc_router_public_test.go, clearnode/channel_service_test.go, clearnode/README.md); keep for tests/docs or replace/remove before shipping credentials.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
clearnode/README.md(2 hunks)clearnode/chart/config/prod/clearnode.yaml(1 hunks)clearnode/chart/config/prod/secrets.yaml(1 hunks)clearnode/chart/config/sandbox/clearnode.yaml(1 hunks)clearnode/chart/config/sandbox/secrets.yaml(1 hunks)clearnode/chart/config/uat/clearnode.yaml(1 hunks)clearnode/chart/config/uat/secrets.yaml(1 hunks)clearnode/config.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- clearnode/README.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and Publish (Clearnode)
- GitHub Check: Test (Integration) / Test Integration
- GitHub Check: Analyze (go)
🔇 Additional comments (8)
clearnode/chart/config/uat/secrets.yaml (1)
6-7: Secret paths: consider standardizing “eth-sepolia” → “ethereum-sepolia” for naming consistency.
Env key uses ETHEREUM_SEPOLIA_* while the GCP secret path keeps “eth-sepolia”. Not a blocker, but aligning names reduces confusion.If you plan to align naming, confirm the new secret names exist before changing charts.
clearnode/chart/config/uat/clearnode.yaml (1)
16-18: LGTM on ETHEREUM_ rename.*
Matches the new convention.clearnode/config.go (2)
14-14: Provider‑agnostic wording LGTM.
Comment now refers to generic blockchain RPC. Good.
17-36: Known networks table looks consistent.
Chain IDs and prefixes align with the new naming scheme.clearnode/chart/config/sandbox/secrets.yaml (1)
6-10: *LGTM: secrets moved to _BLOCKCHAIN_RPC and paths updated.
Matches the new env naming; values stay provider-agnostic.clearnode/chart/config/sandbox/clearnode.yaml (1)
19-21: ETHEREUM_ rename LGTM — no remaining ETH_SEPOLIA_ occurrences found repo-wide.**clearnode/chart/config/prod/secrets.yaml (1)
6-12: Secrets switched to BLOCKCHAIN_RPC — confirm GCP secrets & remove Infura example.No INFURA_URL/infura-url variables remain in code; *_BLOCKCHAIN_RPC keys are present in docker-compose and chart configs (clearnode/chart/config/{prod,uat,sandbox}/secrets.yaml, docker-compose.yml, clearnode/config.go). clearnode/README.md still contains an Infura URL example (POLYGON_BLOCKCHAIN_RPC=https://polygon-mainnet.infura.io/v3/your_infura_key).
Action items:
- Verify these GCP secret names exist in ynet-stage (prod): clearnet-prod-polygon-blockchain-rpc, clearnet-prod-worldchain-blockchain-rpc, clearnet-prod-flow-blockchain-rpc, clearnet-prod-base-blockchain-rpc, clearnet-prod-eth-mainnet-blockchain-rpc, clearnet-prod-linea-mainnet-blockchain-rpc, clearnet-prod-xrpl-evm-mainnet-blockchain-rpc.
- Update/remove the Infura example in clearnode/README.md if it should no longer be referenced.
- Approve changes after GCP secret presence is confirmed.
clearnode/chart/config/prod/clearnode.yaml (1)
27-36: Renames validated — config.go reads the new ENV namesclearnode/config.go iterates knownNetworks (includes ETHEREUM, LINEA, XRPL_EVM) and matches prefixes network+"_CUSTODY_CONTRACT_ADDRESS", "_ADJUDICATOR_ADDRESS", "_BALANCE_CHECKER_ADDRESS" and "_BLOCK_STEP", so the envs in clearnode/chart/config/prod/clearnode.yaml will be recognized. Ensure the corresponding network+"_BLOCKCHAIN_RPC" is set if you expect the network to be registered (LoadConfig requires BLOCKCHAIN_RPC plus the three addresses to add a network).
Summary by CodeRabbit