From ea83f43f11527e2826a2d653c43e3662cb49a51e Mon Sep 17 00:00:00 2001 From: gus-opentensor <158077861+gus-opentensor@users.noreply.github.com> Date: Wed, 12 Jun 2024 19:27:00 -0400 Subject: [PATCH] Merge pull request #2016 from opentensor/fix/allow-unstake-below-network-min [Fix] Allow unstake below network min --- bittensor/extrinsics/registration.py | 4 + bittensor/extrinsics/staking.py | 50 +++- bittensor/extrinsics/unstaking.py | 41 ++- tests/e2e_tests/multistep/test_dendrite.py | 2 +- tests/e2e_tests/multistep/test_incentive.py | 4 +- .../weights/test_commit_weights.py | 2 +- tests/integration_tests/test_cli.py | 252 +++++++++++++++--- tests/unit_tests/extrinsics/test_staking.py | 18 +- tests/unit_tests/extrinsics/test_unstaking.py | 20 +- 9 files changed, 319 insertions(+), 74 deletions(-) diff --git a/bittensor/extrinsics/registration.py b/bittensor/extrinsics/registration.py index 887dae8345..e82add8383 100644 --- a/bittensor/extrinsics/registration.py +++ b/bittensor/extrinsics/registration.py @@ -462,6 +462,8 @@ def run_faucet_extrinsic( if attempts == max_allowed_attempts: raise MaxAttemptsException attempts += 1 + # Wait a bit before trying again + time.sleep(1) # Successful registration else: @@ -473,6 +475,8 @@ def run_faucet_extrinsic( if successes == 3: raise MaxSuccessException + + attempts = 1 # Reset attempts on success successes += 1 except KeyboardInterrupt: diff --git a/bittensor/extrinsics/staking.py b/bittensor/extrinsics/staking.py index 44a509eae8..298bb1f0d3 100644 --- a/bittensor/extrinsics/staking.py +++ b/bittensor/extrinsics/staking.py @@ -19,10 +19,34 @@ import bittensor from rich.prompt import Confirm from time import sleep -from typing import List, Union, Optional +from typing import List, Union, Optional, Tuple from bittensor.utils.balance import Balance +def _check_threshold_amount( + subtensor: "bittensor.subtensor", stake_balance: Balance +) -> Tuple[bool, Balance]: + """ + Checks if the new stake balance will be above the minimum required stake threshold. + + Args: + stake_balance (Balance): + the balance to check for threshold limits. + + Returns: + success, threshold (bool, Balance): + ``true`` if the staking balance is above the threshold, or ``false`` if the + staking balance is below the threshold. + The threshold balance required to stake. + """ + min_req_stake: Balance = subtensor.get_minimum_required_stake() + + if min_req_stake > stake_balance: + return False, min_req_stake + else: + return True, min_req_stake + + def add_stake_extrinsic( subtensor: "bittensor.subtensor", wallet: "bittensor.wallet", @@ -91,6 +115,9 @@ def add_stake_extrinsic( coldkey_ss58=wallet.coldkeypub.ss58_address, hotkey_ss58=hotkey_ss58 ) + # Grab the existential deposit. + existential_deposit = subtensor.get_existential_deposit() + # Convert to bittensor.Balance if amount is None: # Stake it all. @@ -100,9 +127,10 @@ def add_stake_extrinsic( else: staking_balance = amount - # Remove existential balance to keep key alive. - if staking_balance > bittensor.Balance.from_rao(1000): - staking_balance = staking_balance - bittensor.Balance.from_rao(1000) + # Leave existential balance to keep key alive. + if staking_balance > old_balance - existential_deposit: + # If we are staking all, we need to leave at least the existential deposit. + staking_balance = old_balance - existential_deposit else: staking_balance = staking_balance @@ -115,6 +143,18 @@ def add_stake_extrinsic( ) return False + # If nominating, we need to check if the new stake balance will be above the minimum required stake threshold. + if not own_hotkey: + new_stake_balance = old_stake + staking_balance + is_above_threshold, threshold = _check_threshold_amount( + subtensor, new_stake_balance + ) + if not is_above_threshold: + bittensor.__console__.print( + f":cross_mark: [red]New stake balance of {new_stake_balance} is below the minimum required nomination stake threshold {threshold}.[/red]" + ) + return False + # Ask before moving on. if prompt: if not own_hotkey: @@ -167,7 +207,7 @@ def add_stake_extrinsic( block = subtensor.get_current_block() new_stake = subtensor.get_stake_for_coldkey_and_hotkey( coldkey_ss58=wallet.coldkeypub.ss58_address, - hotkey_ss58=wallet.hotkey.ss58_address, + hotkey_ss58=hotkey_ss58, block=block, ) # Get current stake diff --git a/bittensor/extrinsics/unstaking.py b/bittensor/extrinsics/unstaking.py index cf47b07928..105bb145b9 100644 --- a/bittensor/extrinsics/unstaking.py +++ b/bittensor/extrinsics/unstaking.py @@ -72,13 +72,13 @@ def __do_remove_stake_single( def check_threshold_amount( - subtensor: "bittensor.subtensor", unstaking_balance: Balance + subtensor: "bittensor.subtensor", stake_balance: Balance ) -> bool: """ - Checks if the unstaking amount is above the threshold or 0 + Checks if the remaining stake balance is above the minimum required stake threshold. Args: - unstaking_balance (Balance): + stake_balance (Balance): the balance to check for threshold limits. Returns: @@ -88,9 +88,9 @@ def check_threshold_amount( """ min_req_stake: Balance = subtensor.get_minimum_required_stake() - if min_req_stake > unstaking_balance > 0: + if min_req_stake > stake_balance > 0: bittensor.__console__.print( - f":cross_mark: [red]Unstaking balance of {unstaking_balance} less than minimum of {min_req_stake} TAO[/red]" + f":cross_mark: [yellow]Remaining stake balance of {stake_balance} less than minimum of {min_req_stake} TAO[/yellow]" ) return False else: @@ -141,6 +141,9 @@ def unstake_extrinsic( coldkey_ss58=wallet.coldkeypub.ss58_address, hotkey_ss58=hotkey_ss58 ) + hotkey_owner = subtensor.get_hotkey_owner(hotkey_ss58) + own_hotkey: bool = wallet.coldkeypub.ss58_address == hotkey_owner + # Convert to bittensor.Balance if amount is None: # Unstake it all. @@ -160,10 +163,14 @@ def unstake_extrinsic( ) return False - if not check_threshold_amount( - subtensor=subtensor, unstaking_balance=unstaking_balance + # If nomination stake, check threshold. + if not own_hotkey and not check_threshold_amount( + subtensor=subtensor, stake_balance=(stake_on_uid - unstaking_balance) ): - return False + bittensor.__console__.print( + f":warning: [yellow]This action will unstake the entire staked balance![/yellow]" + ) + unstaking_balance = stake_on_uid # Ask before moving on. if prompt: @@ -300,6 +307,7 @@ def unstake_multiple_extrinsic( wallet.coldkey old_stakes = [] + own_hotkeys = [] with bittensor.__console__.status( ":satellite: Syncing with chain: [white]{}[/white] ...".format( subtensor.network @@ -313,9 +321,12 @@ def unstake_multiple_extrinsic( ) # Get stake on hotkey. old_stakes.append(old_stake) # None if not registered. + hotkey_owner = subtensor.get_hotkey_owner(hotkey_ss58) + own_hotkeys.append(wallet.coldkeypub.ss58_address == hotkey_owner) + successful_unstakes = 0 - for idx, (hotkey_ss58, amount, old_stake) in enumerate( - zip(hotkey_ss58s, amounts, old_stakes) + for idx, (hotkey_ss58, amount, old_stake, own_hotkey) in enumerate( + zip(hotkey_ss58s, amounts, old_stakes, own_hotkeys) ): # Covert to bittensor.Balance if amount is None: @@ -336,10 +347,14 @@ def unstake_multiple_extrinsic( ) continue - if not check_threshold_amount( - subtensor=subtensor, unstaking_balance=unstaking_balance + # If nomination stake, check threshold. + if not own_hotkey and not check_threshold_amount( + subtensor=subtensor, stake_balance=(stake_on_uid - unstaking_balance) ): - return False + bittensor.__console__.print( + f":warning: [yellow]This action will unstake the entire staked balance![/yellow]" + ) + unstaking_balance = stake_on_uid # Ask before moving on. if prompt: diff --git a/tests/e2e_tests/multistep/test_dendrite.py b/tests/e2e_tests/multistep/test_dendrite.py index 8f7336e1de..6abde7464d 100644 --- a/tests/e2e_tests/multistep/test_dendrite.py +++ b/tests/e2e_tests/multistep/test_dendrite.py @@ -90,7 +90,7 @@ async def test_dendrite(local_chain): metagraph = bittensor.metagraph(netuid=1, network="ws://localhost:9945") neuron = metagraph.neurons[0] # assert stake is 10000 - assert neuron.stake.tao == 9999.999999 + assert neuron.stake.tao == 10_000.0 # assert neuron is not validator assert neuron.active is True diff --git a/tests/e2e_tests/multistep/test_incentive.py b/tests/e2e_tests/multistep/test_incentive.py index d4605faa6a..ea5809dd7f 100644 --- a/tests/e2e_tests/multistep/test_incentive.py +++ b/tests/e2e_tests/multistep/test_incentive.py @@ -252,7 +252,7 @@ async def validator_write_output(stream): alice_neuron = metagraph.neurons[0] assert alice_neuron.validator_permit is False assert alice_neuron.dividends == 0 - assert alice_neuron.stake.tao == 9999.999999 + assert alice_neuron.stake.tao == 10_000.0 assert alice_neuron.validator_trust == 0 # wait until 360 blocks pass (subnet tempo) @@ -287,7 +287,7 @@ async def validator_write_output(stream): alice_neuron = metagraph.neurons[0] assert alice_neuron.validator_permit is True assert alice_neuron.dividends == 1 - assert alice_neuron.stake.tao == 9999.999999 + assert alice_neuron.stake.tao == 10_000.0 assert alice_neuron.validator_trust == 1 diff --git a/tests/e2e_tests/subcommands/weights/test_commit_weights.py b/tests/e2e_tests/subcommands/weights/test_commit_weights.py index 368775204f..4c719b0ebd 100644 --- a/tests/e2e_tests/subcommands/weights/test_commit_weights.py +++ b/tests/e2e_tests/subcommands/weights/test_commit_weights.py @@ -63,7 +63,7 @@ def test_commit_and_reveal_weights(local_chain): "--wallet.path", "/tmp/btcli-wallet2", "--amount", - "999998998", + "100000", ], ) diff --git a/tests/integration_tests/test_cli.py b/tests/integration_tests/test_cli.py index aa019c4178..6fe1acf3bc 100644 --- a/tests/integration_tests/test_cli.py +++ b/tests/integration_tests/test_cli.py @@ -782,72 +782,123 @@ def test_unstake_with_thresholds(self, _): config.no_prompt = True # as the minimum required stake may change, this method allows us to dynamically # update the amount in the mock without updating the tests - config.amount = Balance.from_rao(_subtensor_mock.min_required_stake() - 1) - config.wallet.name = "fake_wallet" - config.hotkeys = ["hk0", "hk1", "hk2"] + min_stake: Balance = _subtensor_mock.get_minimum_required_stake() + # Must be a float + config.amount = min_stake.tao # Unstake below the minimum required stake + wallet_names = ["w0", "w1", "w2"] config.all_hotkeys = False # Notice no max_stake specified mock_stakes: Dict[str, Balance] = { - "hk0": Balance.from_float(10.0), - "hk1": Balance.from_float(11.1), - "hk2": Balance.from_float(12.2), + "w0": 2 * min_stake - 1, # remaining stake will be below the threshold + "w1": 2 * min_stake - 2, + "w2": 2 * min_stake - 5, } - mock_coldkey_kp = _get_mock_keypair(0, self.id()) - mock_wallets = [ SimpleNamespace( - name=config.wallet.name, - coldkey=mock_coldkey_kp, - coldkeypub=mock_coldkey_kp, - hotkey_str=hk, - hotkey=_get_mock_keypair(idx + 100, self.id()), + name=wallet_name, + coldkey=_get_mock_keypair(idx, self.id()), + coldkeypub=_get_mock_keypair(idx, self.id()), + hotkey_str="hk{}".format(idx), # doesn't matter + hotkey=_get_mock_keypair(idx + 100, self.id()), # doesn't matter ) - for idx, hk in enumerate(config.hotkeys) + for idx, wallet_name in enumerate(wallet_names) ] - # Register mock wallets and give them stakes + delegate_hotkey = mock_wallets[0].hotkey.ss58_address - for wallet in mock_wallets: - _ = _subtensor_mock.force_register_neuron( - netuid=1, - hotkey=wallet.hotkey.ss58_address, - coldkey=wallet.coldkey.ss58_address, - stake=mock_stakes[wallet.hotkey_str].rao, - ) + # Register mock neuron, only for w0 + _ = _subtensor_mock.force_register_neuron( + netuid=1, + hotkey=delegate_hotkey, + coldkey=mock_wallets[0].coldkey.ss58_address, + stake=mock_stakes["w0"], + ) - cli = bittensor.cli(config) + # Become a delegate + _ = _subtensor_mock.nominate( + wallet=mock_wallets[0], + ) + + # Stake to the delegate with the other coldkeys + for wallet in mock_wallets[1:]: + # Give balance + _ = _subtensor_mock.force_set_balance( + ss58_address=wallet.coldkeypub.ss58_address, + balance=( + mock_stakes[wallet.name] + _subtensor_mock.get_existential_deposit() + ).tao + + 1.0, + ) + _ = _subtensor_mock.add_stake( + wallet=wallet, + hotkey_ss58=delegate_hotkey, + amount=mock_stakes[wallet.name], + ) def mock_get_wallet(*args, **kwargs): - if kwargs.get("hotkey"): + if kwargs.get("config") and kwargs["config"].get("wallet"): for wallet in mock_wallets: - if wallet.hotkey_str == kwargs.get("hotkey"): + if wallet.name == kwargs["config"].wallet.name: return wallet - else: - return mock_wallets[0] with patch("bittensor.wallet") as mock_create_wallet: mock_create_wallet.side_effect = mock_get_wallet - # Check stakes before unstaking for wallet in mock_wallets: + # Check stakes before unstaking stake = _subtensor_mock.get_stake_for_coldkey_and_hotkey( - hotkey_ss58=wallet.hotkey.ss58_address, + hotkey_ss58=delegate_hotkey, coldkey_ss58=wallet.coldkey.ss58_address, ) - self.assertEqual(stake.rao, mock_stakes[wallet.hotkey_str].rao) + self.assertEqual(stake.rao, mock_stakes[wallet.name].rao) - cli.run() + config.wallet.name = wallet.name + config.hotkey_ss58address = delegate_hotkey # Single unstake - # Check stakes after unstaking - for wallet in mock_wallets: - stake = _subtensor_mock.get_stake_for_coldkey_and_hotkey( - hotkey_ss58=wallet.hotkey.ss58_address, - coldkey_ss58=wallet.coldkey.ss58_address, - ) - # because the amount is less than the threshold, none of these should unstake - self.assertEqual(stake.tao, mock_stakes[wallet.hotkey_str].tao) + cli = bittensor.cli(config) + with patch.object(_subtensor_mock, "_do_unstake") as mock_unstake: + with patch( + "bittensor.__console__.print" + ) as mock_print: # Catch console print + cli.run() + + # Filter for console print calls + console_prints = [ + call[0][0] for call in mock_print.call_args_list + ] + minimum_print = filter( + lambda x: "less than minimum of" in x, console_prints + ) + + unstake_calls = mock_unstake.call_args_list + self.assertEqual(len(unstake_calls), 1) # Only one unstake call + + _, kwargs = unstake_calls[0] + # Verify delegate was unstaked + self.assertEqual(kwargs["hotkey_ss58"], delegate_hotkey) + self.assertEqual(kwargs["wallet"].name, wallet.name) + + if wallet.name == "w0": + # This wallet owns the delegate + # Should unstake specified amount + self.assertEqual( + kwargs["amount"], bittensor.Balance(config.amount) + ) + # No warning for w0 + self.assertRaises( + StopIteration, next, minimum_print + ) # No warning for w0 + else: + # Should unstake *all* the stake + staked = mock_stakes[wallet.name] + self.assertEqual(kwargs["amount"], staked) + + # Check warning was printed + _ = next( + minimum_print + ) # Doesn't raise, so the warning was printed def test_unstake_all(self, _): config = self.config @@ -1671,6 +1722,129 @@ def mock_get_wallet(*args, **kwargs): ) self.assertAlmostEqual(balance.tao, mock_balance.tao, places=4) + def test_stake_with_thresholds(self, _): + config = self.config + config.command = "stake" + config.subcommand = "add" + config.no_prompt = True + + min_stake: Balance = _subtensor_mock.get_minimum_required_stake() + # Must be a float + wallet_names = ["w0", "w1", "w2"] + config.all_hotkeys = False + # Notice no max_stake specified + + mock_stakes: Dict[str, Balance] = { + "w0": min_stake - 1, # new stake will be below the threshold + "w1": min_stake - 2, + "w2": min_stake - 5, + } + + mock_wallets = [ + SimpleNamespace( + name=wallet_name, + coldkey=_get_mock_keypair(idx, self.id()), + coldkeypub=_get_mock_keypair(idx, self.id()), + hotkey_str="hk{}".format(idx), # doesn't matter + hotkey=_get_mock_keypair(idx + 100, self.id()), # doesn't matter + ) + for idx, wallet_name in enumerate(wallet_names) + ] + + delegate_hotkey = mock_wallets[0].hotkey.ss58_address + + # Register mock neuron, only for w0 + _ = _subtensor_mock.force_register_neuron( + netuid=1, + hotkey=delegate_hotkey, + coldkey=mock_wallets[0].coldkey.ss58_address, + balance=(mock_stakes["w0"] + _subtensor_mock.get_existential_deposit()).tao + + 1.0, + ) # No stake, but enough balance + + # Become a delegate + _ = _subtensor_mock.nominate( + wallet=mock_wallets[0], + ) + + # Give enough balance + for wallet in mock_wallets[1:]: + # Give balance + _ = _subtensor_mock.force_set_balance( + ss58_address=wallet.coldkeypub.ss58_address, + balance=( + mock_stakes[wallet.name] + _subtensor_mock.get_existential_deposit() + ).tao + + 1.0, + ) + + def mock_get_wallet(*args, **kwargs): + if kwargs.get("config") and kwargs["config"].get("wallet"): + for wallet in mock_wallets: + if wallet.name == kwargs["config"].wallet.name: + return wallet + + with patch("bittensor.wallet") as mock_create_wallet: + mock_create_wallet.side_effect = mock_get_wallet + + for wallet in mock_wallets: + # Check balances and stakes before staking + stake = _subtensor_mock.get_stake_for_coldkey_and_hotkey( + hotkey_ss58=delegate_hotkey, + coldkey_ss58=wallet.coldkey.ss58_address, + ) + self.assertEqual(stake.rao, 0) # No stake + + balance = _subtensor_mock.get_balance( + address=wallet.coldkeypub.ss58_address + ) + self.assertGreaterEqual( + balance, mock_stakes[wallet.name] + ) # Enough balance + + config.wallet.name = wallet.name + config.wallet.hotkey = delegate_hotkey # Single stake + config.amount = mock_stakes[ + wallet.name + ].tao # Stake an amount below the threshold + + cli = bittensor.cli(config) + with patch.object(_subtensor_mock, "_do_stake") as mock_stake: + with patch( + "bittensor.__console__.print" + ) as mock_print: # Catch console print + cli.run() + + # Filter for console print calls + console_prints = [ + call[0][0] for call in mock_print.call_args_list + ] + minimum_print = filter( + lambda x: "below the minimum required" in x, console_prints + ) + + if wallet.name == "w0": + # This wallet owns the delegate + stake_calls = mock_stake.call_args_list + # Can stake below the threshold + self.assertEqual(len(stake_calls), 1) + + _, kwargs = stake_calls[0] + + # Should stake specified amount + self.assertEqual( + kwargs["amount"], bittensor.Balance(config.amount) + ) + # No error for w0 + self.assertRaises( + StopIteration, next, minimum_print + ) # No warning for w0 + else: + # Should not call stake + self.assertEqual(len(mock_stake.call_args_list), 0) + # Should print error + self.assertIsNotNone(next(minimum_print)) + def test_nominate(self, _): config = self.config config.command = "root" diff --git a/tests/unit_tests/extrinsics/test_staking.py b/tests/unit_tests/extrinsics/test_staking.py index 288e065f78..c3b888520b 100644 --- a/tests/unit_tests/extrinsics/test_staking.py +++ b/tests/unit_tests/extrinsics/test_staking.py @@ -114,9 +114,6 @@ def test_add_stake_extrinsic( else amount ) - if staking_balance > bittensor.Balance.from_rao(1000): - staking_balance = staking_balance - bittensor.Balance.from_rao(1000) - with patch.object( mock_subtensor, "_do_stake", return_value=expected_success ) as mock_add_stake, patch.object( @@ -135,7 +132,20 @@ def test_add_stake_extrinsic( mock_subtensor, "is_hotkey_delegate", return_value=hotkey_delegate ), patch.object(mock_subtensor, "get_delegate_take", return_value=0.01), patch( "rich.prompt.Confirm.ask", return_value=user_accepts - ) as mock_confirm: + ) as mock_confirm, patch.object( + mock_subtensor, + "get_minimum_required_stake", + return_value=bittensor.Balance.from_tao(0.01), + ), patch.object( + mock_subtensor, + "get_existential_deposit", + return_value=bittensor.Balance.from_rao(100_000), + ): + mock_balance = mock_subtensor.get_balance() + existential_deposit = mock_subtensor.get_existential_deposit() + if staking_balance > mock_balance - existential_deposit: + staking_balance = mock_balance - existential_deposit + # Act if not hotkey_owner and not hotkey_delegate: with pytest.raises(exception): diff --git a/tests/unit_tests/extrinsics/test_unstaking.py b/tests/unit_tests/extrinsics/test_unstaking.py index 6ad0a977e7..0fa6ba84c4 100644 --- a/tests/unit_tests/extrinsics/test_unstaking.py +++ b/tests/unit_tests/extrinsics/test_unstaking.py @@ -39,8 +39,8 @@ def mock_get_minimum_required_stake(): ("5FHneW46...", 10.0, True, True, True, False, False, False), # Not enough stake to unstake ("5FHneW46...", 1000.0, True, True, False, None, False, False), - # Unsuccessful - unstake threshold not reached - (None, 0.01, True, True, False, None, False, False), + # Successful - unstake threshold not reached + (None, 0.01, True, True, False, None, True, True), # Successful unstaking all (None, None, False, False, False, None, True, True), # Failure - unstaking failed @@ -51,7 +51,7 @@ def mock_get_minimum_required_stake(): "successful-with-prompt", "failure-prompt-declined", "failure-not-enough-stake", - "failure-threshold-not-reached", + "success-threshold-not-reached", "success-unstake-all", "failure-unstake-failed", ], @@ -166,18 +166,20 @@ def test_unstake_extrinsic( None, None, ), - # Unsuccessful unstake - threshold not reached + # Successful unstake - new stake below threshold ( ["5FHneW46..."], - [0.01], + [ + 100 - mock_get_minimum_required_stake() + 0.01 + ], # New stake just below threshold 100, True, True, False, True, - [None], - False, - 0, + [True], + True, # Sucessful unstake + 1, None, None, ), @@ -247,7 +249,7 @@ def test_unstake_extrinsic( "partial-success-one-fail", "success-no-hotkey", "failure-not-enough-stake", - "failure-threshold-not-reached", + "success-threshold-not-reached", "failure-prompt-declined", "failure-type-error-hotkeys", "failure-value-error-amounts",