Skip to content

Conversation

@fmorency
Copy link
Contributor

This pull request introduces several changes, including dependency updates, code refactoring for improved error handling, and the addition of a new FeesCollector feature. Below is a summary of the most important changes grouped by theme.

Dependency Updates:

  • Added cosmossdk.io/math v1.5.3 to the go.mod file.
  • Removed unused dependencies github.com/davecgh/go-spew and github.com/pmezard/go-difflib from the go.mod file. [1] [2]

Code Refactoring:

  • Refactored error handling in constructors like NewDenomInfoCollector, NewExcludedSupplyCollector, and NewTokenCountCollector to use a more concise else if pattern for checking client.Conn. [1] [2] [3]
  • Updated the Collect method in ExcludedSupplyCollector to use the gRPC client's context (c.grpcClient.Ctx) for better context propagation.

New Feature:

  • Introduced a new FeesCollector in pkg/collectors/autodetect/manifestd/fees.go to monitor transaction fees locked in validators. This includes:
    • Initialization with error handling for gRPC client validation.
    • Methods for describing and collecting Prometheus metrics.
    • Support for querying validator rewards via gRPC and aggregating results.

@fmorency fmorency requested a review from Copilot June 11, 2025 13:53
@fmorency fmorency self-assigned this Jun 11, 2025
@fmorency fmorency requested a review from jgryffindor June 11, 2025 13:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors collector constructors for clearer error handling, updates dependencies, and adds a new FeesCollector to expose validator fee metrics via Prometheus.

  • Consolidate client nil/connection checks in existing collectors with else if patterns
  • Update gRPC context usage in ExcludedSupplyCollector for consistent propagation
  • Introduce FeesCollector feature with metric descriptions, concurrent gRPC queries, and Prometheus reporting

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/collectors/autodetect/manifestd/token_count.go Refactored constructor error checks into if … else if block
pkg/collectors/autodetect/manifestd/excluded_supply.go Switched from context.Background() to c.grpcClient.Ctx
pkg/collectors/autodetect/manifestd/denom_info.go Refactored constructor error checks into if … else if block
pkg/collectors/autodetect/manifestd/fees.go New FeesCollector with Prometheus metrics and concurrent gRPC
go.mod Added cosmossdk.io/math and removed unused dependencies
Comments suppressed due to low confidence (1)

pkg/collectors/autodetect/manifestd/fees.go:1

  • The new FeesCollector adds significant logic and error paths but lacks accompanying unit or integration tests. Please add tests covering successful fee aggregation and each error scenario.
//go:build manifest_node_exporter

Copy link

@jgryffindor jgryffindor left a comment

Choose a reason for hiding this comment

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

Tested ACK, looks good! This is out on a canary in testnet.

@fmorency fmorency merged commit 5603a43 into manifest-network:main Jun 12, 2025
3 checks passed
@fmorency fmorency deleted the val-fee-exporter branch June 12, 2025 12:50
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