New stake policy and tests

Author: ii-cruz

primitives/account/src/account/staking_contract/receipts.rs

@@ -111,6 +111,22 @@ pub struct DeleteValidatorReceipt {
 }
 convert_receipt!(DeleteValidatorReceipt);
 
+#[derive(Debug, PartialEq, Eq, Clone, Hash, Serialize, Deserialize)]
+pub enum BalanceType {
+    Active,
+    Inactive,
+    Retired,
+}
+
+/// Receipt for most staker-related transactions. This is necessary to be able to revert
+/// these transactions.
+#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]
+pub struct AddStakeReceipt {
+    /// the balance which the stake was attributed to.
+    pub credited_balance: BalanceType,
+}
+convert_receipt!(AddStakeReceipt);
+
 /// Receipt for most staker-related transactions. This is necessary to be able to revert
 /// these transactions.
 #[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]

primitives/account/src/account/staking_contract/staker.rs

@@ -9,6 +9,8 @@ use nimiq_primitives::coin::Coin;
 use nimiq_primitives::policy::Policy;
 use serde::{Deserialize, Serialize};
 
+use super::AddStakeReceipt;
+use crate::BalanceType;
 #[cfg(feature = "interaction-traits")]
 use crate::{
     account::staking_contract::{
@@ -26,8 +28,10 @@ use crate::{
 ///
 /// Actions concerning a staker are:
 /// 1. Create:           Creates a staker.
-/// 2. AddStake:         Adds coins from any outside address to the staker's active balance.
-///                      This action is only possible if:
+/// 2. AddStake:         Adds coins from any outside address to the staker.
+///                      The balance to accredit these funds is preferentially the active balance. If there is no
+///                      positive active balance, it credits the inactive balance instead.
+///                      This operation has a minimum value of minimum stake, this it always respects the invariant:
 ///                        (a) the resulting non-retired funds respect the invariant 1 - minimum stake for non-retired funds.
 /// 3. SetActiveStake:   Re-balances between active and inactive stake by setting the amount of active stake.
 ///                      This action restarts the lock-up period of the inactive stake.
@@ -146,6 +150,7 @@ impl Staker {
     /// Invariants:
     ///         (1) active + inactive balances must be == 0 or >= minimum stake
     ///         (2) active + inactive + retired balances must be == 0 or >= minimum stake
+    /// IMPORTANT: This code is shared between new and legacy add stake versions.
     pub(crate) fn enforce_min_stake(
         active_balance: Coin,
         inactive_balance: Coin,
@@ -276,75 +281,127 @@ impl StakingContract {
         Ok(())
     }
 
-    /// Adds more Coins to a staker's balance. It will be directly added to the staker's balance.
+    /// Adds more Coins to a staker's balance. It adds to the staker's active balance
+    /// if there are some funds on it (active balance >0). Otherwise, it credits the value to
+    /// the inactive balance.
     /// Anyone can add stake for a staker. The staker must already exist.
     pub fn add_stake(
         &mut self,
         store: &mut StakingContractStoreWrite,
         staker_address: &Address,
         value: Coin,
+        protocol_version: u16,
         tx_logger: &mut TransactionLog,
-    ) -> Result<(), AccountError> {
+    ) -> Result<AddStakeReceipt, AccountError> {
+        if protocol_version < Policy::ADD_STAKE_PROTOCOL_UPGRADE_VERSION {
+            log::warn!(%protocol_version, "Adding stake using old protocol version");
+            return self.legacy_add_stake_0v(store, staker_address, value, tx_logger);
+        }
         // Get the staker.
         let mut staker = store.expect_staker(staker_address)?;
 
-        // Fail if the minimum stake would be violated for the non-retired funds (invariant 1).
-        Staker::enforce_min_stake(
-            staker.active_balance + value,
-            staker.inactive_balance,
-            staker.retired_balance,
-        )?;
-
-        // All checks passed, not allowed to fail from here on!
-
-        // If we are delegating to a validator, we need to update it.
+        // Check that the delegation is still valid, i.e. the validator hasn't been deleted.
         if let Some(validator_address) = &staker.delegation {
-            // Check that the delegation is still valid, i.e. the validator hasn't been deleted.
             store.expect_validator(validator_address)?;
-            self.increase_stake_to_validator(store, validator_address, value);
         }
 
+        // Add stake txs never violate minimum stake for the non-retired funds (invariant 1),
+        // because the intrinsic tx checks that value is >= min stake.
+        assert!(
+            Staker::enforce_min_stake(
+                staker.active_balance + value,
+                staker.inactive_balance,
+                staker.retired_balance,
+            )
+            .is_ok(),
+            "Add stake should never violate the min stake invariants"
+        );
+
+        // All checks passed, not allowed to fail from here on!
+
         // Update the staker's and staking contract's balances.
-        staker.active_balance += value;
+        // We want to preferentially credit the active balance. Only if there is no active balance,
+        // then it will attribute the funds to the inactive balance.
+        let credited_balance = if !staker.active_balance.is_zero() {
+            staker.active_balance += value;
+            BalanceType::Active
+        } else {
+            staker.inactive_balance += value;
+            if staker.inactive_from.is_none() {
+                staker.inactive_from = Some(0); // To uphold the (invariant 3)
+            }
+            BalanceType::Inactive
+        };
         self.balance += value;
 
-        // Build the return logs
+        // Create the receipt.
+        let receipt = AddStakeReceipt {
+            credited_balance: credited_balance.clone(),
+        };
+
+        // If we are actively delegating to a validator, we need to update it.
+        if credited_balance == BalanceType::Active {
+            if let Some(validator_address) = &staker.delegation {
+                self.increase_stake_to_validator(store, validator_address, value);
+            }
+        }
+
+        // Build the return logs.
         tx_logger.push_log(Log::Stake {
             staker_address: staker_address.clone(),
             validator_address: staker.delegation.clone(),
             value,
+            credited_balance,
         });
 
         // Update the staker entry.
         store.put_staker(staker_address, staker);
 
-        Ok(())
+        Ok(receipt)
     }
 
     /// Reverts a stake transaction.
+    /// IMPORTANT: This code is shared between new and legacy add stake versions.
     pub fn revert_add_stake(
         &mut self,
         store: &mut StakingContractStoreWrite,
         staker_address: &Address,
         value: Coin,
+        receipt: AddStakeReceipt,
         tx_logger: &mut TransactionLog,
     ) -> Result<(), AccountError> {
         // Get the staker.
         let mut staker = store.expect_staker(staker_address)?;
 
         // If we are delegating to a validator, we need to update it too.
-        if let Some(validator_address) = &staker.delegation {
-            self.decrease_stake_from_validator(store, validator_address, value);
+        if receipt.credited_balance == BalanceType::Active {
+            if let Some(validator_address) = &staker.delegation {
+                self.decrease_stake_from_validator(store, validator_address, value);
+            }
         }
 
         // Update the staker's and staking contract's balances.
-        staker.active_balance -= value;
+        match receipt.credited_balance {
+            BalanceType::Active => {
+                staker.active_balance -= value;
+            }
+            BalanceType::Inactive => {
+                staker.inactive_balance -= value;
+                if staker.inactive_balance.is_zero() {
+                    staker.inactive_from = None;
+                }
+            }
+            BalanceType::Retired => {
+                staker.retired_balance -= value;
+            }
+        }
         self.balance -= value;
 
         tx_logger.push_log(Log::Stake {
             staker_address: staker_address.clone(),
             validator_address: staker.delegation.clone(),
             value,
+            credited_balance: receipt.credited_balance,
         });
 
         // Update the staker entry.
@@ -949,6 +1006,7 @@ impl StakingContract {
     }
 
     /// Adds `value` coins to a given validator's total stake.
+    /// IMPORTANT: This code is shared between new and legacy add stake versions.
     fn increase_stake_to_validator(
         &mut self,
         store: &mut StakingContractStoreWrite,
@@ -982,6 +1040,7 @@ impl StakingContract {
     }
 
     /// Removes `value` coins from a given validator's inactive total stake.
+    /// IMPORTANT: This code is shared between new and legacy add stake versions.
     fn decrease_stake_from_validator(
         &mut self,
         store: &mut StakingContractStoreWrite,
@@ -1015,4 +1074,57 @@ impl StakingContract {
         // Neither validator nor tombstone exist, this is an error.
         panic!("inconsistent contract state");
     }
+
+    // Pre protocol upgrade.
+    // IMPORTANT: DO NOT REMOVE THIS CODE!
+    // It is needed for history nodes to sync.
+
+    /// Legacy add stake logic, it adds more Coins to a staker's active balance.
+    /// It will be directly added to the staker's balance.
+    /// Anyone can add stake for a staker. The staker must already exist.
+    pub fn legacy_add_stake_0v(
+        &mut self,
+        store: &mut StakingContractStoreWrite,
+        staker_address: &Address,
+        value: Coin,
+        tx_logger: &mut TransactionLog,
+    ) -> Result<AddStakeReceipt, AccountError> {
+        // Get the staker.
+        let mut staker = store.expect_staker(staker_address)?;
+
+        // Fail if the minimum stake would be violated for the non-retired funds (invariant 1).
+        Staker::enforce_min_stake(
+            staker.active_balance + value,
+            staker.inactive_balance,
+            staker.retired_balance,
+        )?;
+
+        // All checks passed, not allowed to fail from here on!
+
+        // If we are delegating to a validator, we need to update it.
+        if let Some(validator_address) = &staker.delegation {
+            // Check that the delegation is still valid, i.e. the validator hasn't been deleted.
+            store.expect_validator(validator_address)?;
+            self.increase_stake_to_validator(store, validator_address, value);
+        }
+
+        // Update the staker's and staking contract's balances.
+        staker.active_balance += value;
+        self.balance += value;
+
+        // Build the return logs
+        tx_logger.push_log(Log::Stake {
+            staker_address: staker_address.clone(),
+            validator_address: staker.delegation.clone(),
+            value,
+            credited_balance: BalanceType::Active,
+        });
+
+        // Update the staker entry.
+        store.put_staker(staker_address, staker);
+
+        Ok(AddStakeReceipt {
+            credited_balance: BalanceType::Active,
+        })
+    }
 }

primitives/account/src/account/staking_contract/traits.rs

@@ -169,8 +169,14 @@ impl AccountTransactionInteraction for StakingContract {
                 .map(|_| None)
             }
             IncomingStakingTransactionData::AddStake { staker_address } => self
-                .add_stake(&mut store, &staker_address, transaction.value, tx_logger)
-                .map(|_| None),
+                .add_stake(
+                    &mut store,
+                    &staker_address,
+                    transaction.value,
+                    block_state.protocol_version,
+                    tx_logger,
+                )
+                .map(|receipt| Some(receipt.into())),
             IncomingStakingTransactionData::UpdateStaker {
                 new_delegation,
                 reactivate_all_stake,
@@ -282,7 +288,15 @@ impl AccountTransactionInteraction for StakingContract {
                 self.revert_create_staker(&mut store, &staker_address, transaction.value, tx_logger)
             }
             IncomingStakingTransactionData::AddStake { staker_address } => {
-                self.revert_add_stake(&mut store, &staker_address, transaction.value, tx_logger)
+                let receipt = receipt.ok_or(AccountError::InvalidReceipt)?.try_into()?;
+
+                self.revert_add_stake(
+                    &mut store,
+                    &staker_address,
+                    transaction.value,
+                    receipt,
+                    tx_logger,
+                )
             }
             IncomingStakingTransactionData::UpdateStaker { proof, .. } => {
                 // Get the staker address from the proof.

primitives/account/src/logs.rs

@@ -10,6 +10,8 @@ use nimiq_transaction::{
     Transaction,
 };
 
+use crate::BalanceType;
+
 #[derive(Debug, PartialEq, Eq, Clone, Hash, Serialize, Deserialize)]
 // Renaming affects only the struct names and thus their tag, the "type" field.
 #[serde(rename_all = "kebab-case", tag = "type")]
@@ -105,6 +107,7 @@ pub enum Log {
         staker_address: Address,
         validator_address: Option<Address>,
         value: Coin,
+        credited_balance: BalanceType,
     },
 
     #[serde(rename_all = "camelCase")]