Skip to content
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

Increment nonce only when processing stake / unstake transactions #2569

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions data_structures/benches/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ fn populate(b: &mut Bencher) {
let coins = i;
let epoch = i;
stakes
.add_stake(address.as_str(), coins, epoch, MINIMUM_VALIDATOR_STAKE)
.add_stake(
address.as_str(),
coins,
epoch,
true,
MINIMUM_VALIDATOR_STAKE,
)
.unwrap();

i += 1;
Expand All @@ -37,7 +43,13 @@ fn rank(b: &mut Bencher) {
let address = format!("{}", rng.gen::<u64>());

stakes
.add_stake(address.as_str(), coins, epoch, MINIMUM_VALIDATOR_STAKE)
.add_stake(
address.as_str(),
coins,
epoch,
true,
MINIMUM_VALIDATOR_STAKE,
)
.unwrap();

i += 1;
Expand Down Expand Up @@ -69,7 +81,13 @@ fn query_power(b: &mut Bencher) {
let address = format!("{i}");

stakes
.add_stake(address.as_str(), coins, epoch, MINIMUM_VALIDATOR_STAKE)
.add_stake(
address.as_str(),
coins,
epoch,
true,
MINIMUM_VALIDATOR_STAKE,
)
.unwrap();

i += 1;
Expand Down
16 changes: 11 additions & 5 deletions data_structures/src/staking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ pub mod test {
let mut stakes = StakesTester::default();

// Alpha stakes 2 @ epoch 0
stakes.add_stake("Alpha", 2, 0, MIN_STAKE_NANOWITS).unwrap();
stakes
.add_stake("Alpha", 2, 0, true, MIN_STAKE_NANOWITS)
.unwrap();

// Nobody holds any power just yet
let rank = stakes.by_rank(Capability::Mining, 0).collect::<Vec<_>>();
Expand All @@ -42,7 +44,9 @@ pub mod test {
assert_eq!(rank, vec![("Alpha".into(), 2)]);

// Beta stakes 5 @ epoch 10
stakes.add_stake("Beta", 5, 10, MIN_STAKE_NANOWITS).unwrap();
stakes
.add_stake("Beta", 5, 10, true, MIN_STAKE_NANOWITS)
.unwrap();

// Alpha is still leading, but Beta has scheduled its takeover
let rank = stakes.by_rank(Capability::Mining, 10).collect::<Vec<_>>();
Expand All @@ -56,7 +60,7 @@ pub mod test {

// Gamma should never take over, even in a million epochs, because it has only 1 coin
stakes
.add_stake("Gamma", 1, 30, MIN_STAKE_NANOWITS)
.add_stake("Gamma", 1, 30, true, MIN_STAKE_NANOWITS)
.unwrap();
let rank = stakes
.by_rank(Capability::Mining, 1_000_000)
Expand All @@ -72,7 +76,7 @@ pub mod test {

// But Delta is here to change it all
stakes
.add_stake("Delta", 1_000, 50, MIN_STAKE_NANOWITS)
.add_stake("Delta", 1_000, 50, true, MIN_STAKE_NANOWITS)
.unwrap();
let rank = stakes.by_rank(Capability::Mining, 50).collect::<Vec<_>>();
assert_eq!(
Expand All @@ -96,7 +100,9 @@ pub mod test {
);

// If Alpha removes all of its stake, it should immediately disappear
stakes.remove_stake("Alpha", 2, MIN_STAKE_NANOWITS).unwrap();
stakes
.remove_stake("Alpha", 2, true, MIN_STAKE_NANOWITS)
.unwrap();
let rank = stakes.by_rank(Capability::Mining, 51).collect::<Vec<_>>();
assert_eq!(
rank,
Expand Down
10 changes: 8 additions & 2 deletions data_structures/src/staking/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ where
&mut self,
coins: Coins,
epoch: Epoch,
increment_nonce: bool,
minimum_stakeable: Coins,
) -> StakesResult<Coins, Address, Coins, Epoch> {
// Make sure that the amount to be staked is equal or greater than the minimum
Expand Down Expand Up @@ -112,7 +113,9 @@ where
self.epochs.update(capability, epoch_after);
}

self.nonce += Nonce::from(1);
if increment_nonce {
self.nonce += Nonce::from(1);
}

Ok(coins_after)
}
Expand Down Expand Up @@ -141,6 +144,7 @@ where
pub fn remove_stake(
&mut self,
coins: Coins,
increment_nonce: bool,
minimum_stakeable: Coins,
) -> StakesResult<Coins, Address, Coins, Epoch> {
let coins_after = self.coins.sub(coins);
Expand All @@ -154,7 +158,9 @@ where

self.coins = coins_after;

self.nonce += Nonce::from(1);
if increment_nonce {
self.nonce += Nonce::from(1);
}

Ok(self.coins)
}
Expand Down
90 changes: 58 additions & 32 deletions data_structures/src/staking/stakes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ where
key: ISK,
coins: Coins,
epoch: Epoch,
increment_nonce: bool,
minimum_stakeable: Coins,
) -> StakesResult<Stake<UNIT, Address, Coins, Epoch, Nonce, Power>, Address, Coins, Epoch>
where
Expand All @@ -240,7 +241,7 @@ where
stake
.value
.write()?
.add_stake(coins, epoch, minimum_stakeable)?;
.add_stake(coins, epoch, increment_nonce, minimum_stakeable)?;

// Update all indexes if needed (only when the stake entry didn't exist before)
if !stake_found {
Expand Down Expand Up @@ -368,6 +369,7 @@ where
&mut self,
key: ISK,
coins: Coins,
increment_nonce: bool,
minimum_stakeable: Coins,
) -> StakesResult<Coins, Address, Coins, Epoch>
where
Expand All @@ -380,7 +382,7 @@ where
let final_coins = {
let mut stake = by_address_entry.get_mut().value.write()?;

stake.remove_stake(coins, minimum_stakeable)?
stake.remove_stake(coins, increment_nonce, minimum_stakeable)?
};

// No need to keep the entry if the stake has gone to zero
Expand Down Expand Up @@ -521,7 +523,7 @@ where
.value
.write()
.unwrap()
.add_stake(coins, current_epoch, 0.into());
.add_stake(coins, current_epoch, false, 0.into());

Ok(())
}
Expand All @@ -548,7 +550,7 @@ where
.value
.write()
.unwrap()
.remove_stake(coins, minimum_stakeable);
.remove_stake(coins, false, minimum_stakeable);

Ok(())
}
Expand Down Expand Up @@ -807,7 +809,7 @@ where
key.validator.bech32(environment)
);

stakes.add_stake(key, coins, epoch, minimum_stakeable.into())?;
stakes.add_stake(key, coins, epoch, true, minimum_stakeable.into())?;

log::debug!("Current state of the stakes tracker: {:#?}", stakes);

Expand Down Expand Up @@ -866,7 +868,7 @@ where
coins.wits_and_nanowits().0,
);

stakes.remove_stake(key, coins, minimum_stakeable.into())?;
stakes.remove_stake(key, coins, true, minimum_stakeable.into())?;

log::debug!("Current state of the stakes tracker: {:#?}", stakes);

Expand Down Expand Up @@ -1004,7 +1006,7 @@ mod tests {
// Let's make Alice stake 100 Wit at epoch 100
assert_eq!(
stakes
.add_stake(alice_charlie, 100, 100, MIN_STAKE_NANOWITS)
.add_stake(alice_charlie, 100, 100, true, MIN_STAKE_NANOWITS)
.unwrap(),
Stake::from_parts(
100,
Expand All @@ -1028,7 +1030,7 @@ mod tests {
// Let's make Alice stake 50 Wits at epoch 150 this time
assert_eq!(
stakes
.add_stake(alice_charlie, 50, 300, MIN_STAKE_NANOWITS)
.add_stake(alice_charlie, 50, 300, true, MIN_STAKE_NANOWITS)
.unwrap(),
Stake::from_parts(
150,
Expand Down Expand Up @@ -1059,7 +1061,7 @@ mod tests {
// Now let's make Bob stake 500 Wits at epoch 1000 this time
assert_eq!(
stakes
.add_stake(bob_david, 500, 1_000, MIN_STAKE_NANOWITS)
.add_stake(bob_david, 500, 1_000, true, MIN_STAKE_NANOWITS)
.unwrap(),
Stake::from_parts(
500,
Expand Down Expand Up @@ -1118,13 +1120,13 @@ mod tests {
let charlie_erin = (charlie, erin);

stakes
.add_stake(alice_charlie, 10, 0, MIN_STAKE_NANOWITS)
.add_stake(alice_charlie, 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(bob_david, 20, 20, MIN_STAKE_NANOWITS)
.add_stake(bob_david, 20, 20, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(charlie_erin, 30, 30, MIN_STAKE_NANOWITS)
.add_stake(charlie_erin, 30, 30, true, MIN_STAKE_NANOWITS)
.unwrap();

// Let's really start our test at epoch 100
Expand Down Expand Up @@ -1270,19 +1272,19 @@ mod tests {
let erin_alice = (erin, alice);

stakes
.add_stake(alice_bob, 10, 0, MIN_STAKE_NANOWITS)
.add_stake(alice_bob, 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(bob_charlie, 20, 10, MIN_STAKE_NANOWITS)
.add_stake(bob_charlie, 20, 10, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(charlie_david, 30, 20, MIN_STAKE_NANOWITS)
.add_stake(charlie_david, 30, 20, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(david_erin, 40, 30, MIN_STAKE_NANOWITS)
.add_stake(david_erin, 40, 30, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(erin_alice, 50, 40, MIN_STAKE_NANOWITS)
.add_stake(erin_alice, 50, 40, true, MIN_STAKE_NANOWITS)
.unwrap();

// Power of validators at epoch 90:
Expand Down Expand Up @@ -1331,13 +1333,13 @@ mod tests {
let charlie_erin = (charlie, erin);

stakes
.add_stake(alice_charlie, 10, 0, MIN_STAKE_NANOWITS)
.add_stake(alice_charlie, 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(bob_david, 20, 30, MIN_STAKE_NANOWITS)
.add_stake(bob_david, 20, 30, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(charlie_erin, 40, 50, MIN_STAKE_NANOWITS)
.add_stake(charlie_erin, 40, 50, true, MIN_STAKE_NANOWITS)
.unwrap();

let result = stakes.query_stakes(QueryStakesKey::Key(bob_david.into()));
Expand Down Expand Up @@ -1415,7 +1417,7 @@ mod tests {

let alice_bob = (alice.clone(), bob.clone());
stakes
.add_stake(alice_bob, 123, 456, MIN_STAKE_NANOWITS)
.add_stake(alice_bob, 123, 456, true, MIN_STAKE_NANOWITS)
.ok();

let serialized = bincode::serialize(&stakes).unwrap().clone();
Expand Down Expand Up @@ -1448,7 +1450,7 @@ mod tests {

// Use the validator with a (validator, withdrawer) pair
stakes
.add_stake((alice, bob), 10, 0, MIN_STAKE_NANOWITS)
.add_stake((alice, bob), 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();

// The validator is used, we can still stake as long as the correct withdrawer is used
Expand Down Expand Up @@ -1476,26 +1478,50 @@ mod tests {
let alice_charlie = (alice, charlie);
let bob_david = (bob, david);

// Test nonces increasing
stakes
.add_stake(alice_charlie, 10, 0, MIN_STAKE_NANOWITS)
.add_stake(alice_charlie, 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(bob_david, 20, 10, MIN_STAKE_NANOWITS)
.add_stake(bob_david, 20, 10, true, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(1));

stakes
.remove_stake(bob_david, 10, MIN_STAKE_NANOWITS)
.remove_stake(bob_david, 10, true, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(2));

stakes
.add_stake(bob_david, 40, 30, MIN_STAKE_NANOWITS)
.add_stake(bob_david, 40, 30, true, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(3));

// Test nonces not increasing
stakes
.remove_stake(bob_david, 10, false, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(3));

stakes
.reserve_collateral(bob, 10, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(3));

stakes
.add_stake(bob_david, 40, 30, false, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(3));

stakes.add_reward(bob, 40, 30).unwrap();
assert_eq!(stakes.query_nonce(alice_charlie), Ok(1));
assert_eq!(stakes.query_nonce(bob_david), Ok(3));
}

#[test]
Expand All @@ -1509,10 +1535,10 @@ mod tests {

// Add some stake and verify the power
stakes
.add_stake((alice, charlie), 10, 0, MIN_STAKE_NANOWITS)
.add_stake((alice, charlie), 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake((bob, david), 20, 10, MIN_STAKE_NANOWITS)
.add_stake((bob, david), 20, 10, true, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(stakes.query_power(alice, Capability::Mining, 30), Ok(300));
assert_eq!(stakes.query_power(bob, Capability::Mining, 30), Ok(400));
Expand Down Expand Up @@ -1563,16 +1589,16 @@ mod tests {

// Add some stake and verify the power
stakes
.add_stake(alice_alice, 10, 0, MIN_STAKE_NANOWITS)
.add_stake(alice_alice, 10, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(bob_alice, 20, 0, MIN_STAKE_NANOWITS)
.add_stake(bob_alice, 20, 0, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(charlie_charlie, 30, 10, MIN_STAKE_NANOWITS)
.add_stake(charlie_charlie, 30, 10, true, MIN_STAKE_NANOWITS)
.unwrap();
stakes
.add_stake(david_charlie, 40, 10, MIN_STAKE_NANOWITS)
.add_stake(david_charlie, 40, 10, true, MIN_STAKE_NANOWITS)
.unwrap();
assert_eq!(
stakes.by_rank(Capability::Mining, 30).collect::<Vec<_>>(),
Expand Down
Loading
Loading