Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add: set logger to price-feeder #265

Closed

Conversation

leonz789
Copy link
Contributor

@leonz789 leonz789 commented Dec 11, 2024

Description

tasks

  • update price-feeder dependency to next version
  • integrate logger from exocored to price-feeder
  • update local_node.sh to be compatible with the new price-feeder

Closes #XXX

Summary by CodeRabbit

  • New Features

    • Enhanced command operations with improved logging and fine-tuned state synchronization for a streamlined experience.
    • Refined local node setup with updated token configurations, adjusted start block, and new debugging options.
  • Chores

    • Upgraded dependency versions and directives to bolster security, performance, and compatibility.
    • Updated Go module proxy settings for improved dependency resolution during releases.

Copy link
Contributor

coderabbitai bot commented Dec 11, 2024

Walkthrough

The pull request updates the Exocore application at multiple levels. The command root now integrates a logger when starting the price feeder and sets explicit state synchronization parameters. Dependencies in the module file have been revised with several version updates and replace directives. Additionally, the local node setup script has been modified to adjust token feeder parameters, introduce new configuration fields, and update RPC as well as debugger settings for improved configuration management and initialization.

Changes

File(s) Summary of Changes
cmd/exocored/root.go Updated NewRootCmd by passing serverCtx.Logger to the price feeder and enhancing logging. Modified initAppConfig to explicitly set SnapshotInterval and SnapshotKeepRecent for state synchronization. Public function behaviors remain the same though internal logic is adjusted.
go.mod Upgraded dependency versions: price-feeder from v0.1.15 to v0.2.3, replaced go.uber.org/atomic with go.uber.org/zap v1.27.0, and updated several replace directives (Cosmos SDK, evmos, gin, goleveldb, and golang.org/x/exp) for compatibility and security fixes.
local_node.sh Modified token feeder parameter start_base_block from "20" to "15". Added a new nstid field in oracle_env_beaconchain.yaml. Updated oracle_feeder.yaml structure to define tokens with token and sources, revised the RPC address format, and introduced a debugger section for gRPC settings as part of local node configuration enhancements.
Makefile Changed the GOPROXY environment variable in the release-dry-run target from https://goproxy.cn,direct to https://proxy.golang.org,direct, altering the source for Go modules during the release dry run process.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as Command Line
    participant PreRun as startCmd.PreRunE
    participant Logger as serverCtx.Logger
    participant PF as PriceFeeder

    CLI->>PreRun: Execute start command
    PreRun->>PF: Invoke StartPriceFeeder(logger)
    PF->>Logger: Log operation events
Loading
sequenceDiagram
    participant Script as local_node.sh
    participant YAML as Config Files (oracle_env, oracle_feeder, genesis)
    participant Node as Exocore Node

    Script->>YAML: Update configuration (nstid, token structure, start_base_block)
    YAML-->>Script: Return updated config data
    Script->>Node: Initialize and start node with new parameters
    Node-->>Script: Node running with metrics and updated logging
Loading

Possibly related PRs

  • use official proxy for goreleaser #310: The changes in the main PR and the retrieved PR are both related to modifications in the Makefile, specifically updating the GOPROXY environment variable, indicating a direct connection at the code level.

Suggested reviewers

  • TimmyExogenous
  • MaxMustermann2

Poem

I'm a rabbit tapping keys today,
Hoping logs and configs lead the way.
With new fields and tokens bright,
Exocore hops into the light.
Dependencies tuned, the code will play 🐇✨
Rejoice in bytes and bouncy sway!
Cheers to change on this fine day!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e0cacd and 68bfcdf.

📒 Files selected for processing (1)
  • Makefile (0 hunks)
💤 Files with no reviewable changes (1)
  • Makefile
⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: test-unit-e2e
  • GitHub Check: test-unit-cover
  • GitHub Check: run-lint
  • GitHub Check: Analyze (go)
  • GitHub Check: goreleaser
  • GitHub Check: docker-build
  • GitHub Check: docker-localnet-build

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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

🧹 Outside diff range and nitpick comments (1)
cmd/exocored/root.go (1)

164-165: LGTM! Good enhancement to logging.

The addition of server context for structured logging is a good improvement. The logger is properly configured with the module name "price-feeder".

Consider adding log level configuration for the price-feeder module specifically, allowing for fine-grained control over logging verbosity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d63b34b and b78193a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • cmd/exocored/root.go (1 hunks)
  • go.mod (3 hunks)
  • local_node.sh (2 hunks)
  • proto/exocore/delegation/v1/query.proto (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • proto/exocore/delegation/v1/query.proto
🔇 Additional comments (4)
go.mod (2)

56-56: LGTM! Aligned with logging improvements.

The addition of go.uber.org/zap as an indirect dependency aligns with the logging enhancements in cmd/exocored/root.go.


10-10: Verify the price-feeder dependency version.

The price-feeder dependency has been updated to a development version (v0.0.0-20241211184332-72b02c56c68d) from a release version (v0.1.15). This could introduce instability.

local_node.sh (2)

131-131: Verify impact of reduced start_base_block.

The start_base_block parameter has been reduced from 20 to 15 blocks. While this speeds up the start of token feeders, ensure this provides sufficient blockchain stability.

Consider documenting the rationale for choosing 15 blocks as the starting point and any potential implications for oracle reliability.


234-235: LGTM! Well-structured oracle configuration.

The addition of the nstid field with a properly formatted Ethereum address enhances the oracle configuration.

@leonz789 leonz789 marked this pull request as draft January 13, 2025 18:18
@leonz789 leonz789 force-pushed the develop-oracle-refactorfeeder branch from b78193a to 1413a38 Compare February 19, 2025 14:39
@github-actions github-actions bot removed the C:Proto label Feb 19, 2025
@leonz789 leonz789 changed the title [WIP] add: set logger to price-feeder add: set logger to price-feeder Feb 19, 2025
@leonz789 leonz789 marked this pull request as ready for review February 19, 2025 14:40
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

🔭 Outside diff range comments (1)
cmd/exocored/root.go (1)

149-151: 💡 Verification agent

🧩 Analysis chain

Address the TODO comment regarding EVMOS dependency.

The comment indicates a temporary workaround for a version mismatch. This should be resolved once the price feeder is updated to use EVMOS v16.


🏁 Script executed:

#!/bin/bash
# Check the current EVMOS version in price-feeder
grep -r "github.com/evmos/evmos/" $(find . -name go.mod)

Length of output: 206


Update on Price Feeder and EVMOS Dependency Handling

  • We verified via the go.mod entries that the project now references EVMOS v16. In particular, the dependency is defined as a standard v16 (v16.0.0) with a replacement mapping to the Exocore fork at v16.0.3-0.20240828081344-d5cfcd34a812.
  • This confirms that the intended update toward EVMOS v16 is in place. However, the price feeder remains temporarily disabled per the TODO comment in cmd/exocored/root.go (lines 149–151).
  • Actionable Suggestions:
    • Revisit the temporary disablement of the price feeder. If the updated EVMOS dependency (via the Exocore fork) now resolves the version mismatch issue, re-enable the price feeder.
    • While you're at it, consider improving the error handling in the price feeder initialization to include more specific error messages. This will help in diagnosing any future issues related to dependency mismatches or initialization failures.
🧹 Nitpick comments (1)
cmd/exocored/root.go (1)

156-162: Enhance error message in panic recovery.

Consider adding more context to the error message to help with debugging.

-fmt.Println("price-feeder failed", err)
+fmt.Printf("price-feeder initialization failed: %v\n", err)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b78193a and 1413a38.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • cmd/exocored/root.go (1 hunks)
  • go.mod (3 hunks)
  • local_node.sh (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: test-unit-e2e
  • GitHub Check: Analyze (go)
🔇 Additional comments (7)
cmd/exocored/root.go (1)

164-165: LGTM! Enhanced logging for price feeder.

The addition of structured logging with module context improves observability.

go.mod (2)

270-286: LGTM! Security and compatibility improvements in replace directives.

The updates address security advisories and ensure compatibility with the latest versions.


11-11:

❓ Verification inconclusive

Verify compatibility with the new price-feeder version.

The update to price-feeder v0.2.3 and addition of zap logger align with the logging improvements.

Also applies to: 61-61


🏁 Script executed:

#!/bin/bash
# Check for breaking changes in price-feeder v0.2.3
curl -s https://api.github.com/repos/ExocoreNetwork/price-feeder/releases/tags/v0.2.3 | jq -r '.body'

Length of output: 106


Dependency and Compatibility Verification

  • The upgrade from price-feeder v0.1.15 to v0.2.3 aligns with the intended PR objectives. However, since fetching the release notes for v0.2.3 returned null, there are no documented breaking changes from that source. Please verify via the official documentation or through integration tests to ensure compatibility.
  • The addition of the zap logger dependency (line 61) and the updated replace directives (lines 270–286) support the required logging improvements and overall security adjustments.
local_node.sh (4)

130-130: Verify impact of reduced start_base_block.

The change from 20 to 15 blocks could affect token feeder synchronization. Please ensure this doesn't impact data consistency.


233-238: LGTM! Improved token configuration structure.

The new format with explicit token and sources fields improves readability.


244-248: LGTM! Enhanced debugging capabilities.

The addition of debugger configuration and explicit RPC/WebSocket settings improves operability.


260-261: LGTM! Added beacon chain identifier.

The nstid configuration is properly formatted and enables beacon chain integration.

@leonz789
Copy link
Contributor Author

go mod download works fine locally with either:
directly 'ExocoreNetwork/price-feeder', or
'replace to imua-xyz/price-feeder' set in go.mod

@bwhour
Copy link
Contributor

bwhour commented Feb 21, 2025

@leonz789 rebase & resolve the confict

@leonz789
Copy link
Contributor Author

supersed by #315

@leonz789 leonz789 closed this Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants