bittensor icon indicating copy to clipboard operation
bittensor copied to clipboard

Incorrect Scope of payment_info in get_transfer_fee Method and issue with Balance Class Design

Open wolfentensor opened this issue 1 year ago • 0 comments

Describe the bug

The get_transfer_fee method within the Subtensor class in the Bittensor project exhibits multiple significant issues, particularly in handling different types of token values (int, float, Balance) and in its dependency on the Balance class design. The method's type hint suggests that it can accept a Balance object as the value argument. However, the method lacks explicit handling for Balance objects, only directly managing int and float types. This oversight can lead to situations where Balance objects are passed to the method but not correctly processed, potentially causing runtime errors or incorrect fee calculations.

Moreover, the Balance class, which is central to financial operations on the Bittensor network, contains recursive logic that might result in unexpected behaviors under certain conditions. The class aims to represent financial values in both rao (int) and tao (float) but lacks clear documentation and handling for cases when neither type matches the input. Furthermore, the presence of redundant definitions for comparison and conversion dunder methods (e.g., __int__, __float__) could lead to confusion and errors in financial computations, which require high precision and reliability.

Affected Components:

  • get_transfer_fee method in the Subtensor class.
  • Balance class used for financial transactions in the Bittensor network.

Impact:

  • Runtime Errors and Incorrect Calculations: The method may produce errors or incorrect calculations when handling Balance objects due to missing logic.
  • Unexpected Behavior in Financial Transactions: The Balance class's recursive calculations and redundant definitions could cause unexpected results in financial transactions, leading to potential inaccuracies in value representations and comparisons.

To Reproduce

  1. Invoke the get_transfer_fee method with a Balance object as the value argument and observe missing handling logic.
  2. Examine the Balance class's implementation, noting the recursive calculations in static methods and the redundant dunder method definitions.

Expected behavior

  • The get_transfer_fee method should explicitly handle Balance objects, ensuring that all types mentioned in the type hint are correctly processed.
  • The Balance class should be designed to prevent recursion and redundant definitions, ensuring clear, predictable behavior and precision in financial computations.

Screenshots

subtensor.py - Lines #1117 - 1162

Environment

Issue present in the current release and all previous versions.

Additional context

Suggested Fix:

  1. For get_transfer_fee:

    • Implement explicit handling for Balance objects within the method, ensuring that fees are accurately calculated regardless of the value type passed.
  2. For the Balance class:

    • Refactor the class to eliminate unnecessary recursion in its static methods and to ensure that each dunder method is defined only once. This refactoring should include clear documentation on how financial values are converted and compared within the class.

Example Fixes:

  • In get_transfer_fee method:
# Add handling for Balance objects
if isinstance(value, Balance):
    transfer_balance = value
else:
    # Existing handling for int and float

wolfentensor avatar Apr 05 '24 09:04 wolfentensor