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

feat: implement validators commands #162

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

epsjunior
Copy link
Contributor

Summary

This PR introduces new features, bug fixes, and comprehensive tests for validator commands and the JSON RPC client. It also resolves a critical bug that prevented proper error handling in JSON RPC requests, causing related commands to fail silently.


Changes Introduced

New Features

  1. Validator Commands:

    • Added commands for managing validators, including create, update, delete, count, create-random, and get.
    • Integrated robust input validation and error handling.
  2. Enhanced JSON RPC Client:

    • Improved error handling to ensure exceptions are propagated and logged effectively.
    • Updated response parsing for better error detection and reporting.
  3. Simulator Service Improvements:

    • Fixed method names for createRandomValidators and deleteAllValidators to match the correct RPC API.
    • Enhanced handling of database cleanup errors to ensure visibility and proper logging.

Bug Fixes

  1. JSON RPC Client:

    • Resolved an issue where errors from RPC requests were not being propagated.
    • This fix addresses a long-standing issue likely introduced in a past commit.
  2. Simulator Service:

    • Corrected RPC method calls for random validator creation, resolving initialization failures.
    • Fixed container name prefix usage in Docker management functions.
  3. Validators Action:

    • Improved parameter validation and error handling for commands like create, update, and create-random.
    • Enhanced messaging for invalid inputs to provide clearer feedback to users.

Tests Added

  • Increased test coverage for all validator commands, including success and error scenarios.
  • Enhanced tests for the simulator service, ensuring robust error logging and handling.
  • Validated proper propagation of errors in the JSON RPC client to ensure no silent failures.

Notes

  • The JSON RPC error-handling bug had been present since September, leading to undetected failures in related methods. This PR resolves the issue and improves error visibility.
  • The updates bring the simulator service to 100% test coverage.

How to Test

  1. Run the updated test suite to validate the functionality of new features and bug fixes.
  2. Execute validator commands (create, update, delete, etc.) to ensure correct operation.
  3. Simulate JSON RPC errors to confirm proper logging and handling.

Related Links

  • Original bug introduction: Commit ebdeeef
  • This PR fixes the issue and enhances error detection for JSON RPC requests.

@epsjunior epsjunior linked an issue Jan 5, 2025 that may be closed by this pull request
…-released' into 59-implement-validators-commands
@epsjunior epsjunior requested a review from cristiam86 January 5, 2025 20:59
}
}

private async confirmPrompt(message: string): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@epsjunior this seems like a generic function that could be part of a super Action class inherited by all the actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
}

public async createValidator(options: CreateValidatorOptions): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@epsjunior I like this interactive approach, but we also need to provide a way to pass all the parameters from the command itself. Otherwise, implementing other tools without human interaction that use the CLI to perform actions on GenLayer wouldn't be possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@cristiam86 cristiam86 left a comment

Choose a reason for hiding this comment

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

sorry, I missed this one in the previous review

.command("create-random")
.description("Create random validators")
.option("--count <count>", "Number of validators to create", "1") // Default to "1"
.option(
Copy link
Collaborator

Choose a reason for hiding this comment

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

@epsjunior we are also missing the --models option to match the endpoint full parameters: https://github.com/yeagerai/genlayer-studio/blob/main/backend/protocol_rpc/endpoints.py#L190

Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@e48e31e). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff            @@
##             main      #162   +/-   ##
========================================
  Coverage        ?   100.00%           
========================================
  Files           ?        26           
  Lines           ?      1266           
  Branches        ?       311           
========================================
  Hits            ?      1266           
  Misses          ?         0           
  Partials        ?         0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Implement validators commands
2 participants