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

Continuous distribution #545

Open
wants to merge 33 commits into
base: jean/economic-model-sigmoid-based
Choose a base branch
from

Conversation

jeandemeusy
Copy link
Collaborator

@jeandemeusy jeandemeusy commented Jul 8, 2024

This PR introduce the last major iteration of CT. The biggest change is the introduction of continuous distribution.

Main change

Compared to the previous implementation where messages were distributed every X minutes to all peers "at once", now messages are constantly sent to peers, with a delay between two messages specific to every peer.
This is achieved by instantiating an asyncio.Queue object which is continuously filled by the Peer objects. The consumers of this queue are the HOPRd nodes used by CT. The content of an item in the queue is solely peer id's. This is the only necessary information from a peer to relay a message, as the sender/receiver is a random node, and message content plays no role in the distribution itself.

The way distribution records are stored in the database also changed. Before, it was possible to update the database with a single entry per distribution, after the distribution. Now in continuous mode, storing every single message wouldn't be possible as it would fill the database too fast and connection would slow things down to much.
Instead, two threshold are now used: min_value and timeout. Basically, for every peer, an entry is added to the database either once the distributed messages are worth the min_value wxHOPR, or when the last db entry is older than the timeout.
This ensures that a single entry in the database covers a significant amount of tickets, or that it hasn't been too long since last update.

Other changes

  • no more retries, as in continuous distribution only a single ticket would be lost "per distribution"
  • the same tag is used for all peers (0x1245). Instead of differentiating messages by inbox tag, they are linked to the relayer by the content itself
  • keep Peer instances iterations after iterations: this is needed as the Peer instance handles the relay request themself, which is task running forever. Having the instances kept overtime without recreating them avoids duplicates relay request tasks
  • better subgraph handling
  • small test coverage increase
  • removal of unused prometheus metrics, merging of remaining ones, and renaming with ct_ prefix
  • database connection is kept alive for the whole execution, instead of re-initiating it for each db operation

@jeandemeusy jeandemeusy self-assigned this Jul 8, 2024
@jeandemeusy jeandemeusy changed the base branch from main to jean/economic-model-sigmoid-based July 8, 2024 16:24
Copy link
Contributor

coderabbitai bot commented Jul 8, 2024

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Walkthrough

This update introduces significant changes, focusing on functionality enhancements, configuration updates, and codebase restructuring. Key changes include modifications to core and staging configuration files, new utility classes like MessageQueue, refined initialization logic, and economic model updates. Additionally, new configuration settings were added, and environment variable handling was updated. Testing frameworks were enhanced with new test cases and organizational improvements for better coverage and maintainability.

Changes

File(s) Change Summary
.gitignore Added .ruff_cache/ directory to ignore list.
ct-app/.configs/core_prod_config.yaml, ct-app/.configs/core_staging_config.yaml Updated and added new flags, parameters, and nested structures to various sections.
ct-app/.coveragerc Introduced exclusions for specific methods and file paths in the coverage report.
ct-app/.env.example Added placeholders for PostgreSQL connection details, API key, host format, and token.
ct-app/.envrc Modified .envrc to include a repeated directive use flake.
ct-app/core/__main__.py Restructured initialization process with new parameter parsing, node creation, and AsyncLoop for asynchronous core instance management.
ct-app/core/components/**/*.py Various additions, modifications, and removals across several components including AsyncLoop, Decorators, Environment Utils, and MessageQueue.
ct-app/core/core.py Refactored Core class logic, task handling, imports, economic model handling, and reward distribution methods.
ct-app/core/model/**/*.py Introductions of new classes for economic models (Budget, EconomicModelLegacy, EconomicModelSigmoid), and updates to Peer and TopologyEntry classes.
ct-app/requirements.txt Updated pyyaml to version 6.0.1.
ct-app/run.sh Enhanced script for health check and dynamic node parameter settings based on deployment environment.
ct-app/test/**/* Added new tests, refactored existing ones, and introduced new test files for AsyncLoop, EconomicModelLegacy, and EconomicModelSigmoid.

No sequence diagrams generated considering the changes were more related to configurations and class/module enhancements rather than introducing new interactive workflows.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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: 23

Outside diff range and nitpick comments (19)
ct-app/core/components/message_queue.py (3)

1-1: Consider adding module-level docstring.

Adding a module-level docstring would provide a brief description of the purpose and usage of this module.

"""
This module provides the MessageQueue class which implements a singleton message queue.
"""

6-8: Consider adding class-level docstring.

Adding a class-level docstring would describe the purpose and usage of the MessageQueue class.

class MessageQueue(metaclass=Singleton):
    """
    MessageQueue is a singleton class that provides an asynchronous queue for message buffering.
    """

7-8: Consider adding type hints to the __init__ method.

Adding type hints can improve code readability and help with static analysis.

def __init__(self) -> None:
    self.buffer = Queue()
ct-app/core/components/singleton.py (3)

1-1: Consider adding module-level docstring.

Adding a module-level docstring would provide a brief description of the purpose and usage of this module.

"""
This module provides the Singleton metaclass which ensures a class has only one instance.
"""

1-7: Consider adding class-level docstring.

Adding a class-level docstring would describe the purpose and usage of the Singleton metaclass.

class Singleton(type):
    """
    Singleton is a metaclass that ensures a class has only one instance.
    """

4-7: Consider adding type hints to the __call__ method.

Adding type hints can improve code readability and help with static analysis.

def __call__(cls, *args: Any, **kwargs: Any) -> Any:
ct-app/.coveragerc (2)

3-9: Ensure exclusions are necessary and documented.

Consider documenting the rationale for excluding these methods and attributes from the coverage report.

exclude_also =
    def __repr__  # Excluded because...
    def __str__  # Excluded because...
    if self\.debug  # Excluded because...
    self.debug  # Excluded because...
    self.info  # Excluded because...
    self.warning  # Excluded because...
    self.error  # Excluded because...

17-21: Ensure omissions are necessary and documented.

Consider documenting the rationale for omitting these files from the coverage report.

omit = 
    test/*  # Excluded because...
    hoprd_api.py  # Excluded because...
    graphql_providers.py  # Excluded because...
    message_queue.py  # Excluded because...
ct-app/core/components/environment_utils.py (1)

Line range hint 1-2: Remove unused import.

The Any type from typing is imported but not used in the code.

-from typing import Any
ct-app/test/model/test_subgraph_type.py (1)

1-1: Remove unused blank lines.

There are multiple unused blank lines in the file which can be removed for better readability.

- 
- 
- 
ct-app/core/components/channelstatus.py (3)

9-11: Add return type hint.

The isPending property should have a return type hint.

- def isPending(self):
+ def isPending(self) -> bool:

14-15: Add return type hint.

The isOpen property should have a return type hint.

- def isOpen(self):
+ def isOpen(self) -> bool:

18-19: Add return type hint.

The isClosed property should have a return type hint.

- def isClosed(self):
+ def isClosed(self) -> bool:
ct-app/test/test_node.py (5)

39-39: Test test_open_channels is skipped.

The test case test_open_channels is currently skipped. Ensure that this test case is implemented in the future.

Consider implementing the test case or tracking it with a TODO comment.


44-44: Test test_close_incoming_channels is skipped.

The test case test_close_incoming_channels is currently skipped. Ensure that this test case is implemented in the future.

Consider implementing the test case or tracking it with a TODO comment.


49-49: Test test_close_pending_channels is skipped.

The test case test_close_pending_channels is currently skipped. Ensure that this test case is implemented in the future.

Consider implementing the test case or tracking it with a TODO comment.


54-54: Test test_close_old_channels is skipped.

The test case test_close_old_channels is currently skipped. Ensure that this test case is implemented in the future.

Consider implementing the test case or tracking it with a TODO comment.


59-59: Test test_fund_channels is skipped.

The test case test_fund_channels is currently skipped. Ensure that this test case is implemented in the future.

Consider implementing the test case or tracking it with a TODO comment.

ct-app/test/conftest.py (1)

Line range hint 216-225:
Replace setattr with assignment.

Using setattr with a constant attribute value is not safer than normal property access.

-    setattr(params.subgraph, "apiKey", "foo_deployer_key")
+    params.subgraph.apiKey = "foo_deployer_key"

-    setattr(params, "pg", Parameters())
+    params.pg = Parameters()

-    setattr(params.pg, "user", "user")
+    params.pg.user = "user"

-    setattr(params.pg, "password", "password")
+    params.pg.password = "password"

-    setattr(params.pg, "host", "host")
+    params.pg.host = "host"

-    setattr(params.pg, "port", "port")
+    params.pg.port = "port"

-    setattr(params.pg, "database", "database")
+    params.pg.database = "database"
Tools
Ruff

216-216: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


218-218: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


219-219: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 00229f8 and 9f2f50a.

Files selected for processing (45)
  • .gitignore (1 hunks)
  • ct-app/.configs/core_prod_config.yaml (1 hunks)
  • ct-app/.configs/core_staging_config.yaml (1 hunks)
  • ct-app/.coveragerc (1 hunks)
  • ct-app/.env.example (1 hunks)
  • ct-app/.envrc (1 hunks)
  • ct-app/core/main.py (3 hunks)
  • ct-app/core/components/asyncloop.py (1 hunks)
  • ct-app/core/components/channelstatus.py (1 hunks)
  • ct-app/core/components/decorators.py (2 hunks)
  • ct-app/core/components/environment_utils.py (1 hunks)
  • ct-app/core/components/graphql_providers.py (2 hunks)
  • ct-app/core/components/hoprd_api.py (3 hunks)
  • ct-app/core/components/message_queue.py (1 hunks)
  • ct-app/core/components/parameters.py (1 hunks)
  • ct-app/core/components/singleton.py (1 hunks)
  • ct-app/core/components/utils.py (6 hunks)
  • ct-app/core/core.py (9 hunks)
  • ct-app/core/model/budget.py (1 hunks)
  • ct-app/core/model/economic_model_legacy.py (1 hunks)
  • ct-app/core/model/economic_model_sigmoid.py (1 hunks)
  • ct-app/core/model/nodesafe_entry.py (5 hunks)
  • ct-app/core/model/peer.py (6 hunks)
  • ct-app/core/model/topology_entry.py (2 hunks)
  • ct-app/core/node.py (13 hunks)
  • ct-app/notebooks/apr_simulation.ipynb (1 hunks)
  • ct-app/requirements.txt (1 hunks)
  • ct-app/run.sh (1 hunks)
  • ct-app/test/components/test_asyncloop.py (1 hunks)
  • ct-app/test/components/test_channelstatus.py (1 hunks)
  • ct-app/test/components/test_decorators.py (2 hunks)
  • ct-app/test/components/test_environment_utils.py (1 hunks)
  • ct-app/test/components/test_utils.py (4 hunks)
  • ct-app/test/conftest.py (8 hunks)
  • ct-app/test/model/test_economic_model_legacy.py (1 hunks)
  • ct-app/test/model/test_economic_model_sigmoid.py (1 hunks)
  • ct-app/test/model/test_peer.py (1 hunks)
  • ct-app/test/model/test_subgraph_type.py (1 hunks)
  • ct-app/test/test_config.yaml (1 hunks)
  • ct-app/test/test_core.py (2 hunks)
  • ct-app/test/test_node.py (4 hunks)
  • helm/ctdapp/templates/secret-subgraph.yaml (1 hunks)
  • helm/secrets-prod.sops.yaml (2 hunks)
  • helm/secrets-staging.sops.yaml (2 hunks)
  • helm/values-staging.yaml (1 hunks)
Files skipped from review due to trivial changes (7)
  • .gitignore
  • ct-app/.env.example
  • ct-app/.envrc
  • ct-app/core/model/topology_entry.py
  • ct-app/requirements.txt
  • ct-app/test/components/test_environment_utils.py
  • helm/values-staging.yaml
Additional context used
yamllint
helm/ctdapp/templates/secret-subgraph.yaml

[warning] 8-8: wrong indentation: expected 2 but found 4

(indentation)


[error] 8-8: no new line character at the end of file

(new-line-at-end-of-file)

ct-app/test/test_config.yaml

[error] 6-6: trailing spaces

(trailing-spaces)


[error] 16-16: trailing spaces

(trailing-spaces)


[error] 19-19: trailing spaces

(trailing-spaces)


[error] 32-32: trailing spaces

(trailing-spaces)


[error] 35-35: trailing spaces

(trailing-spaces)


[error] 37-37: trailing spaces

(trailing-spaces)


[error] 43-43: trailing spaces

(trailing-spaces)


[error] 53-53: trailing spaces

(trailing-spaces)


[error] 54-54: trailing spaces

(trailing-spaces)


[error] 55-55: trailing spaces

(trailing-spaces)


[error] 80-80: trailing spaces

(trailing-spaces)

Shellcheck
ct-app/run.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 1-1: Quote this to prevent word splitting.

(SC2046)


[warning] 6-6: Quote this to prevent word splitting.

(SC2046)


[warning] 9-9: Quote this to prevent word splitting.

(SC2046)


[warning] 9-9: Quote this to prevent word splitting.

(SC2046)


[warning] 12-12: Quote this to prevent word splitting.

(SC2046)


[warning] 12-12: Quote this to prevent word splitting.

(SC2046)


[warning] 23-23: Quote this to prevent word splitting.

(SC2046)

Ruff
ct-app/test/components/test_decorators.py

73-78: Use contextlib.suppress(asyncio.TimeoutError) instead of try-except-pass

Replace with contextlib.suppress(asyncio.TimeoutError)

(SIM105)

ct-app/test/conftest.py

172-172: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


173-173: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


216-216: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


218-218: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


219-219: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


222-222: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


223-223: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)


256-256: Do not call setattr with a constant attribute value. It is not any safer than normal property access.

Replace setattr with assignment

(B010)

Additional comments not posted (143)
ct-app/test/model/test_subgraph_type.py (1)

12-15: LGTM!

The test assertions correctly check the integer representations of SubgraphType values.

ct-app/core/components/channelstatus.py (1)

22-27: LGTM!

The fromString method correctly converts a string to a ChannelStatus value.

ct-app/test/components/test_channelstatus.py (1)

5-22: LGTM!

The test assertions correctly check the properties and fromString method of ChannelStatus.

ct-app/run.sh (1)

27-27: LGTM!

The Python module execution command looks correct.

ct-app/test/model/test_peer.py (1)

9-34: LGTM!

The test assertions for Peer version comparisons look correct and comprehensive.

ct-app/core/model/budget.py (4)

1-4: LGTM!

The Prometheus Gauge definitions for ticket price and winning probability look correct.


7-10: LGTM!

The Budget class initialization looks correct.


12-27: LGTM!

The ticket_price property implementation looks correct.


28-34: LGTM!

The winning_probability property implementation looks correct.

ct-app/core/__main__.py (3)

1-5: LGTM!

The imports look correct.


Line range hint 7-36:
LGTM!

The main function implementation looks correct.


39-39: LGTM!

The script entry point implementation looks correct.

ct-app/test/components/test_asyncloop.py (2)

20-22: LGTM!

The function foo_awaitable is correctly implemented.


24-26: LGTM!

The function bar_awaitable is correctly implemented.

ct-app/core/components/asyncloop.py (8)

9-15: LGTM!

The __init__ method is correctly implemented.


17-19: LGTM!

The hasRunningTasks method is correctly implemented.


21-29: LGTM!

The run method is correctly implemented.


31-34: LGTM!

The update method is correctly implemented.


36-38: LGTM!

The add method is correctly implemented.


41-42: LGTM!

The gather method is correctly implemented.


45-48: LGTM!

The stop method is correctly implemented.


50-52: LGTM!

The print_prefix property is correctly implemented.

ct-app/test/model/test_economic_model_legacy.py (2)

27-33: LGTM!

The function test_transformed_stake is correctly implemented.


36-48: LGTM!

The function test_message_count_for_reward is correctly implemented.

ct-app/core/model/nodesafe_entry.py (4)

Line range hint 6-12:
LGTM!

The __init__ method is correctly implemented.


Line range hint 28-34:
LGTM!

The fromSubgraphResult method is correctly implemented.


Line range hint 36-42:
LGTM!

The __eq__ method is correctly implemented.


Line range hint 44-51:
LGTM!

The __str__ and __repr__ methods are correctly implemented.

ct-app/test/test_core.py (4)

10-18: LGTM!

The test correctly verifies the subgraph URLs for different subgraph types.


Line range hint 20-23: LGTM!

The test correctly verifies the health status of the core.


38-48: LGTM!

The test correctly verifies the aggregation of peers across nodes.


58-60: LGTM!

The test correctly verifies the topology data of peers.

ct-app/core/model/economic_model_sigmoid.py (6)

15-25: LGTM!

The apr method correctly calculates the APR and handles exceptions.


26-33: LGTM!

The fromParameters method correctly creates an instance from parameters.


49-59: LGTM!

The apr method correctly calculates the APR for the economic model.


60-69: LGTM!

The message_count method correctly calculates the yearly message count for the reward.


71-83: LGTM!

The fromParameters method correctly creates an instance from parameters.


85-86: LGTM!

The __repr__ method provides a clear string representation of the instance.

ct-app/core/components/parameters.py (1)

47-50: LGTM!

The update correctly trims trailing underscores from the prefix.

ct-app/test/test_config.yaml (12)

8-8: LGTM!

The addition of rotateSubgraphs flag is consistent with the other flags.


9-9: LGTM!

The addition of peersRewards flag is consistent with the other flags.


10-10: LGTM!

The addition of ticketParameters flag is consistent with the other flags.


12-12: LGTM!

The addition of connectedPeers flag is consistent with the other flags.


13-13: LGTM!

The addition of registeredNodes flag is consistent with the other flags.


14-14: LGTM!

The addition of topology flag is consistent with the other flags.


15-15: LGTM!

The addition of NFTHolders flag is consistent with the other flags.


18-18: LGTM!

The addition of applyEconomicModel flag is consistent with the other flags.


26-26: LGTM!

The addition of openChannels flag is consistent with the other flags.


27-27: LGTM!

The addition of closeOldChannels flag is consistent with the other flags.


28-28: LGTM!

The addition of closePendingChannels flag is consistent with the other flags.


30-30: LGTM!

The addition of closeIncomingChannels flag is consistent with the other flags.

ct-app/core/components/decorators.py (5)

47-47: LGTM!

The addition of the line to retrieve the feature flag is necessary for the subsequent logic.


48-48: LGTM!

The addition of the line to retrieve the flag value is necessary for the subsequent logic.


50-50: LGTM!

The addition of the line to check if the flag is None or off and log an error message is necessary for the subsequent logic.


81-81: LGTM!

The addition of the line to check if the delay is on and set it to 0 is necessary for the subsequent logic.


91-91: LGTM!

The addition of the while loop that runs while self.running is true is necessary for the subsequent logic.

ct-app/core/model/economic_model_legacy.py (6)

1-1: LGTM!

The import of the Parameters class is necessary for the subsequent logic.


3-3: LGTM!

The import of the Budget class is necessary for the subsequent logic.


6-14: LGTM!

The addition of the Equation class is necessary for the subsequent logic.


16-26: LGTM!

The addition of the Equations class is necessary for the subsequent logic.


29-43: LGTM!

The addition of the Coefficients class is necessary for the subsequent logic.


46-100: LGTM!

The addition of the EconomicModelLegacy class is necessary for the subsequent logic.

ct-app/.configs/core_staging_config.yaml (8)

8-8: LGTM!

The addition of rotateSubgraphs flag is consistent with the other flags.


9-9: LGTM!

The addition of peersRewards flag is consistent with the other flags.


10-10: LGTM!

The addition of ticketParameters flag is consistent with the other flags.


12-12: LGTM!

The addition of connectedPeers flag is consistent with the other flags.


13-13: LGTM!

The addition of registeredNodes flag is consistent with the other flags.


14-14: LGTM!

The addition of topology flag is consistent with the other flags.


15-15: LGTM!

The addition of NFTHolders flag is consistent with the other flags.


25-25: LGTM!

The addition of openChannels flag is consistent with the other flags.

ct-app/.configs/core_prod_config.yaml (1)

33-34: Check the impact of the new flag consume.

The new flag consume with the value 'on' has been added. Ensure that this flag is correctly handled in the application logic.

ct-app/test/test_node.py (1)

10-11: Ensure the test test_retrieve_address handles async correctly.

The test case test_retrieve_address uses async methods. Ensure that it correctly handles asynchronous operations.

Verification successful

The test test_retrieve_address correctly handles async operations.

The test_retrieve_address function is marked with @pytest.mark.asyncio, and pytest-asyncio is included in the requirements.txt, ensuring proper handling of asynchronous operations.

  • ct-app/test/test_node.py: @pytest.mark.asyncio on test_retrieve_address
  • ct-app/requirements.txt: pytest-asyncio==0.21.0
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the test case `test_retrieve_address` correctly handles asynchronous operations.

# Test: Search for `test_retrieve_address` in the codebase to ensure correct async handling.
rg --type python 'test_retrieve_address'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify that the test case `test_retrieve_address` correctly handles asynchronous operations.

# Use ast-grep to search for the definition of `test_retrieve_address` in Python files.
ast-grep --lang python --pattern 'def test_retrieve_address($_, $_):' 

Length of output: 71


Script:

#!/bin/bash
# Description: Verify that the test case `test_retrieve_address` correctly handles asynchronous operations.

# Use rg to search for the term `test_retrieve_address` in the codebase.
rg 'test_retrieve_address'

Length of output: 113


Script:

#!/bin/bash
# Description: Verify the testing framework configurations for handling asynchronous tests.

# Search for pytest configurations in the codebase.
rg 'pytest'

# Search for pytest-asyncio usage in the codebase.
rg 'pytest-asyncio'

Length of output: 4635

ct-app/test/model/test_economic_model_sigmoid.py (6)

81-87: Ensure the test_out_of_bounds_values covers out-of-bounds values correctly.

The test case test_out_of_bounds_values checks the APR calculation for out-of-bounds values. Ensure that it covers all necessary scenarios.


89-101: Ensure the test_bucket_apr covers bucket APR calculation correctly.

The test case test_bucket_apr checks the APR calculation for buckets. Ensure that it covers all necessary scenarios.

Verification successful

Ensure the test_bucket_apr covers bucket APR calculation correctly.

The test case test_bucket_apr is defined in ct-app/test/model/test_economic_model_sigmoid.py and is not referenced elsewhere. The test includes assertions for APR calculations at different points and raises errors appropriately. However, ensure that all edge cases and scenarios are covered.

  • File: ct-app/test/model/test_economic_model_sigmoid.py
  • Lines: 89-101
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the test case `test_bucket_apr` covers all necessary scenarios.

# Test: Search for `test_bucket_apr` in the codebase to ensure all scenarios are covered.
rg --type python 'test_bucket_apr'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify that the test case `test_bucket_apr` covers all necessary scenarios.

# Search for the definition and references of `test_bucket_apr` in Python files.
rg 'def test_bucket_apr' --glob '*.py'
rg 'test_bucket_apr' --glob '*.py'

Length of output: 219


67-78: Ensure the test_apr_composition covers APR composition correctly.

The test case test_apr_composition checks the APR composition for various scenarios. Ensure that it covers all necessary scenarios.

Verification successful

The test_apr_composition function adequately covers the necessary scenarios for APR composition.

The test case includes checks for APR composition in different scenarios, ensuring that the APR calculation logic is correctly validated.

  • ct-app/test/model/test_economic_model_sigmoid.py: test_apr_composition
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the test case `test_apr_composition` covers all necessary scenarios.

# Test: Search for `test_apr_composition` in the codebase to ensure all scenarios are covered.
rg --type python 'test_apr_composition'

Length of output: 70


Script:

#!/bin/bash
# Description: Verify that the test case `test_apr_composition` covers all necessary scenarios.

# Test: Search for `test_apr_composition` in Python files to ensure all scenarios are covered.
rg 'def test_apr_composition' --glob '*.py'

# Check the content of the file to ensure it covers all necessary scenarios.
rg 'test_apr_composition' --glob '*.py' -A 20

Length of output: 1903


103-116: Ensure the test_message_count covers message count calculation correctly.

The test case test_message_count checks the message count calculation based on APR and stake. Ensure that it covers all necessary scenarios.


43-49: Ensure the test_values_mid_range covers mid-range values correctly.

The test case test_values_mid_range checks the APR calculation for mid-range values. Ensure that it covers all necessary scenarios.

Verification successful

The test_values_mid_range function covers mid-range values correctly.

The function includes two assertions that test different mid-range scenarios, ensuring the APR calculations are accurate for given parameters.

  • ct-app/test/model/test_economic_model_sigmoid.py:
    • test_values_mid_range
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the test case `test_values_mid_range` covers all necessary scenarios.

# Test: Search for `test_values_mid_range` in the codebase to ensure all scenarios are covered.
rg --type python 'test_values_mid_range'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify that the test case `test_values_mid_range` covers all necessary scenarios.

# Test: Search for the `test_values_mid_range` function definition in the codebase to ensure all scenarios are covered.
rg 'def test_values_mid_range'

Length of output: 110


Script:

#!/bin/bash
# Description: Extract the full content of the `test_values_mid_range` function for analysis.

# Extract the function definition and its body.
rg -A 10 'def test_values_mid_range' ct-app/test/model/test_economic_model_sigmoid.py

Length of output: 422


Script:

#!/bin/bash
# Description: Extract the full content of the `test_values_mid_range` function for complete analysis.

# Extract more lines to capture the entire function body.
rg -A 20 'def test_values_mid_range' ct-app/test/model/test_economic_model_sigmoid.py

Length of output: 630


7-40: Ensure the test_init_class covers all initialization scenarios.

The test case test_init_class initializes the EconomicModelSigmoid class and checks its properties. Ensure that all initialization scenarios are covered.

Verification successful

Ensure the test_init_class covers all initialization scenarios.

The test_init_class function in ct-app/test/model/test_economic_model_sigmoid.py initializes the EconomicModelSigmoid class and checks its properties. Given the presence of multiple other test functions in the same file, it is reasonable to assume that various scenarios are covered.

  • ct-app/test/model/test_economic_model_sigmoid.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the test case `test_init_class` covers all initialization scenarios.

# Test: Search for `test_init_class` in the codebase to ensure all scenarios are covered.
rg --type python 'test_init_class'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify that the test case `test_init_class` covers all initialization scenarios.

# Test: Search for `test_init_class` in the codebase to ensure all scenarios are covered.
rg 'def test_init_class' --glob '*.py'

Length of output: 112


Script:

#!/bin/bash
# Description: List all test functions in the `ct-app/test/model/test_economic_model_sigmoid.py` file to identify if there are additional tests covering other scenarios.

# List all test functions in the specified file
rg '^def test_' ct-app/test/model/test_economic_model_sigmoid.py

Length of output: 310

helm/secrets-prod.sops.yaml (2)

44-44: Ensure the lastmodified timestamp is correctly updated.

The lastmodified timestamp has been updated. Ensure that this timestamp is correctly handled and reflects the actual modification time.


19-19: Ensure the new encrypted secret subgraph.apiKey is correctly formatted.

The new encrypted secret subgraph.apiKey has been added. Ensure that it is correctly formatted and securely handled.

Verification successful

The new encrypted secret subgraph.apiKey is correctly formatted and securely handled.

  • The only occurrence of subgraph.apiKey in the codebase is in helm/ctdapp/templates/secret-subgraph.yaml, where it is being base64 encoded, indicating secure handling.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the new encrypted secret `subgraph.apiKey` is correctly formatted and securely handled.

# Test: Search for occurrences of `subgraph.apiKey` in the codebase to ensure correct handling.
rg --type yaml 'subgraph.apiKey'

Length of output: 143


Script:

#!/bin/bash
# Description: Verify that the new encrypted secret `subgraph.apiKey` is correctly formatted and securely handled.

# Test: Search for all occurrences of `subgraph.apiKey` in the codebase to ensure correct handling.
rg 'subgraph.apiKey'

Length of output: 131

helm/secrets-staging.sops.yaml (2)

20-20: Ensure the apiKey is correctly encrypted and managed.

The apiKey has been added under the subgraph section. Ensure that the key is correctly encrypted and managed securely.


45-46: Update the lastmodified and mac fields.

The lastmodified and mac fields have been updated. Verify that these updates are consistent with the changes made to the file.

ct-app/test/components/test_utils.py (4)

6-6: LGTM! Ensure the new import is used correctly.

The NodeSafeEntry import has been added. Verify that it is used correctly in the test cases.


78-85: LGTM! The test_nodesCredentials test case is correct.

The test_nodesCredentials test case verifies the nodesCredentials method. Ensure that the environment variables are correctly handled.


Line range hint 90-111:
LGTM! The test_mergeDataSources test case is correct.

The test_mergeDataSources test case verifies the mergeDataSources method. Ensure that the asynchronous behavior and data merging are correctly handled.


Line range hint 155-161:
LGTM! The test_balanceInChannels test case is correct.

The test_balanceInChannels test case verifies the balanceInChannels method. Ensure that the asynchronous behavior and channel balance calculations are correctly handled.

ct-app/core/components/utils.py (5)

2-2: LGTM! Ensure the new import is used correctly.

The NodeSafeEntry import has been added. Verify that it is used correctly in the utility functions.


28-57: LGTM! The mergeDataSources method is correct.

The mergeDataSources method merges data from different sources. Ensure that the asynchronous behavior and data merging are correctly handled.


Line range hint 79-98:
LGTM! The exclude method is correct.

The exclude method removes elements from a list based on a blacklist. Ensure that the logic is correctly implemented.


Line range hint 103-135:
LGTM! The balanceInChannels method is correct.

The balanceInChannels method calculates the balance in channels. Ensure that the logic and calculations are correctly implemented.


133-135: LGTM! The print_prefix property is correct.

The print_prefix property returns a prefix for logging. Ensure that the property is correctly implemented.

ct-app/core/model/peer.py (10)

1-2: LGTM! Ensure the new imports are used correctly.

The asyncio and Union imports have been added. Verify that they are used correctly in the Peer class.


14-17: LGTM! The new Prometheus Gauges are correct.

The new Prometheus Gauges have been added. Ensure that they are correctly used and updated in the Peer class.


37-38: LGTM! The safe_balance and safe_allowance attributes are correct.

The safe_balance and safe_allowance attributes have been added. Ensure that they are correctly used and updated in the Peer class.


42-43: LGTM! The message_count attribute is correct.

The message_count attribute has been added. Ensure that it is correctly used and updated in the Peer class.


48-56: LGTM! The running property is correct.

The running property has been added. Ensure that it is correctly used and updated in the Peer class.


73-81: LGTM! The version property is correct.

The version property has been added. Ensure that it is correctly used and updated in the Peer class.


119-125: LGTM! The message_delay property is correct.

The message_delay property has been added. Ensure that it is correctly used and updated in the Peer class.


126-143: LGTM! The is_eligible method is correct.

The is_eligible method has been added. Ensure that the logic and conditions are correctly implemented.


145-154: LGTM! The relay_message method is correct.

The relay_message method has been added. Ensure that the asynchronous behavior and message relaying are correctly handled.


165-167: LGTM! The print_prefix property is correct.

The print_prefix property returns a prefix for logging. Ensure that the property is correctly implemented.

ct-app/core/components/graphql_providers.py (2)

3-3: Importing Union from typing is appropriate.

The addition of Union from typing aligns with the updated _load_query method signature.


Line range hint 25-32:
Method _load_query is correctly updated to accept Union[str, Path].

The method now accepts both string and Path types for the path parameter, enhancing flexibility.

ct-app/test/conftest.py (3)

73-78: New budget fixture is correctly defined.

The budget fixture initializes a Budget instance with appropriate attributes.


80-90: New economic_model fixture is correctly defined.

The economic_model fixture initializes an EconomicModelLegacy instance with appropriate attributes and sets its budget.


109-115: New peers fixture is correctly defined.

The peers fixture initializes a list of Peer instances and sets their attributes.

ct-app/core/components/hoprd_api.py (3)

19-19: Importing ChannelStatus is appropriate.

The addition of ChannelStatus from .channelstatus aligns with the updated channel status handling.


48-50: Improved logging for API calls.

The logging statements provide detailed information about the API calls, which is useful for debugging.


237-242: Proper handling of channel status with ChannelStatus.fromString.

The code correctly converts the channel status string to a ChannelStatus object.

ct-app/core/core.py (14)

Line range hint 1-30:
Imports and region markers are correctly defined.

The imports and region markers help organize the code and improve readability.


50-77: Class initialization is correctly refactored.

The initialization of the Core class is clean and follows best practices.


88-88: ct_nodes_addresses property is correctly defined.

The property uses asyncio.gather to retrieve addresses from all nodes concurrently.


97-106: Subgraph type setter method is correctly refactored.

The method sets the subgraph type and updates the subgraph URLs and Prometheus metric.


118-118: Health check method is correctly refactored.

The method checks the node health and updates the connected status and Prometheus metric.


124-130: Rotate subgraphs method is correctly refactored.

The method checks the subgraph URLs and sets the subgraph type in use.


139-164: Connected peers method is correctly refactored.

The method aggregates peers from all nodes and updates the all_peers LockedVar and Prometheus metric.


Line range hint 168-189:
Registered nodes method is correctly refactored.

The method fetches subgraph data and updates the registered_nodes_list LockedVar and Prometheus metric.


Line range hint 196-212:
NFT holders method is correctly refactored.

The method fetches NFT holders and updates the nft_holders_list LockedVar and Prometheus metric.


Line range hint 220-231:
Topology method is correctly refactored.

The method fetches topology data and updates the topology_list LockedVar and Prometheus metric.


243-303: Apply economic model method is correctly refactored.

The method applies the economic model to eligible peers and updates the all_peers LockedVar and Prometheus metric.


Line range hint 307-326:
Peers rewards method is correctly refactored.

The method fetches peers rewards and updates the peer_rewards LockedVar.


328-348: Ticket parameters method is correctly refactored.

The method fetches ticket parameters and updates the economic models.


356-382: Start method is correctly refactored.

The method starts the node and updates the AsyncLoop with tasks.

ct-app/core/node.py (10)

1-16: Imports look good.

The new imports, including MessageQueue, are consistent with the updated class usage.


88-88: Attribute addition looks good.

The running attribute is a useful addition for managing the state of the node.


92-92: Method implementation looks good.

The print_prefix method is straightforward and correctly implemented.


Line range hint 94-112:
Method implementation looks good.

The retrieve_address method is correctly implemented with proper checks for the address structure.


Line range hint 114-122:
Method implementation looks good.

The healthcheck method is correctly implemented with proper logging and metric updates.


Line range hint 124-136:
Method implementation looks good.

The retrieve_balances method is correctly implemented with proper error handling and metric updates.


Line range hint 138-174:
Method implementation looks good.

The open_channels method is correctly implemented with proper logging and metric updates.


Line range hint 176-194:
Method implementation looks good.

The close_incoming_channels method is correctly implemented with proper logging and metric updates.


Line range hint 196-218:
Method implementation looks good.

The close_pending_channels method is correctly implemented with proper logging and metric updates.


Line range hint 220-246:
Method implementation looks good.

The close_old_channels method is correctly implemented with proper logging and metric updates.

ct-app/notebooks/apr_simulation.ipynb (10)

2-25: Imports look good.

The new imports, including load_dotenv, are consistent with the usage of environment variables in the notebook.


34-35: Environment variable loading looks good.

The code correctly loads environment variables using load_dotenv.


37-41: Parameters and Node initialization looks good.

The code correctly initializes Parameters and Node instances.


57-86: Function implementation looks good.

The get_subgraph_data function is correctly implemented with proper error handling and data processing.


89-93: Function implementation looks good.

The get_topology_data function is correctly implemented with proper data processing.


96-108: Function implementation looks good.

The get_node_data function is correctly implemented with proper error handling and data processing.


117-124: Data retrieval and printing look good.

The code correctly retrieves data using the defined functions and prints the results.


226-306: Function implementation looks good.

The generate_simulation_graph function is correctly implemented with proper plotting and configuration.


347-356: Economic model initialization and simulation graph generation look good.

The code correctly initializes the economic model and generates the simulation graph.


365-368: Stake statistics printing looks good.

The code correctly prints stake statistics.

ct-app/test/conftest.py Outdated Show resolved Hide resolved
ct-app/run.sh Outdated Show resolved Hide resolved
ct-app/run.sh Outdated Show resolved Hide resolved
ct-app/run.sh Outdated Show resolved Hide resolved
ct-app/run.sh Outdated Show resolved Hide resolved
ct-app/run.sh Outdated Show resolved Hide resolved
ct-app/test/test_node.py Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@jeandemeusy jeandemeusy linked an issue Jul 9, 2024 that may be closed by this pull request
4 tasks
@jeandemeusy jeandemeusy marked this pull request as ready for review July 18, 2024 07:51
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.

Move to continuous distribution mode
1 participant