-
Notifications
You must be signed in to change notification settings - Fork 373
feat: add get_subnet_owner_hotkey method to Subtensor #2815
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
base: staging
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add unit tests
for these methods and test for new method in tests/e2e_tests/test_subtensor_functions.py
Thank you for contribution!
bittensor/core/subtensor.py
Outdated
|
||
# Assuming the query returns the SS58 address directly or an object with a value attribute | ||
# Adjust based on the actual return type if needed | ||
owner_hotkey = getattr(result, "value", result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this check because self.query_subtensor
returns None
or str
from this request
bittensor/core/subtensor.py
Outdated
if isinstance(owner_hotkey, str) and is_valid_ss58_address(owner_hotkey): | ||
return owner_hotkey | ||
else: | ||
logging.error( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider to use debug
level in this case
bittensor/core/subtensor.py
Outdated
# Check if the returned hotkey is valid (basic check) | ||
if isinstance(owner_hotkey, str) and is_valid_ss58_address(owner_hotkey): | ||
return owner_hotkey | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need else
in this step. pls see the comment suggestion for async method
bittensor/core/subtensor.py
Outdated
# Assuming the query returns the SS58 address directly or an object with a value attribute | ||
# Adjust based on the actual return type if needed | ||
owner_hotkey = getattr(result, "value", result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same as in async method suggestion
Co-authored-by: Roman <[email protected]>
Co-authored-by: Roman <[email protected]>
@basfroman i will continue work on it today. |
hey @gap-editor , pls fix my typo in the suggestion instead of block_hash+ = await self.determine_block_hash(block, block_hash, reuse_block) use block_hash_ = await self.determine_block_hash(block, block_hash, reuse_block) |
tests/unit_tests/test_subtensor.py
Outdated
@@ -83,24 +83,35 @@ def test_methods_comparable(mock_substrate): | |||
|
|||
# methods which lives in async subtensor only | |||
excluded_async_subtensor_methods = ["initialize"] | |||
subtensor_methods = [m for m in dir(subtensor) if not m.startswith("_")] | |||
subtensor_methods = [m for m in dir(subtensor) if not m.startswith("_") and callable(getattr(subtensor, m))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and callable(getattr(subtensor, m))
we also check fields and properties too. remove it pls
tests/unit_tests/test_subtensor.py
Outdated
m | ||
for m in dir(async_subtensor) | ||
if not m.startswith("_") and m not in excluded_async_subtensor_methods | ||
if not m.startswith("_") and m not in excluded_async_subtensor_methods and callable(getattr(async_subtensor, m)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same as above
tests/unit_tests/test_subtensor.py
Outdated
assert hasattr( | ||
async_subtensor, method | ||
), f"`Subtensor.{method}` not found as an attribute in `AsyncSubtensor` instance." | ||
# Ensure the attribute is actually callable (a method) | ||
assert callable( | ||
getattr(async_subtensor, method) | ||
), f"Attribute `AsyncSubtensor.{method}` is not callable." | ||
|
||
# Check if all relevant public methods in AsyncSubtensor exist in Subtensor | ||
for method in async_subtensor_method_names: | ||
assert hasattr( | ||
subtensor, method | ||
), f"`AsyncSubtensor.{method}` not found as an attribute in `Subtensor` instance." | ||
# Ensure the attribute is actually callable (a method) | ||
assert callable( | ||
getattr(subtensor, method) | ||
), f"Attribute `Subtensor.{method}` is not callable." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same reason as above to reverse it back
bittensor/core/subtensor.py
Outdated
) | ||
return None | ||
|
||
def get_subnet_reveal_period_epochs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have this methods already. pls remove
this is the reason you have this failed ci/circleci: lint-and-type-check
checkers
bittensor/core/subtensor.py:3520: error: Name "get_subnet_reveal_period_epochs" already defined on line 1657 [no-redef]
async def get_subnet_owner_hotkey( | ||
self, | ||
netuid: int, | ||
block: Optional[int] = None, | ||
block_hash: Optional[str] = None, | ||
reuse_block: bool = False, | ||
) -> Optional[str]: | ||
""" | ||
Retrieves the hotkey of the owner for a specific subnet asynchronously. | ||
|
||
Arguments: | ||
netuid (int): The unique identifier of the subnet. | ||
block (Optional[int]): The blockchain block number for the query. Do not specify if using block_hash or reuse_block. | ||
block_hash (Optional[str]): The hash of the blockchain block number for the query. Do not specify if using block or reuse_block. | ||
reuse_block (bool): Whether to reuse the last-used block hash. Do not set if using block_hash or block. | ||
|
||
Returns: | ||
Optional[str]: The SS58 address of the subnet owner's hotkey if the subnet exists, `None` otherwise. | ||
""" | ||
block_hash_ = await self.determine_block_hash(block, block_hash, reuse_block) | ||
|
||
if not await self.subnet_exists( | ||
netuid, block_hash=block_hash_, reuse_block=reuse_block | ||
): | ||
logging.debug(f"Subnet {netuid} does not exist.") | ||
return None | ||
|
||
owner_hotkey = await self.query_subtensor( | ||
name="SubnetOwnerHotkey", | ||
params=[netuid], | ||
block_hash=block_hash_, | ||
reuse_block=reuse_block, | ||
) | ||
|
||
if owner_hotkey is None: | ||
# Should not happen if subnet_exists check passes, but handle defensively | ||
return None | ||
|
||
# Check if the returned hotkey is valid (basic check) | ||
if is_valid_ss58_address(owner_hotkey): | ||
return owner_hotkey | ||
|
||
logging.error( | ||
f"Received invalid owner hotkey format for subnet {netuid}: {owner_hotkey}" | ||
) | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to arrange methods in alphabetical order by sections. Please place methods in the right place in the methods section. Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've double-checked, and the unit tests are already located in the correct alphabetical position within their section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've double-checked, and the unit tests are already located in the correct alphabetical position within their section
I meant new methods get_subnet_owner_hotkey
for Subtensor. Comment left to the method.
Unit tests look good. But there is an error in the test. One argument is missing - period = 5.
bittensor/core/subtensor.py
Outdated
def get_subnet_owner_hotkey( | ||
self, netuid: int, block: Optional[int] = None | ||
) -> Optional[str]: | ||
""" | ||
Retrieves the hotkey of the owner for a specific subnet. | ||
|
||
Arguments: | ||
netuid (int): The unique identifier of the subnet. | ||
block (Optional[int]): The blockchain block number for the query. | ||
|
||
Returns: | ||
Optional[str]: The SS58 address of the subnet owner's hotkey if the subnet exists, `None` otherwise. | ||
""" | ||
if not self.subnet_exists(netuid, block=block): | ||
logging.debug(f"Subnet {netuid} does not exist.") | ||
return None | ||
|
||
owner_hotkey = self.query_subtensor( # Use result directly | ||
name="SubnetOwnerHotkey", params=[netuid], block=block | ||
) | ||
|
||
if owner_hotkey is None: | ||
# Should not happen if subnet_exists check passes, but handle defensively | ||
return None | ||
|
||
# Check if the returned hotkey is valid (basic check) | ||
if is_valid_ss58_address(owner_hotkey): | ||
return owner_hotkey | ||
|
||
logging.debug( | ||
f"Received invalid owner hotkey format for subnet {netuid}: {owner_hotkey}" | ||
) | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to arrange methods in alphabetical order by sections. Please place methods in the right place in the methods section. Thank you
to avoid |
will continue work tommorow |
Hey @gap-editor any updates? |
hi. was little bit busy those days. i am going to take a look today. thanks
and sorry for this delay
|
I also have a question. could you please write down a list of tasks that I have to do? because due to my main job I am a bit out of context. i would be very grateful to you. |
I think you need to go through all my comments in this PR. By closing them accordingly you will reach your goal regarding this task. |
This pull request introduces a new public method,
get_subnet_owner_hotkey
, to theSubtensor
class located inbittensor/core/subtensor.py
.Motivation:
Currently, retrieving the hotkey of a subnet owner requires using the lower-level
query_subtensor
method directly, like so:This PR aims to provide a more convenient and explicit high-level interface for this common query, enhancing the usability of the SDK for developers.
Implementation Details:
get_subnet_owner_hotkey(self, netuid: int, block: Optional[int] = None) -> Optional[str]
to theSubtensor
class.netuid
(subnet unique identifier) and an optionalblock
number.self.subnet_exists(netuid, block=block)
to ensure the target subnet is valid before proceeding.self.query_subtensor
method to query theSubnetOwnerHotkey
storage item on the Subtensor module with the providednetuid
.bittensor.utils.is_valid_ss58_address()
to ensure the returned value is a valid SS58 address format.None
(e.g., if the subnet doesn't exist or the returned data is invalid).Subtensor
class, grouped near other subnet-related query methods likeget_subnet_hyperparameters
.Benefits:
Closes #2796.