Add support for the --all to proxy remove
#791
+779
−29
Conversation
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
Contributor
Author
|
Hi @thewhaleking , would you review my PR? cc: @ibraheem-abe |
Comment on lines
+9524
to
+9528
| if delegate: | ||
| if not json_output: | ||
| console.print( | ||
| "[yellow]Warning: --delegate is ignored when --all flag is used.[/yellow]" | ||
| ) |
Contributor
There was a problem hiding this comment.
This should probably kick back an error rather than just briefly displaying a warning.
Comment on lines
+312
to
+360
| async def remove_proxies( | ||
| subtensor: "SubtensorInterface", | ||
| wallet: "Wallet", | ||
| prompt: bool, | ||
| decline: bool, | ||
| quiet: bool, | ||
| wait_for_inclusion: bool, | ||
| wait_for_finalization: bool, | ||
| period: int, | ||
| json_output: bool, | ||
| ) -> None: | ||
| """ | ||
| Executes the remove_proxies call on the chain to remove all proxies for the account | ||
| """ | ||
| if prompt: | ||
| confirmation = Prompt.ask( | ||
| "[red]WARNING:[/red] This will remove ALL proxies associated with this account.\n" | ||
| "[red]All proxy relationships will be permanently lost.[/red]\n" | ||
| "To proceed, enter [red]REMOVE[/red]" | ||
| ) | ||
| if confirmation != "REMOVE": | ||
| print_error("Invalid input. Operation cancelled.") | ||
| return None | ||
| if not (ulw := unlock_key(wallet, print_out=not json_output)).success: | ||
| if not json_output: | ||
| print_error(ulw.message) | ||
| else: | ||
| json_console.print_json( | ||
| data={ | ||
| "success": ulw.success, | ||
| "message": ulw.message, | ||
| "extrinsic_identifier": None, | ||
| } | ||
| ) | ||
| return None | ||
| call = await subtensor.substrate.compose_call( | ||
| call_module="Proxy", | ||
| call_function="remove_proxies", | ||
| call_params={}, | ||
| ) | ||
| return await submit_proxy( | ||
| subtensor=subtensor, | ||
| wallet=wallet, | ||
| call=call, | ||
| wait_for_inclusion=wait_for_inclusion, | ||
| wait_for_finalization=wait_for_finalization, | ||
| period=period, | ||
| json_output=json_output, | ||
| ) |
Contributor
There was a problem hiding this comment.
This should probably be combined in remove_proxy function, with a flag.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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 adds functionality to remove all proxies associated with an account in a single operation. Previously, users had to remove proxies one by one using the
--delegateflag.Now, users can use the
--allflag to remove all proxies at once.Changes
Core Functionality (
bittensor_cli/src/commands/proxy.py)remove_proxies()function (lines 312-360):remove_proxiesextrinsic call on the chainProxy.remove_proxiessubstrate callCLI Integration (
bittensor_cli/cli.py)proxy_remove()command (lines 9449-9590):--allflag option to remove all proxiesdelegateparameter optional (required only when--allis not used)--delegateor--allis provided--delegateis required when--allis not used--delegateis provided alongside--allflag (delegate is ignored)remove_proxies()when--allflag is used, otherwise uses existingremove_proxy()functionFeatures
Single Proxy Removal (Existing Functionality)
Bulk Proxy Removal (New Functionality)
Usage Examples
Remove all proxies with confirmation:
btcli proxy remove --all # User must type "REMOVE" to confirmRemove all proxies without prompts:
Remove all proxies with JSON output:
Testing
End-to-End Tests (
tests/e2e_tests/test_proxy.py)test_remove_all_proxies()(lines 689-844):--allflagtest_remove_proxy_validation()(lines 847-924):--delegatenor--allis provided--allworks even when no proxies existtest_remove_proxy()(lines 927-1140):--allflagUnit Tests (
tests/unit_tests/test_cli.py)test_proxy_remove_requires_delegate_or_all()(lines 816-843):--delegatenor--allis providedtest_proxy_remove_requires_delegate_or_all_json()(lines 846-876):test_proxy_remove_with_all_flag()(lines 879-929):--allflag correctly callsremove_proxies()--delegateis provided with--allAll tests pass successfully and cover both success and failure scenarios, including edge cases.
User Experience Improvements
--allflag, users must type "REMOVE" to confirm the destructive operation--delegateflag--delegatealongside--allflag (delegate is ignored)Implementation Details
Validation Flow
--delegateor--allis provided--allis True → callsremove_proxies()--delegateis provided → callsremove_proxy()--allis False, ensures--delegateis providedError Handling
print_error()for user-friendly messagessuccess,message, andextrinsic_identifierfieldsBackward Compatibility
This change is fully backward compatible. Existing functionality for removing individual proxies remains unchanged. The
--allflag is an additional option that doesn't affect existing workflows. All existing tests continue to pass.Related Files
bittensor_cli/src/commands/proxy.py- Core proxy management functions (addedremove_proxies())bittensor_cli/cli.py- CLI command integration (enhancedproxy_remove())tests/e2e_tests/test_proxy.py- Comprehensive end-to-end test suitetests/unit_tests/test_cli.py- Unit tests for CLI validation logicScreenshot
Contribution by Gittensor, learn more at https://gittensor.io/