Debloat pallet subtensor

[ales-otf]

Description

Warning

While, the diff is big and might be terrifying to review, the PR is organized in a way it can be easily reviewed. Please, read the description.

This is a cleanup of pallet-subtensor. It removes the methods, which complicate the code reading and debugging, while not encapsulating any complex logic and/or providing a public API (in the sense of usage outside the library).

Also, the PR is organized in the way, you can easily review each method change. There is a single method refactor per commit. So if you're not certain in a method deletion (listed below), check a commit which contains the change.

Refactoring Method

All public methods of pallet_subtensor::Pallet were collected from cargo-doc tool generated docs. Then a log of references for each method was created using this script:

#!/bin/bash

file="$1"
src="$2"

while IFS= read -r line
do
  echo "$line usages:"
  grep -rIn --include="*.rs" "$line" "$src"
  printf "\n"
done < "$file"

Each method from this log was checked manually for its usage within the whole code base and a list of methods, which most likely could be removed/refactored was created.

After each remove, cargo test --worskpace --all-features was running, to make it sure everything worked as expected.

The major part of the whole refactor was the methods, which are used storage API under the hood. A special script to refactor them was created:

#!/bin/bash

set -eou pipefail

method_name=$1
storage_name=$2
storage_method=$3
args_num=$#

check_args() {
    if [ "$args_num" -ne 3 ]; then
        echo "Usage: $0 <method_name> <storage_name> <storage_method>"
        exit 1
    fi
}

replace() {
    find pallets -name '*.rs' -exec sed -i '' "s/Self::${method_name}(/${storage_name}::<T>::${storage_method}(/g" {} +
    find pallets -name '*.rs' -exec sed -i '' "s/SubtensorModule::${method_name}(/${storage_name}::<Test>::${storage_method}(/g" {} +
    find pallets -name '*.rs' -exec sed -i '' "s/Subtensor::<T>::${method_name}(/${storage_name}::<T>::${storage_method}(/g" {} +
    find pallets -name '*.rs' -exec sed -i '' "s/pallet_subtensor::Pallet::<T>::${method_name}(/pallet_subtensor::${storage_name}::<T>::${storage_method}(/g" {} +
}

check() {
    cargo fix --all --allow-dirty
    cargo fmt --all
    cargo clippy --workspace --all-features --all-targets --fix --allow-dirty
    cargo test --workspace --all-features
}

commit_changes() {
    git add .
    git commit -m "Remove ${method_name} from pallet-subtensor::Pallet"
}

check_args
replace
check
commit_changes

The method definitions were removed manually. Also, other cases were refactored manually. That means, that the most attention during the review should be given to the methods, which were refactored by other reasons. As methods in this group were refactored in mostly automated fashion.

Stats

Each removed method was collected into a log and grouped by the reason of a remove/refactor.

Totally 132 methods have removed/refactored.

Here is the list of reasons, with the count and the percentage of the total refactoring cases:

Reason description	Count	Percentage
storage API used via method (for example, Self::get_foo instead of Foo::<T>::get)	113	86%
dead code	8	6%
trivial method, which is public API, but used only in tests and/or in a single place/privately	5	4%
one-liner boolean expression, used privately	3	2%
other - see below	3	2%

Log

Storage API used via method (for example, Self::get_foo instead of Foo::<T>::get)

The reason for this refactor is getters/setters make debugging and reading of the code harder, because you should check implementations and switch context. Also, most of the time they are not shorter than using storage API.

This refactoring affects not only the library, but dependent code within the whole code base as well. This is because currently all storage types from pallet-subtensor are public. But, within the whole code base only pallet-admin-utils used this API. Also, a single storage read were in the metagraph precompile. You can confirm it by checking the list of changed files. Anyway, it would be better to make these types private to prevent side effect mutations and do use getters/setters in case a storage should be mutated/read outside. But it's not in the scope of this PR and should be done in another refactor pass with other types and methods visibility review.

Methods refactored:

• delegate_hotkey
• get_active
• get_activity_cutoff
• get_adjustment_alpha
• get_adjustment_interval
• get_all_staked_hotkeys
• get_alpha_values
• get_axon_info
• get_blocks_since_last_step
• get_bonds_moving_average
• get_burn_as_u64
• get_burn_registrations_this_interval
• get_childkey_take
• get_children
• get_coldkey_for_hotkey
• get_commit_reveal_weights_enabled
• get_consensus
• get_default_childkey_take
• get_default_delegate_take
• get_difficulty
• get_difficulty_as_u64
• get_dividends
• get_emission
• get_emission_value
• get_hotkey_emission_tempo
• get_hotkey_take
• get_immunity_period
• get_kappa
• get_last_adjustment_block
• get_last_tx_block
• get_last_tx_block_delegate_take
• get_last_update
• get_liquid_alpha_enabled
• get_lock_reduction_interval
• get_max_allowed_uids
• get_max_allowed_validators
• get_max_burn_as_u64
• get_max_childkey_take
• get_max_difficulty
• get_max_root_validators
• get_max_subnets
• get_max_weight_limit
• get_min_allowed_weights
• get_min_burn_as_u64
• get_min_childkey_take
• get_min_delegate_take
• get_min_difficulty
• get_network_immunity_period
• get_network_last_lock
• get_network_max_stake
• get_network_min_lock
• get_network_registered_block
• get_network_registration_allowed
• get_neuron_block_at_registration
• get_nominator_min_required_stake
• get_num_subnets
• get_owned_hotkeys
• get_owning_coldkey_for_hotkey
• get_parents
• get_pending_emission
• get_pending_hotkey_emission
• get_pow_registrations_this_interval
• get_prometheus_info
• get_pruning_score
• get_rank
• get_rao_recycled
• get_registrations_this_block
• get_registrations_this_interval
• get_reveal_period
• get_rho
• get_scaling_law_power
• get_serving_rate_limit
• get_stake_for_coldkey_and_hotkey
• get_stake_threshold
• get_subnet_emission_value
• get_subnet_locked_balance
• get_subnet_owner
• get_subnet_owner_cut
• get_subnetwork_n
• get_target_registrations_per_interval
• get_target_stakes_per_interval
• get_tempo
• get_total_issuance
• get_total_stake
• get_total_stake_for_coldkey
• get_total_stake_for_hotkey
• get_tx_childkey_take_rate_limit
• get_tx_delegate_take_rate_limit
• get_tx_rate_limit
• get_validator_permit
• get_validator_trust
• get_weights_set_rate_limit
• get_weights_version_key
• hotkey_account_exists
• hotkey_is_delegate
• if_subnet_exist
• is_uid_exist_on_network
• set_blocks_since_last_step
• set_burn
• set_burn_registrations_this_interval
• set_commit_reveal_weights_enabled
• set_last_adjustment_block
• set_last_mechanism_step_block
• set_last_tx_block
• set_last_tx_block_delegate_take
• set_liquid_alpha_enabled
• set_network_last_lock
• set_nominator_min_required_stake
• set_pow_registrations_this_interval
• set_registrations_this_block
• set_registrations_this_interval
• set_stake_interval
• set_subnet_locked_balance

Dead code

• check_allowed_register
• do_set_senate_required_stake_perc
• get_float_rho
• get_incentive
• get_last_tx_block_childkey_take
• get_max_delegate_take
• get_trust
• set_senate_required_stake_perc

Trivial methods, which is public API, but used only in tests and/or in a single place/privately

• decrease_stake_on_hotkey_account
• decrease_total_stake
• increase_total_stake
• has_enough_stake
• is_senate_member

One-liner boolean expression, used privately

Methods, which was a simple single line boolean expression. They used in a few places only within the library.

• check_len_uids_within_allowed
• check_validator_permit
• uids_match_values

Other

• get_root_netuid - refactored to associated constant
• get_key_swap_cost - used privately, replaced with the const getter
• get_rate_limit_on_subnet - just recalls get_rate_limit method (_netuid
  parameter is not used)

Related Issue(s)

• Part of Clean up/refactor Pallet type from the Subtensor module #983

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Other (please describe):

Breaking Change

If this PR introduces a breaking change, please provide a detailed description
of the impact and the migration path for existing applications.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have run cargo fmt and cargo clippy to ensure my code is formatted and linted correctly
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules