Add commit-reveal subprocess and related utilities#2355
Closed
opendansor wants to merge 44 commits intostagingfrom
Closed
Add commit-reveal subprocess and related utilities#2355opendansor wants to merge 44 commits intostagingfrom
opendansor wants to merge 44 commits intostagingfrom
Conversation
Introducing a new subprocess for the commit-reveal mechanism alongside necessary utility functions. Also added end-to-end tests to ensure proper functionality of the new subprocess feature.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
added 7 commits
October 16, 2024 09:10
Replace hardcoded log file paths with constants for stdout and stderr log files. This improves code readability and maintainability by centralizing the log file paths. Removed redundant code that reads and prints the log files.
This commit introduces a check for whether commit-reveal is enabled before setting weights. If enabled, weights will be committed with a generated salt; otherwise, weights are set directly. Additionally, this commit adjusts the sleep interval for the subprocess and removes an unnecessary comment.
Added an option for starting the commit reveal subprocess during the Subtensor initialization. Enhanced subprocess management and logging details to capture wallet and commit information more clearly. Streamlined error handling in set_weights module.
Enhanced the commit-reveal mechanism by adding a subprocess with sleep interval control, class-based handling in commit_reveal.py, and improved reveal checking. Adjusted tests and utils accordingly to ensure functionality and performance consistency.
Introduce a method to calculate the number of blocks remaining until the next epoch for a specific subnet. This addition aids in network governance and operational planning by providing essential timing information within the Bittensor blockchain.
Added the version_key parameter to function signatures across multiple files for better version tracking. Updated docstrings for enhanced clarity and consistency, improving code readability and maintainability.
Introduced batch processing for weight reveal operations in the Bittensor network. This includes a new method `batch_reveal_weights` and necessary changes to accommodate batch transactions, ensuring efficiency and scalability. Additionally, the commit/reveal intervals were replaced with periods for more precise control.
basfroman
requested changes
Oct 19, 2024
added 4 commits
October 21, 2024 15:33
Introduce a function to verify local reveal list consistency with the Subtensor chain every 100 iterations. Refactor `check_reveal` and `reveal_candidates` to improve clarity and extract logic for retrieving all commits. Update docstrings and handle logs for better readability and debugging.
Subprocess initialization is now disabled by default. Reorganized socket server code with ThreadPoolExecutor and improved error handling. Removed redundant return type annotations and added docstrings.
added 12 commits
October 22, 2024 12:12
Renamed subprocess utility scripts from `/scripts` to `/bittensor/utils`. Updated import paths and refactored handling of subprocess logging by adding dynamic PID log management. Adjusted tests accordingly to reflect these changes.
Rearranged calls to 'convert_weights_and_uids_for_emit' to align with the logic flow across test_reveal_weights.py and set_weights.py. This ensures 'convert_weights_and_uids_for_emit' is invoked only in commit-reveal scenarios, enhancing code clarity and maintaining intended functionality.
Moved weight conversion and normalization steps into the beginning of the `set_weights` function, ensuring consistency between weight submission and commit operations. This update affects both the `set_weights` logic and its corresponding end-to-end tests.
Introduces a test to validate the commit-reveal mechanism for weights when the number of commits exceeds the limit. The test sets up a subnet, registers a neuron, and verifies behavior when attempting more than the allowed number of weight commits.
Added logic to conditionally start the commit reveal subprocess based on existing commits and introduced an expire block to manage commit object lifecycle. Updated tests and utilities to reflect these changes, ensuring proper subprocess and database handling.
Added 'max_retries' parameter to set_weights to handle retries. Updated commit log in commit_reveal to include commit_block. Also removed unnecessary part of the check_reveal function docstring for clarity.
Set `wait_for_inclusion` to `True` for weight commits, ensuring inclusion before proceeding. Added debug prints for commit hashes and blocks in the `subtensor` and `commit_reveal` modules to assist with troubleshooting and monitoring.
… subtensor + add revealed flag This commit adjusts the commit weights process by ensuring the commit_reveal subprocess starts appropriately, checking for its readiness, and syncing commit data. It also simplifies the test cases and enhances the subprocess logging mechanism.
Refactor the commit reveal process to only include candidates with a reveal block less than or equal to the current block. Also, remove unnecessary logging of received requests to improve code readability and reduce noise.
…alignment with commits Reduced the max_retries parameter to 1 in multiple methods to expedite failure recognition. Refactored inline substrate call compositions to use a more functional approach by passing the composed extrinsic directly into the retry wrapper, streamlining the code and improving readability.
added 2 commits
November 1, 2024 11:34
Update the conditional statement for subprocess initialization in `subtensor.py` for clarity. Adjust the unit tests in `test_set_weights.py` to patch `get_subnet_hyperparameters` method to return an object with `commit_reveal_weights_enabled` set to `False`.
Closed
Change the network parameter to self.chain_endpoint in subprocess initialization for better encapsulation. Clarify the commit_reveal_weights_interval docstring and update the default network address in argument parsing.
basfroman
requested changes
Nov 2, 2024
Collaborator
basfroman
left a comment
basfroman
left a comment
There was a problem hiding this comment.
Looks pretty well. Just lest a few comments.
Remove redundant `read_commit_reveal_logs` function. Enhance `is_table_empty` to handle table existence check. Add error handling and logging improvements for subprocess management.
Closed
added 5 commits
November 4, 2024 22:12
Refactored the commit and reveal weight functions to simplify code, eliminate retry loops, and remove user prompts. The subprocess handling has been improved for better subprocess control, and utility functions have been moved to a new module.
Refactored the commit and reveal weight functions to simplify code, eliminate retry loops, and remove user prompts. The subprocess handling has been improved for better subprocess control, and utility functions have been moved to a new module.
Closed
basfroman
approved these changes
Nov 5, 2024
# Conflicts: # bittensor/core/extrinsics/commit_weights.py # bittensor/core/extrinsics/set_weights.py # bittensor/core/subtensor.py # tests/e2e_tests/test_commit_weights.py # tests/unit_tests/test_subtensor.py
Closed
thewhaleking
requested changes
Nov 6, 2024
Contributor
thewhaleking
left a comment
thewhaleking
left a comment
There was a problem hiding this comment.
Most of the file changes are fine, but I think everything within background_subprocess needs a serious re-examination (except the SQL stuff).
| version_key (int): Version key for compatibility with the network. Defaults to `settings.version_as_int`. | ||
| """ | ||
|
|
||
| def send_command(command_): |
Contributor
There was a problem hiding this comment.
Is there a reason this function isn't reused?
| server_thread = threading.Thread(target=_start_socket_server) | ||
| server_thread.start() | ||
|
|
||
| counter = 0 # Initialize counter |
Comment on lines
+774
to
+790
| parser = argparse.ArgumentParser( | ||
| description="Run the Bittensor commit-reveal background_subprocess script." | ||
| ) | ||
| parser.add_argument( | ||
| "--network", | ||
| type=str, | ||
| default="wss://test.finney.opentensor.ai:443", | ||
| help="Subtensor network address", | ||
| ) | ||
| parser.add_argument( | ||
| "--sleep-interval", | ||
| type=float, | ||
| default=12.0, | ||
| help="Interval between block checks in seconds", | ||
| ) | ||
| args = parser.parse_args() | ||
| main(args) |
Contributor
There was a problem hiding this comment.
Why is this using an arg parser?
| server_thread.start() | ||
|
|
||
| counter = 0 # Initialize counter | ||
| print("commit_reveal background_subprocess is ready") |
Contributor
There was a problem hiding this comment.
Do we want to use print here?
| server.bind((HOST, PORT)) | ||
| server.listen(5) | ||
| server.settimeout(2) # Set timeout for any incoming requests to 2 seconds | ||
| print(f"Listening on port {PORT}...") |
Contributor
There was a problem hiding this comment.
Do we want to use print here?
| for combination in unique_combinations: | ||
| ss58, netuid = combination | ||
| try: | ||
| response = subtensor.query_module( |
Contributor
There was a problem hiding this comment.
Is this keeping the Subtensor object (and underlying ws connection) alive for the duration of this background process?
Contributor
|
By design of Constt, we're skipping from v1 to v3. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a new subprocess dedicated to automating the commit-reveal mechanism. Key utility functions are also included, along with end-to-end tests to verify the subprocess functionality.
With this update, the commit-reveal process can be fully automated. Subnet owners need only enable the
commit-revealflag within the hyper-parameter settings, and the subprocess will handle the commit-reveal process autonomously.Details
Long-Lived Subprocess:
This subprocess is designed to operate independently of the parent process. If the validator is stopped and restarted, the subprocess will continue managing existing commit-reveal cycles on its own. It doesn’t terminate when the parent thread is closed.
Local WebSocket Communication:
A WebSocket connection on
localhostallows the parent and subprocess to communicate. This connection offers the flexibility to start, terminate, commit, and reveal actions independently of the parent process, even if the parent has stopped. For example, you can terminate the subprocess from the terminal:Benefits
This enhancement improves:
The commit-reveal process becomes more robust, maintaining continuity across validator restarts.
Greater flexibility in managing the subprocess independently enhances control over commit-reveal operations.