bittensor icon indicating copy to clipboard operation
bittensor copied to clipboard

Balance class comparison methods mishandle different types and can lead to incorrect or exception-throwing behaviors

Open wolfentensor opened this issue 1 year ago • 0 comments

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:

  1. Equality Comparison (__eq__): When comparing a Balance instance with a float that should be considered equal, the method incorrectly returns False due to casting issues and not accounting for floating-point representations.

  2. Non-Equality Comparison (__ne__): A TypeError is raised if the method is passed a Balance object that was initialized using from_rao, which is inconsistent with the expected behavior of determining inequality.

  3. Greater Than Comparison (__gt__): Since rao is 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.

  4. 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 the Balance instance should be less than the other.

To Reproduce

  1. Create instances of the Balance class with various rao values, including floats less than 1.
  2. Attempt to compare these instances with floats, ints, and other Balance instances using the __eq__, __ne__, __gt__, and __lt__ methods.
  3. 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.

wolfentensor avatar Apr 05 '24 09:04 wolfentensor