Fix: burned_register supports root subnet (netuid=0 param)#2732
Conversation
thewhaleking
left a comment
There was a problem hiding this comment.
Overall LGTM, just fix the gather catch
| @@ -2901,6 +2901,14 @@ async def burned_register( | |||
| bool: ``True`` if the registration is successful, False otherwise. | |||
| """ | |||
| async with self: | |||
There was a problem hiding this comment.
I know this is unrelated to this PR, but why do we do this?
There was a problem hiding this comment.
probably to make sure Subtensor/Substrate is properly initialized?
| try: | ||
| recycle_call, balance = await asyncio.gather( | ||
| subtensor.get_hyperparameter( | ||
| param_name="Burn", | ||
| netuid=netuid, | ||
| block_hash=block_hash, | ||
| ), | ||
| subtensor.get_balance( | ||
| wallet.coldkeypub.ss58_address, | ||
| block_hash=block_hash, | ||
| ), | ||
| ) | ||
| except TypeError as e: |
There was a problem hiding this comment.
This is actually a fairly difficult one. asyncio.gather returns errors in an odd non-standard way, where doing it like this can usually catch fine, but sometimes will not. You'll instead want to use asyncio.gather(*cos, return_exceptions=True) and then check to see if the returns are the exception type. It's not great, but it's the spec.
There was a problem hiding this comment.
I don't really understand. Can you provide an example of "non-catchable" exception thrown from asyncio.gather?
There was a problem hiding this comment.
I think TypeError very likely can be safely removed anyway (I've copied it from existing root_register method code) as it seems to be a leftover from this: 835dfdb#diff-87f2770377552e515f60ce0d2e31a3a9bb129bffa240ab2133e3233d3e22f6c3R1141-R1145
#2716