fix pr comments

coordinator/src/cosign_evaluator.rs

@@ -204,14 +204,12 @@ impl<D: Db> CosignEvaluator<D> {
 
       let mut total_stake = 0;
       let mut total_on_distinct_chain = 0;
-      // TODO: `network` isn't being used in the following loop. is this a bug?
+      for network in EXTERNAL_NETWORKS {
-      // why are we going through the networks here?
-      for _network in EXTERNAL_NETWORKS {
         // Get the current set for this network
         let set_with_keys = {
           let mut res;
           while {
-            res = set_with_keys_fn(&serai, cosign.network).await;
+            res = set_with_keys_fn(&serai, network).await;
             res.is_err()
           } {
             log::error!(

coordinator/src/substrate/mod.rs

@@ -11,7 +11,7 @@ use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto};
 use serai_client::{
   coins::CoinsEvent,
   in_instructions::InInstructionsEvent,
-  primitives::{BlockHash, ExternalNetworkId, NetworkId},
+  primitives::{BlockHash, ExternalNetworkId},
   validator_sets::{
     primitives::{ExternalValidatorSet, ValidatorSet},
     ValidatorSetsEvent,
@@ -229,13 +229,8 @@ async fn handle_block<D: Db, Pro: Processors>(
       panic!("NewSet event wasn't NewSet: {new_set:?}");
     };
 
-    // If this is Serai, do nothing
     // We only coordinate/process external networks
-    if set.network == NetworkId::Serai {
+    let Ok(set) = ExternalValidatorSet::try_from(set) else { continue };
-      continue;
-    }
-
-    let set: ExternalValidatorSet = set.try_into().unwrap();
     if HandledEvent::is_unhandled(db, hash, event_id) {
       log::info!("found fresh new set event {:?}", new_set);
       let mut txn = db.txn();
@@ -290,11 +285,7 @@ async fn handle_block<D: Db, Pro: Processors>(
       panic!("AcceptedHandover event wasn't AcceptedHandover: {accepted_handover:?}");
     };
 
-    if set.network == NetworkId::Serai {
+    let Ok(set) = ExternalValidatorSet::try_from(set) else { continue };
-      continue;
-    }
-
-    let set: ExternalValidatorSet = set.try_into().unwrap();
     if HandledEvent::is_unhandled(db, hash, event_id) {
       log::info!("found fresh accepted handover event {:?}", accepted_handover);
       // TODO: This isn't atomic with the event handling
@@ -312,11 +303,7 @@ async fn handle_block<D: Db, Pro: Processors>(
       panic!("SetRetired event wasn't SetRetired: {retired_set:?}");
     };
 
-    if set.network == NetworkId::Serai {
+    let Ok(set) = ExternalValidatorSet::try_from(set) else { continue };
-      continue;
-    }
-
-    let set: ExternalValidatorSet = set.try_into().unwrap();
     if HandledEvent::is_unhandled(db, hash, event_id) {
       log::info!("found fresh set retired event {:?}", retired_set);
       let mut txn = db.txn();

substrate/coins/pallet/src/lib.rs

@@ -161,11 +161,9 @@ pub mod pallet {
     pub fn mint(to: Public, balance: Balance) -> Result<(), Error<T, I>> {
       // If the coin isn't Serai, which we're always allowed to mint, and the mint isn't explicitly
       // allowed, error
-      if !balance.coin.is_native() &&
+      if !ExternalCoin::try_from(balance.coin)
-        (!T::AllowMint::is_allowed(&ExternalBalance {
+        .map(|coin| T::AllowMint::is_allowed(&ExternalBalance { coin, amount: balance.amount }))
-          coin: balance.coin.try_into().unwrap(),
+        .unwrap_or(true)
-          amount: balance.amount,
-        }))
       {
         Err(Error::<T, I>::MintNotAllowed)?;
       }

substrate/dex/pallet/src/lib.rs

@@ -509,7 +509,7 @@ pub mod pallet {
 
     /// A hook to be called whenever a network's session is rotated.
     pub fn on_new_session(network: NetworkId) {
-      // reset the oracle value
+      // Only track the price for non-SRI coins as this is SRI denominated
       if let NetworkId::External(n) = network {
         for coin in n.coins() {
           SecurityOracleValue::<T>::set(coin, Self::median_price(coin));
@@ -936,11 +936,9 @@ pub mod pallet {
     pub fn get_pool_id(coin1: Coin, coin2: Coin) -> Result<PoolId, Error<T>> {
       ensure!((coin1 == Coin::Serai) || (coin2 == Coin::Serai), Error::<T>::PoolNotFound);
       ensure!(coin1 != coin2, Error::<T>::EqualCoins);
-      if coin1 == Coin::Serai {
+      ExternalCoin::try_from(coin1)
-        Ok(coin2.try_into().unwrap())
+        .or_else(|()| ExternalCoin::try_from(coin2))
-      } else {
+        .map_err(|()| Error::<T>::PoolNotFound)
-        Ok(coin1.try_into().unwrap())
-      }
     }
 
     /// Returns the balance of each coin in the pool.

substrate/emissions/pallet/src/lib.rs

@@ -244,12 +244,13 @@ pub mod pallet {
 
       // distribute the rewards within the network
       for (n, reward) in rewards_per_network {
-        let (validators_reward, network_pool_reward) = if n == NetworkId::Serai {
+        let validators_reward = if let NetworkId::External(external_network) = n {
-          (reward, 0)
-        } else {
           // calculate pool vs validator share
-          let capacity = ValidatorSets::<T>::total_allocated_stake(n).unwrap_or(Amount(0)).0;
+          let capacity =
-          let required = ValidatorSets::<T>::required_stake_for_network(n.try_into().unwrap());
+            ValidatorSets::<T>::total_allocated_stake(NetworkId::from(external_network))
+              .unwrap_or(Amount(0))
+              .0;
+          let required = ValidatorSets::<T>::required_stake_for_network(external_network);
           let unused_capacity = capacity.saturating_sub(required);
 
           let distribution = unused_capacity.saturating_mul(ACCURACY_MULTIPLIER) / capacity;
@@ -257,42 +258,44 @@ pub mod pallet {
 
           let validators_reward = DESIRED_DISTRIBUTION.saturating_mul(reward) / total;
           let network_pool_reward = reward.saturating_sub(validators_reward);
-          (validators_reward, network_pool_reward)
+
+          // send the rest to the pool
+          if network_pool_reward != 0 {
+            // these should be available to unwrap if we have a network_pool_reward. Because that
+            // means we had an unused capacity hence in a post-ec era.
+            let vpn = volume_per_network.as_ref().unwrap();
+            let vpc = volume_per_coin.as_ref().unwrap();
+            for c in external_network.coins() {
+              let pool_reward = u64::try_from(
+                u128::from(network_pool_reward).saturating_mul(u128::from(vpc[&c])) /
+                  u128::from(vpn[&n]),
+              )
+              .unwrap();
+
+              if Coins::<T>::mint(
+                Dex::<T>::get_pool_account(c),
+                Balance { coin: Coin::Serai, amount: Amount(pool_reward) },
+              )
+              .is_err()
+              {
+                // TODO: log the failure
+                continue;
+              }
+            }
+          }
+
+          validators_reward
+        } else {
+          reward
         };
 
         // distribute validators rewards
         Self::distribute_to_validators(n, validators_reward);
-
-        // send the rest to the pool
-        if network_pool_reward != 0 {
-          // these should be available to unwrap if we have a network_pool_reward. Because that
-          // means we had an unused capacity hence in a post-ec era.
-          let vpn = volume_per_network.as_ref().unwrap();
-          let vpc = volume_per_coin.as_ref().unwrap();
-          for c in n.coins() {
-            let pool_reward = u64::try_from(
-              u128::from(network_pool_reward)
-                .saturating_mul(u128::from(vpc[&c.try_into().unwrap()])) /
-                u128::from(vpn[&n]),
-            )
-            .unwrap();
-
-            if Coins::<T>::mint(
-              Dex::<T>::get_pool_account(c.try_into().unwrap()),
-              Balance { coin: Coin::Serai, amount: Amount(pool_reward) },
-            )
-            .is_err()
-            {
-              // TODO: log the failure
-              continue;
-            }
-          }
-        }
       }
 
       // TODO: we have the past session participants here in the emissions pallet so that we can
       // distribute rewards to them in the next session. Ideally we should be able to fetch this
-      // information from valiadtor sets pallet.
+      // information from validator sets pallet.
       Self::update_participants();
       Weight::zero() // TODO
     }
@@ -365,6 +368,18 @@ pub mod pallet {
         if EconomicSecurity::<T>::economic_security_block(n).is_some() {
           Err(Error::<T>::NetworkHasEconomicSecurity)?;
         }
+      } else {
+        // we target 20% of the network's stake to be behind the Serai network
+        let mut total_stake = 0;
+        for n in NETWORKS {
+          total_stake += ValidatorSets::<T>::total_allocated_stake(n).unwrap_or(Amount(0)).0;
+        }
+
+        let stake = ValidatorSets::<T>::total_allocated_stake(network).unwrap_or(Amount(0)).0;
+        let desired_stake = total_stake / (100 / SERAI_VALIDATORS_DESIRED_PERCENTAGE);
+        if stake >= desired_stake {
+          Err(Error::<T>::NetworkHasEconomicSecurity)?;
+        }
       }
 
       // swap half of the liquidity for SRI to form PoL.

substrate/in-instructions/primitives/src/lib.rs

@@ -2,7 +2,6 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use serai_primitives::ExternalBalance;
 #[cfg(feature = "std")]
 use zeroize::Zeroize;
 
@@ -21,7 +20,7 @@ use sp_std::vec::Vec;
 use sp_runtime::RuntimeDebug;
 
 #[rustfmt::skip]
-use serai_primitives::{BlockHash, Balance, ExternalNetworkId, NetworkId, SeraiAddress, ExternalAddress, system_address};
+use serai_primitives::{BlockHash, Balance, ExternalNetworkId, NetworkId, SeraiAddress, ExternalBalance, ExternalAddress, system_address};
 
 mod shorthand;
 pub use shorthand::*;

substrate/primitives/src/networks.rs

@@ -1,7 +1,7 @@
 #[cfg(feature = "std")]
 use zeroize::Zeroize;
 
-use scale::{Encode, Decode, MaxEncodedLen};
+use scale::{Decode, Encode, EncodeLike, MaxEncodedLen};
 use scale_info::TypeInfo;
 
 #[cfg(feature = "borsh")]
@@ -16,11 +16,8 @@ use sp_std::{vec, vec::Vec};
 use crate::{borsh_serialize_bounded_vec, borsh_deserialize_bounded_vec};
 
 /// The type used to identify external networks.
-#[derive(
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord, TypeInfo)]
-  Clone, Copy, PartialEq, Eq, Hash, Debug, Encode, Decode, PartialOrd, Ord, MaxEncodedLen, TypeInfo,
-)]
 #[cfg_attr(feature = "std", derive(Zeroize))]
-#[cfg_attr(feature = "borsh", derive(BorshSerialize, BorshDeserialize))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum ExternalNetworkId {
   Bitcoin,
@@ -28,18 +25,105 @@ pub enum ExternalNetworkId {
   Monero,
 }
 
+impl Encode for ExternalNetworkId {
+  fn encode(&self) -> Vec<u8> {
+    match self {
+      ExternalNetworkId::Bitcoin => vec![1],
+      ExternalNetworkId::Ethereum => vec![2],
+      ExternalNetworkId::Monero => vec![3],
+    }
+  }
+}
+
+impl Decode for ExternalNetworkId {
+  fn decode<I: scale::Input>(input: &mut I) -> Result<Self, scale::Error> {
+    let kind = input.read_byte()?;
+    match kind {
+      1 => Ok(Self::Bitcoin),
+      2 => Ok(Self::Ethereum),
+      3 => Ok(Self::Monero),
+      _ => Err(scale::Error::from("invalid format")),
+    }
+  }
+}
+
+impl MaxEncodedLen for ExternalNetworkId {
+  fn max_encoded_len() -> usize {
+    1
+  }
+}
+
+impl EncodeLike for ExternalNetworkId {}
+
+#[cfg(feature = "borsh")]
+impl BorshSerialize for ExternalNetworkId {
+  fn serialize<W: std::io::Write>(&self, writer: &mut W) -> std::io::Result<()> {
+    writer.write_all(&self.encode())
+  }
+}
+
+#[cfg(feature = "borsh")]
+impl BorshDeserialize for ExternalNetworkId {
+  fn deserialize_reader<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self> {
+    let mut kind = [0; 1];
+    reader.read_exact(&mut kind)?;
+    ExternalNetworkId::decode(&mut kind.as_slice())
+      .map_err(|_| std::io::Error::other("invalid format"))
+  }
+}
+
 /// The type used to identify networks.
-#[derive(
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord, TypeInfo)]
-  Clone, Copy, PartialEq, Eq, Hash, Debug, Encode, Decode, PartialOrd, Ord, MaxEncodedLen, TypeInfo,
-)]
 #[cfg_attr(feature = "std", derive(Zeroize))]
-#[cfg_attr(feature = "borsh", derive(BorshSerialize, BorshDeserialize))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum NetworkId {
   Serai,
   External(ExternalNetworkId),
 }
 
+impl Encode for NetworkId {
+  fn encode(&self) -> Vec<u8> {
+    match self {
+      NetworkId::Serai => vec![0],
+      NetworkId::External(network) => network.encode(),
+    }
+  }
+}
+
+impl Decode for NetworkId {
+  fn decode<I: scale::Input>(input: &mut I) -> Result<Self, scale::Error> {
+    let kind = input.read_byte()?;
+    match kind {
+      0 => Ok(Self::Serai),
+      _ => Ok(ExternalNetworkId::decode(input)?.into()),
+    }
+  }
+}
+
+impl MaxEncodedLen for NetworkId {
+  fn max_encoded_len() -> usize {
+    1
+  }
+}
+
+impl EncodeLike for NetworkId {}
+
+#[cfg(feature = "borsh")]
+impl BorshSerialize for NetworkId {
+  fn serialize<W: std::io::Write>(&self, writer: &mut W) -> std::io::Result<()> {
+    writer.write_all(&self.encode())
+  }
+}
+
+#[cfg(feature = "borsh")]
+impl BorshDeserialize for NetworkId {
+  fn deserialize_reader<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self> {
+    let mut kind = [0; 1];
+    reader.read_exact(&mut kind)?;
+    NetworkId::decode(&mut kind.as_slice()).map_err(|_| std::io::Error::other("invalid format"))
+  }
+}
+
 impl ExternalNetworkId {
   pub fn coins(&self) -> Vec<ExternalCoin> {
     match self {
@@ -63,11 +147,7 @@ impl NetworkId {
 
 impl From<ExternalNetworkId> for NetworkId {
   fn from(network: ExternalNetworkId) -> Self {
-    match network {
+    NetworkId::External(network)
-      ExternalNetworkId::Bitcoin => Self::External(ExternalNetworkId::Bitcoin),
-      ExternalNetworkId::Ethereum => Self::External(ExternalNetworkId::Ethereum),
-      ExternalNetworkId::Monero => Self::External(ExternalNetworkId::Monero),
-    }
   }
 }
 
@@ -103,24 +183,9 @@ pub const COINS: [Coin; 5] = [
   Coin::External(ExternalCoin::Monero),
 ];
 
-/// The type used to identify coins.
-#[derive(
-  Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Encode, Decode, MaxEncodedLen, TypeInfo,
-)]
-#[cfg_attr(feature = "std", derive(Zeroize))]
-#[cfg_attr(feature = "borsh", derive(BorshSerialize, BorshDeserialize))]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub enum Coin {
-  Serai,
-  External(ExternalCoin),
-}
-
 /// The type used to identify external coins.
-#[derive(
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, TypeInfo)]
-  Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Encode, Decode, MaxEncodedLen, TypeInfo,
-)]
 #[cfg_attr(feature = "std", derive(Zeroize))]
-#[cfg_attr(feature = "borsh", derive(BorshSerialize, BorshDeserialize))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum ExternalCoin {
   Bitcoin,
@@ -129,14 +194,108 @@ pub enum ExternalCoin {
   Monero,
 }
 
+impl Encode for ExternalCoin {
+  fn encode(&self) -> Vec<u8> {
+    match self {
+      ExternalCoin::Bitcoin => vec![4],
+      ExternalCoin::Ether => vec![5],
+      ExternalCoin::Dai => vec![6],
+      ExternalCoin::Monero => vec![7],
+    }
+  }
+}
+
+impl Decode for ExternalCoin {
+  fn decode<I: scale::Input>(input: &mut I) -> Result<Self, scale::Error> {
+    let kind = input.read_byte()?;
+    match kind {
+      4 => Ok(Self::Bitcoin),
+      5 => Ok(Self::Ether),
+      6 => Ok(Self::Dai),
+      7 => Ok(Self::Monero),
+      _ => Err(scale::Error::from("invalid format")),
+    }
+  }
+}
+impl MaxEncodedLen for ExternalCoin {
+  fn max_encoded_len() -> usize {
+    1
+  }
+}
+
+impl EncodeLike for ExternalCoin {}
+
+#[cfg(feature = "borsh")]
+impl BorshSerialize for ExternalCoin {
+  fn serialize<W: std::io::Write>(&self, writer: &mut W) -> std::io::Result<()> {
+    writer.write_all(&self.encode())
+  }
+}
+
+#[cfg(feature = "borsh")]
+impl BorshDeserialize for ExternalCoin {
+  fn deserialize_reader<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self> {
+    let mut kind = [0; 1];
+    reader.read_exact(&mut kind)?;
+    ExternalCoin::decode(&mut kind.as_slice()).map_err(|_| std::io::Error::other("invalid format"))
+  }
+}
+
+/// The type used to identify coins.
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, TypeInfo)]
+#[cfg_attr(feature = "std", derive(Zeroize))]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub enum Coin {
+  Serai,
+  External(ExternalCoin),
+}
+
+impl Encode for Coin {
+  fn encode(&self) -> Vec<u8> {
+    match self {
+      Coin::Serai => vec![0],
+      Coin::External(ec) => ec.encode(),
+    }
+  }
+}
+
+impl Decode for Coin {
+  fn decode<I: scale::Input>(input: &mut I) -> Result<Self, scale::Error> {
+    let kind = input.read_byte()?;
+    match kind {
+      0 => Ok(Self::Serai),
+      _ => Ok(ExternalCoin::decode(input)?.into()),
+    }
+  }
+}
+
+impl MaxEncodedLen for Coin {
+  fn max_encoded_len() -> usize {
+    1
+  }
+}
+
+impl EncodeLike for Coin {}
+
+#[cfg(feature = "borsh")]
+impl BorshSerialize for Coin {
+  fn serialize<W: std::io::Write>(&self, writer: &mut W) -> std::io::Result<()> {
+    writer.write_all(&self.encode())
+  }
+}
+
+#[cfg(feature = "borsh")]
+impl BorshDeserialize for Coin {
+  fn deserialize_reader<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self> {
+    let mut kind = [0; 1];
+    reader.read_exact(&mut kind)?;
+    Coin::decode(&mut kind.as_slice()).map_err(|_| std::io::Error::other("invalid format"))
+  }
+}
+
 impl From<ExternalCoin> for Coin {
   fn from(coin: ExternalCoin) -> Self {
-    match coin {
+    Coin::External(coin)
-      ExternalCoin::Bitcoin => Self::External(ExternalCoin::Bitcoin),
-      ExternalCoin::Ether => Self::External(ExternalCoin::Ether),
-      ExternalCoin::Dai => Self::External(ExternalCoin::Dai),
-      ExternalCoin::Monero => Self::External(ExternalCoin::Monero),
-    }
   }
 }
 

substrate/validator-sets/pallet/src/lib.rs

@@ -171,7 +171,6 @@ pub mod pallet {
     ///
     /// This will still include participants which were removed from the DKG.
     pub fn in_set(network: NetworkId, account: Public) -> bool {
-      // TODO: should InSet only work for `ExternalNetworkId`?
       if InSet::<T>::contains_key(network, account) {
         return true;
       }
@@ -695,23 +694,23 @@ pub mod pallet {
         return false;
       }
 
-      if let NetworkId::External(n) = network {
+      let NetworkId::External(n) = network else {
-        // The current session must have set keys for its handover to be completed
-        if !Keys::<T>::contains_key(ExternalValidatorSet { network: n, session }) {
-          return false;
-        }
-
-        // This must be the first session (which has set keys) OR the prior session must have been
-        // retired (signified by its keys no longer being present)
-        (session.0 == 0) ||
-          (!Keys::<T>::contains_key(ExternalValidatorSet {
-            network: n,
-            session: Session(session.0 - 1),
-          }))
-      } else {
         // Handover is automatically complete for Serai as it doesn't have a handover protocol
-        true
+        return true;
+      };
+
+      // The current session must have set keys for its handover to be completed
+      if !Keys::<T>::contains_key(ExternalValidatorSet { network: n, session }) {
+        return false;
       }
+
+      // This must be the first session (which has set keys) OR the prior session must have been
+      // retired (signified by its keys no longer being present)
+      (session.0 == 0) ||
+        (!Keys::<T>::contains_key(ExternalValidatorSet {
+          network: n,
+          session: Session(session.0 - 1),
+        }))
     }
 
     fn new_session() {
@@ -744,9 +743,6 @@ pub mod pallet {
       // Serai doesn't set keys and network slashes are handled by BABE/GRANDPA
       if let NetworkId::External(n) = set.network {
         // If the prior prior set didn't report, emit they're retired now
-        // TODO: we will emit the events 1 session late if there was no call to report_slashes.
-        // Also report_slashes calls must be made after the set publishes its first batch for this
-        // flow to work as expected.
         if PendingSlashReport::<T>::get(n).is_some() {
           Self::deposit_event(Event::SetRetired {
             set: ValidatorSet { network: set.network, session: Session(set.session.0 - 1) },