subtensor
subtensor copied to clipboard
Debloat pallet subtensor
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_hotkeyget_activeget_activity_cutoffget_adjustment_alphaget_adjustment_intervalget_all_staked_hotkeysget_alpha_valuesget_axon_infoget_blocks_since_last_stepget_bonds_moving_averageget_burn_as_u64get_burn_registrations_this_intervalget_childkey_takeget_childrenget_coldkey_for_hotkeyget_commit_reveal_weights_enabledget_consensusget_default_childkey_takeget_default_delegate_takeget_difficultyget_difficulty_as_u64get_dividendsget_emissionget_emission_valueget_hotkey_emission_tempoget_hotkey_takeget_immunity_periodget_kappaget_last_adjustment_blockget_last_tx_blockget_last_tx_block_delegate_takeget_last_updateget_liquid_alpha_enabledget_lock_reduction_intervalget_max_allowed_uidsget_max_allowed_validatorsget_max_burn_as_u64get_max_childkey_takeget_max_difficultyget_max_root_validatorsget_max_subnetsget_max_weight_limitget_min_allowed_weightsget_min_burn_as_u64get_min_childkey_takeget_min_delegate_takeget_min_difficultyget_network_immunity_periodget_network_last_lockget_network_max_stakeget_network_min_lockget_network_registered_blockget_network_registration_allowedget_neuron_block_at_registrationget_nominator_min_required_stakeget_num_subnetsget_owned_hotkeysget_owning_coldkey_for_hotkeyget_parentsget_pending_emissionget_pending_hotkey_emissionget_pow_registrations_this_intervalget_prometheus_infoget_pruning_scoreget_rankget_rao_recycledget_registrations_this_blockget_registrations_this_intervalget_reveal_periodget_rhoget_scaling_law_powerget_serving_rate_limitget_stake_for_coldkey_and_hotkeyget_stake_thresholdget_subnet_emission_valueget_subnet_locked_balanceget_subnet_ownerget_subnet_owner_cutget_subnetwork_nget_target_registrations_per_intervalget_target_stakes_per_intervalget_tempoget_total_issuanceget_total_stakeget_total_stake_for_coldkeyget_total_stake_for_hotkeyget_tx_childkey_take_rate_limitget_tx_delegate_take_rate_limitget_tx_rate_limitget_validator_permitget_validator_trustget_weights_set_rate_limitget_weights_version_keyhotkey_account_existshotkey_is_delegateif_subnet_existis_uid_exist_on_networkset_blocks_since_last_stepset_burnset_burn_registrations_this_intervalset_commit_reveal_weights_enabledset_last_adjustment_blockset_last_mechanism_step_blockset_last_tx_blockset_last_tx_block_delegate_takeset_liquid_alpha_enabledset_network_last_lockset_nominator_min_required_stakeset_pow_registrations_this_intervalset_registrations_this_blockset_registrations_this_intervalset_stake_intervalset_subnet_locked_balance
Dead code
check_allowed_registerdo_set_senate_required_stake_percget_float_rhoget_incentiveget_last_tx_block_childkey_takeget_max_delegate_takeget_trustset_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_accountdecrease_total_stakeincrease_total_stakehas_enough_stakeis_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_allowedcheck_validator_permituids_match_values
Other
get_root_netuid- refactored to associated constantget_key_swap_cost- used privately, replaced with the const getterget_rate_limit_on_subnet- just recallsget_rate_limitmethod (_netuidparameter is not used)
Related Issue(s)
- Part of #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
- [X] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [X] I have run
cargo fmtandcargo clippyto ensure my code is formatted and linted correctly - [ ] I have made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [X] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules