Conversation
pallets/subtensor/src/lib.rs
Outdated
| } | ||
|
|
||
| pub fn total(&self) -> i128 { | ||
| self.coldkey_stake_deltas.values().sum() |
There was a problem hiding this comment.
We should consider if it helps the runtime to update this on every add/remove that way calling total() is faster.
Might be worse runtime depending on where we need to update it.
There was a problem hiding this comment.
The data structure is not finalized (probably, we'll use StorageNMap). We'll reconsider the total operation after that.
| pub fn get_stake_delta_for_hotkey_on_subnet(hotkey: &T::AccountId, netuid: u16) -> u64 { | ||
| let total_stake = StakeDeltaSinceLastEmissionDrain::<T>::get(netuid, hotkey).total(); | ||
|
|
||
| total_stake.abs().saturated_into() |
There was a problem hiding this comment.
How do we know if the sum is positive or negative here?
I think we use it like total_stake.max(0), so we discard the negatives. But abs isn't the same.
There was a problem hiding this comment.
If we remove the stake this epoch we will get a negative value. But we should mitigate the stake movement without taking the movement direction into account (add or remove), shouldn't we? We only dampen the movement. Please, correct me if I'm wrong.
There was a problem hiding this comment.
Here we return only an unsigned int so we can't tell if the delta was positive or negative.
We only truly care about positive deltas anyway (at least right now).
There was a problem hiding this comment.
Here we return only an unsigned int so we can't tell if the delta was positive or negative.
Yes, because I was going to reduce any movement effect ("punish any stake movement for the epoch"). I chose "abs" because otherwise "remove_stake" will create a negative value and we'll increase the emissions, which doesn't seem to be a desired outcome.
We only truly care about positive deltas anyway (at least right now).
Ok. I will go with total_stake.max(0) then. Thanks!
shamil-gadelshin
added
the
skip-cargo-audit
|
The last changes address PR comments, introduce additional places for stake delta effects, add tests, and simplify stake delta storage map by removing the coldkey from the storage. |
|
We chose a different approach: #1731 |
2 participants
Description
This PR reimplements "stake delta" feature that was removed previously. It saves stake movement into the separate storage by hotkey, coldkey, and netuid. During the
epoch()function execution stake delta is used as a "damper" to counter large stake movement effects on theepoch.Related Issue(s)
Type of Change
Breaking Change
The PR changes the
epochcalculations.Checklist
cargo fmtandcargo clippyto ensure my code is formatted and linted correctlyAdditional Notes
The original PR comes as a draft to gather feedback.