Skip to content

YNU-424: fix label in the metrics & reconfigure blockchains and assets#404

Merged
philanton merged 4 commits intomainfrom
feat/off-chain-eth
Oct 31, 2025
Merged

YNU-424: fix label in the metrics & reconfigure blockchains and assets#404
philanton merged 4 commits intomainfrom
feat/off-chain-eth

Conversation

@philanton
Copy link
Contributor

@philanton philanton commented Oct 28, 2025

Summary by CodeRabbit

Release Notes

  • Chores
    • Removed debug logging configuration from production, sandbox, and UAT environments
    • Disabled Flow blockchain in production and adjusted its block processing parameters
    • Disabled xrpl_evm_testnet blockchain in sandbox
    • Disabled a specific token in sandbox configuration
    • Added Ethereum token support to sandbox configuration
    • Updated internal metrics labels for improved blockchain tracking

@philanton philanton requested a review from a team as a code owner October 28, 2025 12:41
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @philanton, 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 introduces a new configuration for Ethereum (ETH) within the sandbox environment's asset definitions. The primary goal is to enable the representation of ETH as an off-chain-only asset, facilitating testing or simulation scenarios that do not require interaction with a live blockchain.

Highlights

  • New Asset Addition: Added "Ethereum" (ETH) as an off-chain-only asset to the sandbox environment configuration.
  • Configuration Details: The new asset is configured with a blockchain_id of 0, a placeholder address, and 18 decimals, indicating its off-chain nature.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Configuration updates across multiple environments disable specific blockchain networks (flow in production, xrpl_evm_testnet in sandbox), remove debug logging environment variables from Helm chart deployments across prod/sandbox/uat, update Prometheus metrics label names from "network" to "blockchainID", and add Ethereum asset configuration.

Changes

Cohort / File(s) Summary
Helm Chart Environment Configuration
clearnode/chart/config/prod/clearnode.yaml, clearnode/chart/config/sandbox/clearnode.yaml, clearnode/chart/config/uat/clearnode.yaml
Removed CLEARNODE_LOG_LEVEL: "debug" environment variable from extraEnvs across all three environments, leaving log level controlled solely by logLevel config field
Blockchain Network Configuration
clearnode/chart/config/prod/blockchains.yaml, clearnode/chart/config/sandbox/blockchains.yaml
Added disabled: true to flow blockchain in prod and xrpl_evm_testnet in sandbox; adjusted block_step for flow from 499 to 50 in prod
Asset Configuration
clearnode/chart/config/sandbox/assets.yaml
Added disabled: true to token with blockchain_id 1449000 under ytest.USD; appended new Ethereum asset with symbol eth, blockchain_id 0, and address 0x0000000000000000000000000000000000000000
Prometheus Metrics
clearnode/metrics.go
Renamed gauge vector label names from "network" to "blockchainID" in three metrics: BrokerBalanceAvailable, BrokerChannelCount, and BrokerWalletBalance

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Metrics label rename (clearnode/metrics.go): Verify that all code paths populating these three gauge vectors have been updated to use blockchainID instead of network labels; check for any external dashboards or alerting rules depending on the old label names
  • Block step change: Confirm that reducing flow's block_step from 499 to 50 in production is intentional and doesn't affect block confirmation timing or synchronization

Possibly related PRs

  • YNU-122: support XRPL EVM network on clearnode #346: Introduces and configures XRPL EVM networks (chain IDs 1440000/1449000) and related asset/config entries; this PR disables the 1449000 token and xrpl_evm_testnet, potentially following configuration decisions from that PR

Suggested reviewers

  • nksazonov

Poem

🐰 Networks tucked away with disabled: true,
Debug logs fade to a softer blue,
From "network" to "blockchainID" we rename,
Ethereum joins the sandbox game!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title Check ✅ Passed The PR title "YNU-424: fix label in the metrics & reconfigure blockchains and assets" is directly related to the primary changes in the changeset. The title accurately reflects two main components: the metrics.go changes where label names were updated from "network" to "blockchainID" across three gauge vectors (BrokerBalanceAvailable, BrokerChannelCount, BrokerWalletBalance), and the configuration reconfigurations across multiple files including adding disabled flags and modifying block_step values in blockchains.yaml and adding disabled flags plus a new Ethereum asset in assets.yaml. While the title does not explicitly mention the removal of CLEARNODE_LOG_LEVEL environment variables from multiple clearnode.yaml files, this represents a smaller ancillary change compared to the primary focus on metrics label fixes and blockchain/asset reconfigurations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/off-chain-eth

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds an off-chain Ethereum asset to the sandbox configuration. My review identifies a critical issue: the token address provided for Ethereum is invalid as it contains non-hexadecimal characters. This will cause a failure during application startup when asset configurations are loaded. I've suggested replacing the invalid address with a conventional sentinel address used for native assets.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
clearnode/chart/config/sandbox/assets.yaml (1)

20-21: Inconsistent YAML formatting: name should be quoted for consistency.

The existing asset "ytest.USD" uses quotes around the name, but the new Ethereum entry does not. This creates style inconsistency in the configuration file.

-  - name: Ethereum
+  - name: "Ethereum"
     symbol: eth
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27289d3 and 9b4fb83.

📒 Files selected for processing (1)
  • clearnode/chart/config/sandbox/assets.yaml (1 hunks)
⏰ 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/chart/config/sandbox/assets.yaml (2)

21-21: Verify symbol naming pattern consistency.

The existing asset uses a dotted-lowercase pattern (ytest.usd), while the new entry uses plain lowercase (eth). Consider whether the symbol should follow the existing pattern for consistency (e.g., ethereum.eth or similar), or if a flatter naming convention is intentional for off-chain-only assets.


24-24: Clarify the placeholder address for off-chain asset.

The address 0x0ethethethethethethethethethethethetheth appears to be a placeholder rather than a valid hex address (it contains letters beyond 'f'). Since this is an off-chain-only asset, confirm that this placeholder is intentional. If it should represent a null/zero address instead, consider using the standard zero address format 0x0000000000000000000000000000000000000000.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/temp-push.yml (1)

32-35: Consider adding semantic version or "latest" tags alongside the short SHA.

The workflow currently tags images only with the short commit SHA (e.g., ghcr.io/repo/clearnode:abc1234). For improved discoverability and easier rollback, consider also tagging with:

  • A semantic version (if using releases)
  • latest tag (for the most recent build on this branch)
  • Branch name (e.g., feat-off-chain-eth)

This improves image management and deployment workflows.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eac23e4 and 3249f09.

📒 Files selected for processing (1)
  • .github/workflows/temp-push.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/temp-push.yml

19-19: the runner of "docker/login-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


30-30: the runner of "docker/build-push-action@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🔇 Additional comments (1)
.github/workflows/temp-push.yml (1)

1-9: Clarify the intended lifespan of this temporary workflow.

The workflow is explicitly named "Temp Push on PR branch" and is hardcoded to trigger only on the feat/off-chain-eth branch. Before merging:

  1. Confirm this workflow should be committed to the main branch (vs. being discarded after the PR).
  2. If it's intended to be permanent, rename to reflect its actual purpose and consider generalizing the branch trigger.
  3. If it's temporary for this feature branch, clarify the cleanup strategy post-merge.

This is relevant to the PR's build/deployment strategy and should be explicitly documented.

@philanton philanton changed the title feat: add off-chain-only eth to sandbox assets YNU-424: fix label in the metrics && reconfigure blockchains and assets Oct 31, 2025
@philanton philanton changed the title YNU-424: fix label in the metrics && reconfigure blockchains and assets YNU-424: fix label in the metrics & reconfigure blockchains and assets Oct 31, 2025
@philanton philanton merged commit c7fa386 into main Oct 31, 2025
13 checks passed
@philanton philanton deleted the feat/off-chain-eth branch October 31, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants