Skip to content

Add stake locks#1731

Closed
shamil-gadelshin wants to merge 4 commits intofeat/uniswapv3-lpfrom
feat/stake-lock-v2
Closed

Add stake locks#1731
shamil-gadelshin wants to merge 4 commits intofeat/uniswapv3-lpfrom
feat/stake-lock-v2

Conversation

@shamil-gadelshin
Copy link
Collaborator

Description

This PR introduces stake locks.

This PR addresses the stake manipulation problem by introducing a stake lock after each stake deposit operation (like add_stake or add_stake_limit). The lock is the block number after which stake removal is allowed. It calculates as "current_block + subnet tempo".

This PR relates to both MEV and "stake delta" problems.

Affected extrinsics:

  • add_stake
  • add_stake_limit
  • remove_stake
  • remove_stake_limit
  • unstake_all
  • unstake_all_alpha
  • move_stake
  • swap_stake
  • swap_stake_limit
  • transfer_stake

All extrinsics that add stake to the destination netuid lock tuple (hotkey, coldkey, netuid) for the tempo period of the subnet. All extrinsics that remove stake check the stake lock first and fail with the StakeLocked error if the lock is active. The only exception is transfer_stake when the destination coldkey is different from the origin (to prevent putting locks on others).

Related Issue(s)

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

The code that removes the stake immediately will stop working.

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

Additional Notes

The additional weights fix commit will be likely required and added after the initial code review.

#[pallet::type_value]
/// Default value for block number
pub fn DefaultBlockNumber<T: Config>() -> BlockNumberFor<T> {
0u32.into()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid confusion, please change to u64 because block numbers are u64 in subtensor.

let coldkey = ensure_signed(origin)?;

// Prevent DoS attacks
let set_lock = coldkey == destination_coldkey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noo, the transfer stake functionality main use case is to transfer from one coldkey to another :)

@bdmason
Copy link
Contributor

bdmason commented Jun 11, 2025

If we're going to lock stake up for a whole epoch, then it seems fair to remove the dynamic staking fee when unstaking. Currently on non root subnets the fee is equivalent to 1 epoch of staking rewards for the amount being unstaked. This should go back to the fixed 50k RAO as you can no longer manipulate an epoch of rewards by staking on the block before the step then leaving the block after.

@shamil-gadelshin shamil-gadelshin mentioned this pull request Jun 11, 2025
13 tasks
@gztensor
Copy link
Contributor

If we're going to lock stake up for a whole epoch, then it seems fair to remove the dynamic staking fee when unstaking. Currently on non root subnets the fee is equivalent to 1 epoch of staking rewards for the amount being unstaked. This should go back to the fixed 50k RAO as you can no longer manipulate an epoch of rewards by staking on the block before the step then leaving the block after.

@bdmason , true, but with swap v3 the dynamic fees will go away and will be replaced with flat rate swap fee (because there will be users providing liquidity who receive these fees).

@RD4Fun
Copy link
Contributor

RD4Fun commented Jun 12, 2025

All extrinsics that remove stake check the stake lock first and fail with the StakeLocked error if the lock is active. The only exception is transfer_stake when the destination coldkey is different from the origin (to prevent putting locks on others).

What about a MEV bot running with two cold keys?
It now bypasses the stake lock by transferring in and transferring out.
imo transfer_stake needs a lock on the destination coldkey also.

@bdmason
Copy link
Contributor

bdmason commented Jun 12, 2025

I've been thinking about this a bit more... whilst I've been wanting stake locks for a long time, Bittensor has moved on and become a marketplace. Have we considered the effect stake locks could have on integrations with exchanges e.g. Binance?

The functionality to stop sandwich attacks already exists in add_stake_limit and remove_stake_limit, you just need to understand how to set a very tight limit correctly, something we can help educate people to do.

Also, on the topic of sandwich attacks, the recent update to mainnet add_stake_limit_aggregate just adds a new type of sandwich attack, you monitor the pool for a submission of add_stake_limit_aggregate, you front run them with your own add_stake_limit call, you are 100% guaranteed to get in before them as they are pushed to the following block, then in the next block you call unstake_all and you are guaranteed to be executed after them as the aggregates are run at the start of the block.

Aren't we just complicating the network to fix a problem that was fixed a long time ago with limit prices?

@shamil-gadelshin
Copy link
Collaborator Author

#1766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants