Mitigate PM change attack (#1236)

* Mitigate PM change attack

* More paranoid test

contracts/mixnet/src/error.rs

@@ -174,4 +174,9 @@ pub enum ContractError {
         total_node_delegation: u128,
         to_subtract: u128,
     },
+    #[error("Profit margin can be updated only once during a rolling 30 day interval, last update was at {last_update_time} and current block time is {current_block_time}")]
+    UpdatePMTooSoon {
+        last_update_time: u64,
+        current_block_time: u64,
+    },
 }

contracts/mixnet/src/mixnodes/storage.rs

@@ -19,6 +19,8 @@ const MIXNODES_PK_CHANGELOG: &str = "mn__change";
 const MIXNODES_OWNER_IDX_NAMESPACE: &str = "mno";
 const MIXNODES_SPHINX_IDX_NAMESPACE: &str = "mns";
 
+const LAST_PM_UPDATE_NAMESPACE: &str = "lpm";
+
 // paged retrieval limits for all queries and transactions
 pub(crate) const BOND_PAGE_MAX_LIMIT: u32 = 75;
 pub(crate) const BOND_PAGE_DEFAULT_LIMIT: u32 = 50;
@@ -26,6 +28,9 @@ pub(crate) const BOND_PAGE_DEFAULT_LIMIT: u32 = 50;
 pub(crate) const TOTAL_DELEGATION: Map<'_, IdentityKeyRef<'_>, Uint128> =
     Map::new(TOTAL_DELEGATION_NAMESPACE);
 
+pub(crate) const LAST_PM_UPDATE_TIME: Map<'_, IdentityKeyRef<'_>, u64> =
+    Map::new(LAST_PM_UPDATE_NAMESPACE);
+
 pub(crate) struct MixnodeBondIndex<'a> {
     pub(crate) owner: UniqueIndex<'a, Addr, StoredMixnodeBond>,
 

contracts/mixnet/src/mixnodes/transactions.rs

@@ -1,7 +1,7 @@
 // Copyright 2021 - Nym Technologies SA <contact@nymtech.net>
 // SPDX-License-Identifier: Apache-2.0
 
-use super::storage;
+use super::storage::{self, LAST_PM_UPDATE_TIME};
 use crate::error::ContractError;
 use crate::mixnet_contract_settings::storage as mixnet_params_storage;
 use crate::mixnodes::layer_queries::query_layer_distribution;
@@ -18,6 +18,8 @@ use mixnet_contract_common::MixNode;
 use vesting_contract_common::messages::ExecuteMsg as VestingContractExecuteMsg;
 use vesting_contract_common::one_ucoin;
 
+const MIN_PM_UPDATE_INTERVAL: u64 = 60 * 60 * 24 * 30; // one month roughly
+
 pub fn try_checkpoint_mixnodes(
     storage: &mut dyn Storage,
     block_height: u64,
@@ -152,6 +154,8 @@ fn _try_add_mixnode(
         storage::TOTAL_DELEGATION.save(deps.storage, identity, &Uint128::zero())?;
     }
 
+    storage::LAST_PM_UPDATE_TIME.save(deps.storage, identity, &env.block.time.seconds())?;
+
     mixnet_params_storage::increment_layer_count(deps.storage, stored_bond.layer)?;
 
     Ok(Response::new().add_event(new_mixnode_bonding_event(
@@ -294,6 +298,19 @@ pub(crate) fn _try_update_mixnode_config(
         });
     }
 
+    let last_update_time = storage::LAST_PM_UPDATE_TIME
+        .load(deps.storage, mixnode_bond.identity())
+        .unwrap_or(0);
+
+    let current_block_time = env.block.time.seconds();
+
+    if current_block_time - last_update_time < MIN_PM_UPDATE_INTERVAL {
+        return Err(ContractError::UpdatePMTooSoon {
+            last_update_time,
+            current_block_time,
+        });
+    }
+
     // We don't have to check lower bound as its an u8
     if profit_margin_percent > 100 {
         return Err(ContractError::InvalidProfitMarginPercent(
@@ -316,6 +333,8 @@ pub(crate) fn _try_update_mixnode_config(
         },
     )?;
 
+    LAST_PM_UPDATE_TIME.save(deps.storage, mixnode_bond.identity(), &current_block_time)?;
+
     let mut response = Response::new();
 
     if let Some(proxy) = proxy {
@@ -362,6 +381,8 @@ fn validate_mixnode_pledge(
 
 #[cfg(test)]
 pub mod tests {
+    use std::f64::MIN;
+
     use super::*;
     use crate::contract::{execute, query, INITIAL_MIXNODE_PLEDGE};
     use crate::error::ContractError;
@@ -715,6 +736,7 @@ pub mod tests {
     #[test]
     fn updating_mixnode_config() {
         let sender = "bob";
+        let mut env = mock_env();
         let mut deps = test_helpers::init_contract();
         let info = mock_info(sender, &[]);
 
@@ -722,7 +744,7 @@ pub mod tests {
         let msg = ExecuteMsg::UpdateMixnodeConfig {
             profit_margin_percent: 10,
         };
-        let ret = execute(deps.as_mut(), mock_env(), info.clone(), msg);
+        let ret = execute(deps.as_mut(), env.clone(), info.clone(), msg);
         assert_eq!(
             ret,
             Err(ContractError::NoAssociatedMixNodeBond {
@@ -751,12 +773,14 @@ pub mod tests {
                 .profit_margin_percent
         );
 
+        env.block.time = env.block.time.plus_seconds(MIN_PM_UPDATE_INTERVAL + 1);
+
         // try updating with an invalid value
         let profit_margin_percent = 101;
         let msg = ExecuteMsg::UpdateMixnodeConfig {
             profit_margin_percent,
         };
-        let ret = execute(deps.as_mut(), mock_env(), info.clone(), msg);
+        let ret = execute(deps.as_mut(), env.clone(), info.clone(), msg);
         assert_eq!(
             ret,
             Err(ContractError::InvalidProfitMarginPercent(
@@ -768,7 +792,7 @@ pub mod tests {
         let msg = ExecuteMsg::UpdateMixnodeConfig {
             profit_margin_percent,
         };
-        execute(deps.as_mut(), mock_env(), info, msg).unwrap();
+        execute(deps.as_mut(), env, info, msg).unwrap();
         assert_eq!(
             profit_margin_percent,
             storage::mixnodes()
@@ -882,4 +906,51 @@ pub mod tests {
         // change identity but reuse sphinx key
         assert!(try_add_mixnode(deps.as_mut(), mock_env(), info_bob, mixnode, sig2).is_err());
     }
+
+    #[test]
+    fn updating_pm_too_often_fails() {
+        use super::MIN_PM_UPDATE_INTERVAL;
+
+        let mut deps = test_helpers::init_contract();
+        let mut env = mock_env();
+
+        let keypair1 = crypto::asymmetric::identity::KeyPair::new(&mut thread_rng());
+        let sig1 = keypair1.private_key().sign_text("alice");
+
+        let info_alice = mock_info("alice", &tests::fixtures::good_mixnode_pledge());
+
+        let mixnode = MixNode {
+            host: "1.2.3.4".to_string(),
+            mix_port: 1234,
+            verloc_port: 1234,
+            http_api_port: 1234,
+            sphinx_key: crypto::asymmetric::encryption::KeyPair::new(&mut thread_rng())
+                .public_key()
+                .to_base58_string(),
+            identity_key: keypair1.public_key().to_base58_string(),
+            version: "v0.1.2.3".to_string(),
+            profit_margin_percent: 10,
+        };
+
+        assert!(try_add_mixnode(
+            deps.as_mut(),
+            mock_env(),
+            info_alice.clone(),
+            mixnode.clone(),
+            sig1
+        )
+        .is_ok());
+
+        env.block.time = env.block.time.plus_seconds(MIN_PM_UPDATE_INTERVAL - 1);
+
+        // fails if too soon after bonding
+        assert!(
+            try_update_mixnode_config(deps.as_mut(), env.clone(), info_alice.clone(), 20).is_err()
+        );
+
+        env.block.time = env.block.time.plus_seconds(2);
+
+        // succeds after some time
+        assert!(try_update_mixnode_config(deps.as_mut(), env, info_alice, 20).is_ok());
+    }
 }