From 1b587548d197c4976d26cf2dc1f0450a4ac04956 Mon Sep 17 00:00:00 2001 From: akildemir Date: Thu, 3 Oct 2024 15:40:24 +0300 Subject: [PATCH] fix pr comments --- coordinator/src/cosign_evaluator.rs | 6 +- coordinator/src/substrate/mod.rs | 21 +- substrate/coins/pallet/src/lib.rs | 8 +- substrate/dex/pallet/src/lib.rs | 10 +- substrate/emissions/pallet/src/lib.rs | 81 +++--- .../in-instructions/primitives/src/lib.rs | 3 +- substrate/primitives/src/networks.rs | 231 +++++++++++++++--- substrate/validator-sets/pallet/src/lib.rs | 34 ++- 8 files changed, 272 insertions(+), 122 deletions(-) diff --git a/coordinator/src/cosign_evaluator.rs b/coordinator/src/cosign_evaluator.rs index a1c2f5f6..84436008 100644 --- a/coordinator/src/cosign_evaluator.rs +++ b/coordinator/src/cosign_evaluator.rs @@ -204,14 +204,12 @@ impl CosignEvaluator { 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? - // why are we going through the networks here? - for _network in EXTERNAL_NETWORKS { + 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!( diff --git a/coordinator/src/substrate/mod.rs b/coordinator/src/substrate/mod.rs index c747e396..a10806a3 100644 --- a/coordinator/src/substrate/mod.rs +++ b/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( 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 { - continue; - } - - let set: ExternalValidatorSet = set.try_into().unwrap(); + let Ok(set) = ExternalValidatorSet::try_from(set) else { continue }; 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( panic!("AcceptedHandover event wasn't AcceptedHandover: {accepted_handover:?}"); }; - if set.network == NetworkId::Serai { - continue; - } - - let set: ExternalValidatorSet = set.try_into().unwrap(); + let Ok(set) = ExternalValidatorSet::try_from(set) else { continue }; 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( panic!("SetRetired event wasn't SetRetired: {retired_set:?}"); }; - if set.network == NetworkId::Serai { - continue; - } - - let set: ExternalValidatorSet = set.try_into().unwrap(); + let Ok(set) = ExternalValidatorSet::try_from(set) else { continue }; if HandledEvent::is_unhandled(db, hash, event_id) { log::info!("found fresh set retired event {:?}", retired_set); let mut txn = db.txn(); diff --git a/substrate/coins/pallet/src/lib.rs b/substrate/coins/pallet/src/lib.rs index dcecdd29..dd64b2b6 100644 --- a/substrate/coins/pallet/src/lib.rs +++ b/substrate/coins/pallet/src/lib.rs @@ -161,11 +161,9 @@ pub mod pallet { pub fn mint(to: Public, balance: Balance) -> Result<(), Error> { // 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() && - (!T::AllowMint::is_allowed(&ExternalBalance { - coin: balance.coin.try_into().unwrap(), - amount: balance.amount, - })) + if !ExternalCoin::try_from(balance.coin) + .map(|coin| T::AllowMint::is_allowed(&ExternalBalance { coin, amount: balance.amount })) + .unwrap_or(true) { Err(Error::::MintNotAllowed)?; } diff --git a/substrate/dex/pallet/src/lib.rs b/substrate/dex/pallet/src/lib.rs index 2a69fd24..8da63a7b 100644 --- a/substrate/dex/pallet/src/lib.rs +++ b/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::::set(coin, Self::median_price(coin)); @@ -936,11 +936,9 @@ pub mod pallet { pub fn get_pool_id(coin1: Coin, coin2: Coin) -> Result> { ensure!((coin1 == Coin::Serai) || (coin2 == Coin::Serai), Error::::PoolNotFound); ensure!(coin1 != coin2, Error::::EqualCoins); - if coin1 == Coin::Serai { - Ok(coin2.try_into().unwrap()) - } else { - Ok(coin1.try_into().unwrap()) - } + ExternalCoin::try_from(coin1) + .or_else(|()| ExternalCoin::try_from(coin2)) + .map_err(|()| Error::::PoolNotFound) } /// Returns the balance of each coin in the pool. diff --git a/substrate/emissions/pallet/src/lib.rs b/substrate/emissions/pallet/src/lib.rs index 7b708a56..99e22e8b 100644 --- a/substrate/emissions/pallet/src/lib.rs +++ b/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 { - (reward, 0) - } else { + let validators_reward = if let NetworkId::External(external_network) = n { // calculate pool vs validator share - let capacity = ValidatorSets::::total_allocated_stake(n).unwrap_or(Amount(0)).0; - let required = ValidatorSets::::required_stake_for_network(n.try_into().unwrap()); + let capacity = + ValidatorSets::::total_allocated_stake(NetworkId::from(external_network)) + .unwrap_or(Amount(0)) + .0; + let required = ValidatorSets::::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::::mint( + Dex::::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::::mint( - Dex::::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::::economic_security_block(n).is_some() { Err(Error::::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::::total_allocated_stake(n).unwrap_or(Amount(0)).0; + } + + let stake = ValidatorSets::::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::::NetworkHasEconomicSecurity)?; + } } // swap half of the liquidity for SRI to form PoL. diff --git a/substrate/in-instructions/primitives/src/lib.rs b/substrate/in-instructions/primitives/src/lib.rs index b5b8c626..3944fd73 100644 --- a/substrate/in-instructions/primitives/src/lib.rs +++ b/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::*; diff --git a/substrate/primitives/src/networks.rs b/substrate/primitives/src/networks.rs index 412a5a74..285f2947 100644 --- a/substrate/primitives/src/networks.rs +++ b/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( - Clone, Copy, PartialEq, Eq, Hash, Debug, Encode, Decode, PartialOrd, Ord, MaxEncodedLen, TypeInfo, -)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord, 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 { + match self { + ExternalNetworkId::Bitcoin => vec![1], + ExternalNetworkId::Ethereum => vec![2], + ExternalNetworkId::Monero => vec![3], + } + } +} + +impl Decode for ExternalNetworkId { + fn decode(input: &mut I) -> Result { + 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(&self, writer: &mut W) -> std::io::Result<()> { + writer.write_all(&self.encode()) + } +} + +#[cfg(feature = "borsh")] +impl BorshDeserialize for ExternalNetworkId { + fn deserialize_reader(reader: &mut R) -> std::io::Result { + 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( - Clone, Copy, PartialEq, Eq, Hash, Debug, Encode, Decode, PartialOrd, Ord, MaxEncodedLen, TypeInfo, -)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord, 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 { + match self { + NetworkId::Serai => vec![0], + NetworkId::External(network) => network.encode(), + } + } +} + +impl Decode for NetworkId { + fn decode(input: &mut I) -> Result { + 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(&self, writer: &mut W) -> std::io::Result<()> { + writer.write_all(&self.encode()) + } +} + +#[cfg(feature = "borsh")] +impl BorshDeserialize for NetworkId { + fn deserialize_reader(reader: &mut R) -> std::io::Result { + 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 { match self { @@ -63,11 +147,7 @@ impl NetworkId { impl From for NetworkId { fn from(network: ExternalNetworkId) -> Self { - match network { - ExternalNetworkId::Bitcoin => Self::External(ExternalNetworkId::Bitcoin), - ExternalNetworkId::Ethereum => Self::External(ExternalNetworkId::Ethereum), - ExternalNetworkId::Monero => Self::External(ExternalNetworkId::Monero), - } + NetworkId::External(network) } } @@ -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( - Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Encode, Decode, MaxEncodedLen, TypeInfo, -)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, 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 { + match self { + ExternalCoin::Bitcoin => vec![4], + ExternalCoin::Ether => vec![5], + ExternalCoin::Dai => vec![6], + ExternalCoin::Monero => vec![7], + } + } +} + +impl Decode for ExternalCoin { + fn decode(input: &mut I) -> Result { + 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(&self, writer: &mut W) -> std::io::Result<()> { + writer.write_all(&self.encode()) + } +} + +#[cfg(feature = "borsh")] +impl BorshDeserialize for ExternalCoin { + fn deserialize_reader(reader: &mut R) -> std::io::Result { + 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 { + match self { + Coin::Serai => vec![0], + Coin::External(ec) => ec.encode(), + } + } +} + +impl Decode for Coin { + fn decode(input: &mut I) -> Result { + 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(&self, writer: &mut W) -> std::io::Result<()> { + writer.write_all(&self.encode()) + } +} + +#[cfg(feature = "borsh")] +impl BorshDeserialize for Coin { + fn deserialize_reader(reader: &mut R) -> std::io::Result { + 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 for Coin { fn from(coin: ExternalCoin) -> Self { - match 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), - } + Coin::External(coin) } } diff --git a/substrate/validator-sets/pallet/src/lib.rs b/substrate/validator-sets/pallet/src/lib.rs index f343b80b..383f94a7 100644 --- a/substrate/validator-sets/pallet/src/lib.rs +++ b/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::::contains_key(network, account) { return true; } @@ -695,23 +694,23 @@ pub mod pallet { return false; } - if let NetworkId::External(n) = network { - // The current session must have set keys for its handover to be completed - if !Keys::::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::::contains_key(ExternalValidatorSet { - network: n, - session: Session(session.0 - 1), - })) - } else { + let NetworkId::External(n) = network 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::::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::::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::::get(n).is_some() { Self::deposit_event(Event::SetRetired { set: ValidatorSet { network: set.network, session: Session(set.session.0 - 1) },