-
Notifications
You must be signed in to change notification settings - Fork 304
Update transaction_payment_wrapper: charge fees for hotkey from its coldkey #2570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devnet-ready
Are you sure you want to change the base?
Changes from all commits
f5ce6d8
fc1a548
385dd03
c4915c6
c1a5111
19dfc69
1c4fbb0
4fda83b
01113d4
a6c96bb
c876e11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -57,9 +57,12 @@ where | |||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| impl<T: Config + pallet_proxy::Config + pallet_utility::Config> ChargeTransactionPaymentWrapper<T> | ||||||||||||||||||
| impl<T: Config + pallet_proxy::Config + pallet_utility::Config + pallet_subtensor::Config> | ||||||||||||||||||
| ChargeTransactionPaymentWrapper<T> | ||||||||||||||||||
| where | ||||||||||||||||||
| RuntimeCallOf<T>: IsSubType<pallet_proxy::Call<T>> + IsSubType<pallet_utility::Call<T>>, | ||||||||||||||||||
| RuntimeCallOf<T>: IsSubType<pallet_proxy::Call<T>> | ||||||||||||||||||
| + IsSubType<pallet_utility::Call<T>> | ||||||||||||||||||
| + IsSubType<pallet_subtensor::Call<T>>, | ||||||||||||||||||
| RuntimeOriginOf<T>: AsSystemOriginSigner<AccountIdOf<T>> + Clone, | ||||||||||||||||||
| { | ||||||||||||||||||
| /// Extract (real, delegate, inner_call) from a `proxy` call. | ||||||||||||||||||
|
|
@@ -182,14 +185,28 @@ where | |||||||||||||||||
|
|
||||||||||||||||||
| common_real | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| fn extract_coldkey_fee_payer(origin: &RuntimeOriginOf<T>) -> Option<AccountIdOf<T>> { | ||||||||||||||||||
| let signer = origin.as_system_origin_signer()?; | ||||||||||||||||||
|
|
||||||||||||||||||
| pallet_subtensor::Pallet::<T>::maybe_coldkey_for_hotkey(signer) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| fn is_coldkey_fee_payer_eligible(call: &RuntimeCallOf<T>) -> bool { | ||||||||||||||||||
| matches!( | ||||||||||||||||||
| call.is_sub_type(), | ||||||||||||||||||
| Some(pallet_subtensor::Call::set_weights { .. }) | ||||||||||||||||||
| ) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| impl<T: Config + pallet_proxy::Config + pallet_utility::Config> | ||||||||||||||||||
| impl<T: Config + pallet_proxy::Config + pallet_utility::Config + pallet_subtensor::Config> | ||||||||||||||||||
| TransactionExtension<RuntimeCallOf<T>> for ChargeTransactionPaymentWrapper<T> | ||||||||||||||||||
| where | ||||||||||||||||||
| RuntimeCallOf<T>: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo> | ||||||||||||||||||
| + IsSubType<pallet_proxy::Call<T>> | ||||||||||||||||||
| + IsSubType<pallet_utility::Call<T>>, | ||||||||||||||||||
| + IsSubType<pallet_utility::Call<T>> | ||||||||||||||||||
| + IsSubType<pallet_subtensor::Call<T>>, | ||||||||||||||||||
| RuntimeOriginOf<T>: AsSystemOriginSigner<AccountIdOf<T>> | ||||||||||||||||||
| + Clone | ||||||||||||||||||
| + From<frame_system::RawOrigin<AccountIdOf<T>>>, | ||||||||||||||||||
|
|
@@ -230,6 +247,12 @@ where | |||||||||||||||||
| // Otherwise, the signer pays as usual. | ||||||||||||||||||
| let fee_origin = if let Some(real) = Self::extract_real_fee_payer(call, &origin) { | ||||||||||||||||||
| frame_system::RawOrigin::Signed(real).into() | ||||||||||||||||||
| } else if Self::is_coldkey_fee_payer_eligible(call) { | ||||||||||||||||||
| if let Some(coldkey) = Self::extract_coldkey_fee_payer(&origin) { | ||||||||||||||||||
| frame_system::RawOrigin::Signed(coldkey).into() | ||||||||||||||||||
| } else { | ||||||||||||||||||
| origin.clone() | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+250
to
+255
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't just have extract_coldkey_fee_payer returns None when not eligible directly in the same function to collapse both into one? Make things cleaner, wdyt?
Suggested change
|
||||||||||||||||||
| } else { | ||||||||||||||||||
| origin.clone() | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| #![allow(clippy::arithmetic_side_effects, clippy::unwrap_used)] | ||
|
|
||
| use { | ||
|
Comment on lines
+1
to
+3
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is subtensor pallet specific if we think about it. Maybe exposing the mock from the subtensor pallet make it easier? |
||
| frame_support::assert_ok, | ||
| node_subtensor_runtime::ExistentialDeposit, | ||
| node_subtensor_runtime::{BuildStorage, Runtime, RuntimeGenesisConfig, System}, | ||
| pallet_subtensor::{ | ||
| BurnHalfLife, BurnIncreaseMult, Error, FirstEmissionBlockNumber, Pallet as SubtensorPallet, | ||
| SubnetAlphaIn, SubnetAlphaInProvided, SubnetTAO, SubtokenEnabled, | ||
| }, | ||
| substrate_fixed::types::U64F64, | ||
| subtensor_runtime_common::{AccountId, AlphaBalance, NetUid, TaoBalance}, | ||
| }; | ||
|
|
||
| pub const ONE: [u8; 32] = [1_u8; 32]; | ||
| pub const TWO: [u8; 32] = [2_u8; 32]; | ||
| pub const THREE: [u8; 32] = [3_u8; 32]; | ||
| pub const FOUR_NO_BALANCE: [u8; 32] = [4_u8; 32]; | ||
|
|
||
| pub fn new_test_ext() -> sp_io::TestExternalities { | ||
| sp_tracing::try_init_simple(); | ||
| let amount = TaoBalance::from(1_000_000_000_000_u64); | ||
| let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig { | ||
| balances: pallet_balances::GenesisConfig { | ||
| balances: vec![ | ||
| (AccountId::from(ONE), amount), | ||
| (AccountId::from(TWO), amount), | ||
| (AccountId::from(THREE), amount), | ||
| ], | ||
| dev_accounts: None, | ||
| }, | ||
| ..Default::default() | ||
| } | ||
| .build_storage() | ||
| .unwrap() | ||
| .into(); | ||
| ext.execute_with(|| System::set_block_number(1)); | ||
| ext | ||
| } | ||
|
|
||
| pub fn add_network_disable_commit_reveal(netuid: NetUid, tempo: u16, _modality: u16) { | ||
| add_network(netuid, tempo, _modality); | ||
| SubtensorPallet::<Runtime>::set_commit_reveal_weights_enabled(netuid, false); | ||
| SubtensorPallet::<Runtime>::set_yuma3_enabled(netuid, false); | ||
| } | ||
|
|
||
| pub fn add_network(netuid: NetUid, tempo: u16, _modality: u16) { | ||
| SubtensorPallet::<Runtime>::init_new_network(netuid, tempo); | ||
| SubtensorPallet::<Runtime>::set_network_registration_allowed(netuid, true); | ||
| FirstEmissionBlockNumber::<Runtime>::insert(netuid, 1); | ||
| SubtokenEnabled::<Runtime>::insert(netuid, true); | ||
|
|
||
| // make interval 1 block so tests can register by stepping 1 block. | ||
| BurnHalfLife::<Runtime>::insert(netuid, 1); | ||
| BurnIncreaseMult::<Runtime>::insert(netuid, U64F64::from_num(1)); | ||
| } | ||
|
|
||
| pub(crate) fn setup_reserves(netuid: NetUid, tao: TaoBalance, alpha: AlphaBalance) { | ||
| SubnetTAO::<Runtime>::set(netuid, tao); | ||
| SubnetAlphaIn::<Runtime>::set(netuid, alpha); | ||
| } | ||
|
|
||
| pub fn register_ok_neuron( | ||
| netuid: NetUid, | ||
| hotkey_account_id: AccountId, | ||
| coldkey_account_id: AccountId, | ||
| _start_nonce: u64, | ||
| ) { | ||
| SubtensorPallet::<Runtime>::set_burn(netuid, TaoBalance::from(0)); | ||
| let reserve: u64 = 1_000_000_000_000; | ||
| let tao_reserve = SubnetTAO::<Runtime>::get(netuid); | ||
| let alpha_reserve = | ||
| SubnetAlphaIn::<Runtime>::get(netuid) + SubnetAlphaInProvided::<Runtime>::get(netuid); | ||
|
|
||
| if tao_reserve == 0.into() && alpha_reserve == 0.into() { | ||
| setup_reserves(netuid, reserve.into(), reserve.into()); | ||
| } | ||
|
|
||
| // Ensure coldkey has enough to pay the current burn AND is not fully drained to zero. | ||
| // This avoids ZeroBalanceAfterWithdrawn in burned_register. | ||
| let top_up_for_burn = |netuid: NetUid, cold: AccountId| { | ||
| let burn: TaoBalance = SubtensorPallet::<Runtime>::get_burn(netuid); | ||
| let burn_u64: TaoBalance = burn; | ||
|
|
||
| // Make sure something remains after withdrawal even if ED is 0 in tests. | ||
| let ed: TaoBalance = ExistentialDeposit::get(); | ||
| let min_remaining: TaoBalance = ed.max(1.into()); | ||
|
|
||
| // Small buffer for safety (fees / rounding / future changes). | ||
| let buffer: TaoBalance = 10.into(); | ||
|
|
||
| let min_balance_needed: TaoBalance = burn_u64 + min_remaining + buffer; | ||
|
|
||
| let bal: TaoBalance = SubtensorPallet::<Runtime>::get_coldkey_balance(&cold); | ||
| if bal < min_balance_needed { | ||
| SubtensorPallet::<Runtime>::add_balance_to_coldkey_account( | ||
| &cold, | ||
| min_balance_needed - bal, | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| top_up_for_burn(netuid, coldkey_account_id.clone()); | ||
|
|
||
| let origin = | ||
| <<Runtime as frame_system::Config>::RuntimeOrigin>::signed(coldkey_account_id.clone()); | ||
| let result = SubtensorPallet::<Runtime>::burned_register( | ||
| origin.clone(), | ||
| netuid, | ||
| hotkey_account_id.clone(), | ||
| ); | ||
|
|
||
| match result { | ||
| Ok(()) => { | ||
| // success | ||
| } | ||
| Err(e) | ||
| if e == Error::<Runtime>::TooManyRegistrationsThisInterval.into() | ||
| || e == Error::<Runtime>::NotEnoughBalanceToStake.into() | ||
| || e == Error::<Runtime>::ZeroBalanceAfterWithdrawn.into() => | ||
| { | ||
| // Re-top-up and retry once (burn can be state-dependent). | ||
| top_up_for_burn(netuid, coldkey_account_id.clone()); | ||
|
|
||
| assert_ok!(SubtensorPallet::<Runtime>::burned_register( | ||
| origin, | ||
| netuid, | ||
| hotkey_account_id.clone() | ||
| )); | ||
| } | ||
| Err(e) => { | ||
| panic!("Expected Ok(_). Got Err({e:?})"); | ||
| } | ||
| } | ||
| SubtensorPallet::<Runtime>::set_burn(netuid, TaoBalance::from(0)); | ||
| log::info!( | ||
| "Register ok neuron: netuid: {netuid:?}, coldkey: {coldkey_account_id:?}, hotkey: {hotkey_account_id:?}" | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| mod common; | ||
| use common::new_test_ext; | ||
| use common::*; | ||
| use frame_support::assert_ok; | ||
| use frame_support::dispatch::GetDispatchInfo; | ||
| use frame_support::sp_runtime::traits::DispatchTransaction; | ||
| use node_subtensor_runtime::{ | ||
| Runtime, RuntimeCall, RuntimeOrigin, | ||
| transaction_payment_wrapper::ChargeTransactionPaymentWrapper, | ||
| }; | ||
| use pallet_subtensor::Pallet as SubtensorPallet; | ||
| use subtensor_runtime_common::{AccountId, NetUid}; | ||
|
|
||
| // SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --package node-subtensor-runtime --test subtensor_weights -- set_weights_fees_payed_by_coldkey --exact --nocapture | ||
| #[test] | ||
| fn set_weights_fees_payed_by_coldkey() { | ||
| new_test_ext().execute_with(|| { | ||
| let hotkey = AccountId::from(common::FOUR_NO_BALANCE); | ||
| let coldkey = AccountId::from(common::TWO); | ||
| let netuid0 = NetUid::from(1); | ||
| let netuid1 = NetUid::from(2); | ||
|
|
||
| SubtensorPallet::<Runtime>::set_weights_set_rate_limit(netuid0, 0); | ||
|
|
||
| add_network_disable_commit_reveal(netuid0, 1, 0); | ||
| add_network_disable_commit_reveal(netuid1, 1, 0); | ||
| register_ok_neuron(netuid0, hotkey.clone(), coldkey.clone(), 2143124); | ||
| register_ok_neuron(netuid1, hotkey.clone(), coldkey.clone(), 3124124); | ||
|
|
||
| let hotkey_balance_before = pallet_balances::Pallet::<Runtime>::free_balance(&hotkey); | ||
| let coldkey_balance_before = pallet_balances::Pallet::<Runtime>::free_balance(&coldkey); | ||
|
|
||
| let weights_keys: Vec<u16> = vec![0]; | ||
| let weight_values: Vec<u16> = vec![1]; | ||
|
|
||
| let call = RuntimeCall::SubtensorModule(pallet_subtensor::Call::set_weights { | ||
| netuid: netuid0, | ||
| dests: weights_keys, | ||
| weights: weight_values, | ||
| version_key: 0, | ||
| }); | ||
|
|
||
| let info = call.get_dispatch_info(); | ||
| let ext = ChargeTransactionPaymentWrapper::<Runtime>::new(0.into()); | ||
| assert_ok!(ext.dispatch_transaction( | ||
| RuntimeOrigin::signed(hotkey.clone()).into(), | ||
| call, | ||
| &info, | ||
| 0, | ||
| 0, | ||
| )); | ||
|
|
||
| let hotkey_balance_after = pallet_balances::Pallet::<Runtime>::free_balance(&hotkey); | ||
| let coldkey_balance_after = pallet_balances::Pallet::<Runtime>::free_balance(&coldkey); | ||
|
|
||
| assert_eq!(hotkey_balance_before, hotkey_balance_after); | ||
| assert!(coldkey_balance_after < coldkey_balance_before); // Fee paid by coldkey | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is super weird that there is a
DefaultAccountfor aOwner, it should probably be anOptionQuerybut that is something for another discussion