bittensor
bittensor copied to clipboard
Balance class comparison methods mishandle different types and can lead to incorrect or exception-throwing behaviors
Describe the bug
The Balance class within the Bittensor project has several comparison dunder methods (__eq__, __ne__, __gt__, __lt__) designed to handle comparisons with instances of Balance itself, or numerical types (int, float). However, there are issues with how these methods handle different types and certain edge cases, leading to unhandled exceptions or incorrect comparison results. Here are the identified issues:
-
Equality Comparison (
__eq__): When comparing aBalanceinstance with a float that should be considered equal, the method incorrectly returnsFalsedue to casting issues and not accounting for floating-point representations. -
Non-Equality Comparison (
__ne__): ATypeErroris raised if the method is passed aBalanceobject that was initialized usingfrom_rao, which is inconsistent with the expected behavior of determining inequality. -
Greater Than Comparison (
__gt__): Sinceraois a float, casting it to an int for comparison can result in incorrect evaluations, especially when both compared values are less than 1 but not equal, leading to an incorrect outcome of the comparison. -
Less Than Comparison (
__lt__): Similar to__gt__, casting a float to an int when the value is less than 1 can lead to incorrect comparison outcomes, especially when both values are less than 1 but theBalanceinstance should be less than theother.
To Reproduce
- Create instances of the
Balanceclass with variousraovalues, including floats less than 1. - Attempt to compare these instances with floats, ints, and other
Balanceinstances using the__eq__,__ne__,__gt__, and__lt__methods. - Observe the inconsistencies and exceptions as described.
Expected behavior
The comparison methods should correctly handle comparisons with different types without raising unexpected exceptions. They should accurately reflect the numerical comparison between the rao value of a Balance instance and other numerical values or rao values of other Balance instances.
Actual Outcome: The methods currently can raise unhandled exceptions or return incorrect comparison results under certain conditions, as detailed above.
Screenshots
eq This will return False when it should return True if passed a float https://github.com/opentensor/bittensor/blob/c5dabe06a562184d9867c97e0d290e4b0ffc5941/bittensor/utils/balance.py#L107
ne This will raise a TypeError if passed a Balance object that was loaded with from_rao https://github.com/opentensor/bittensor/blob/c5dabe06a562184d9867c97e0d290e4b0ffc5941/bittensor/utils/balance.py#L112
gt Rao should already be a float. If the value <1 then this will cast to zero, resulting in an invalid > comparison if both values < 1 https://github.com/opentensor/bittensor/blob/c5dabe06a562184d9867c97e0d290e4b0ffc5941/bittensor/utils/balance.py#L107
lt If other is <1 and self.rao = <1 and < other, this will incorrectly cast other to 0 and return an invalid comparison result https://github.com/opentensor/bittensor/blob/c5dabe06a562184d9867c97e0d290e4b0ffc5941/bittensor/utils/balance.py#L121
Environment
Issue present in the current release and all previous versions.
Additional context
Proposed Fixes:
- Consider implementing a more robust type checking and conversion mechanism to handle floating-point comparisons accurately.
- Ensure that comparison methods return logically consistent results, especially when dealing with edge cases such as values less than 1.
- Introduce custom types for Rao and Tao to improve type safety, encapsulation, and validation. Custom types can offer clear semantics in the codebase, ensuring that operations and conversions involving Rao and Tao are handled consistently and correctly. With custom types, you can implement validation logic directly within the constructor or setter methods, and automatically handle conversion between these types and standard numerical values, ensuring that conversions are consistent and centralized and that that only valid values are assigned to Rao and Tao.