From 4ebfae0b63d0371a52f2e435ca627376321fe56f Mon Sep 17 00:00:00 2001 From: akildemir <34187742+akildemir@users.noreply.github.com> Date: Tue, 5 Dec 2023 16:52:50 +0300 Subject: [PATCH] Ensure economic security on validator sets (#459) * add price oracle * tidy up * add todo * bug fixes * fix pr comments * Use spot price, tweak some formulas Also cleans nits. --------- Co-authored-by: Luke Parker --- Cargo.lock | 1 + substrate/coins/pallet/src/lib.rs | 19 ++++ substrate/dex/pallet/src/lib.rs | 110 ++++++++++++++++++++- substrate/dex/pallet/src/mock.rs | 2 + substrate/dex/pallet/src/types.rs | 4 + substrate/primitives/src/networks.rs | 10 ++ substrate/runtime/src/lib.rs | 2 + substrate/validator-sets/pallet/Cargo.toml | 2 + substrate/validator-sets/pallet/src/lib.rs | 67 ++++++++++++- 9 files changed, 211 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 615e238b..a4d72ba3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7716,6 +7716,7 @@ dependencies = [ "parity-scale-codec", "scale-info", "serai-coins-pallet", + "serai-dex-pallet", "serai-primitives", "serai-validator-sets-primitives", "sp-application-crypto", diff --git a/substrate/coins/pallet/src/lib.rs b/substrate/coins/pallet/src/lib.rs index 71b924ef..b2a83eaf 100644 --- a/substrate/coins/pallet/src/lib.rs +++ b/substrate/coins/pallet/src/lib.rs @@ -1,7 +1,20 @@ #![cfg_attr(not(feature = "std"), no_std)] +use serai_primitives::{Coin, SubstrateAmount, Balance}; + +pub trait AllowMint { + fn is_allowed(balance: &Balance) -> bool; +} + +impl AllowMint for () { + fn is_allowed(_: &Balance) -> bool { + true + } +} + #[frame_support::pallet] pub mod pallet { + use super::*; use sp_std::{vec::Vec, any::TypeId}; use sp_core::sr25519::Public; use sp_runtime::{ @@ -23,6 +36,7 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; + type AllowMint: AllowMint; } #[pallet::genesis_config] @@ -43,6 +57,7 @@ pub mod pallet { AmountOverflowed, NotEnoughCoins, BurnWithInstructionNotAllowed, + MintNotAllowed, } #[pallet::event] @@ -142,6 +157,10 @@ pub mod pallet { /// /// Errors if any amount overflows. pub fn mint(to: Public, balance: Balance) -> Result<(), Error> { + if !T::AllowMint::is_allowed(&balance) { + Err(Error::::MintNotAllowed)?; + } + // update the balance Self::increase_balance_internal(to, balance)?; diff --git a/substrate/dex/pallet/src/lib.rs b/substrate/dex/pallet/src/lib.rs index 553903bd..ebbc2b05 100644 --- a/substrate/dex/pallet/src/lib.rs +++ b/substrate/dex/pallet/src/lib.rs @@ -88,7 +88,7 @@ pub use pallet::*; use sp_runtime::{traits::TrailingZeroInput, DispatchError}; -use serai_primitives::{Coin, SubstrateAmount}; +use serai_primitives::{NetworkId, Coin, SubstrateAmount}; use sp_std::prelude::*; pub use types::*; @@ -154,11 +154,42 @@ pub mod pallet { #[pallet::storage] pub type Pools = StorageMap<_, Blake2_128Concat, PoolId, PoolInfo, OptionQuery>; + #[pallet::storage] + #[pallet::getter(fn spot_price_for_block)] + pub type SpotPriceForBlock = + StorageDoubleMap<_, Identity, BlockNumberFor, Identity, Coin, [u8; 8], ValueQuery>; + + /// Moving window of oracle prices. + /// + /// The second [u8; 8] key is the amount's big endian bytes, and u16 is the amount of inclusions + /// in this multi-set. + #[pallet::storage] + #[pallet::getter(fn oracle_prices)] + pub type OraclePrices = + StorageDoubleMap<_, Identity, Coin, Identity, [u8; 8], u16, OptionQuery>; + impl Pallet { + // TODO: consider an algorithm which removes outliers? This algorithm might work a good bit + // better if we remove the bottom n values (so some value sustained over 90% of blocks instead + // of all blocks in the window). + /// Get the highest sustained value for this window. + /// This is actually the lowest price observed during the windows, as it's the price + /// all prices are greater than or equal to. + pub fn highest_sustained_price(coin: &Coin) -> Option { + let mut iter = OraclePrices::::iter_key_prefix(coin); + // the first key will be the lowest price due to the keys being lexicographically ordered. + iter.next().map(|amount| Amount(u64::from_be_bytes(amount))) + } + } + + #[pallet::storage] + #[pallet::getter(fn oracle_value)] + pub type OracleValue = StorageMap<_, Identity, Coin, Amount, OptionQuery>; + // Pallet's events. #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - /// A successful call of the `CretaPool` extrinsic will create this event. + /// A successful call of the `CreatePool` extrinsic will create this event. PoolCreated { /// The pool id associated with the pool. Note that the order of the coins may not be /// the same as the order specified in the create pool extrinsic. @@ -240,6 +271,13 @@ pub mod pallet { #[pallet::genesis_build] impl BuildGenesisConfig for GenesisConfig { fn build(&self) { + // assert that oracle windows size can fit into u16. Otherwise number of observants + // for a price in the `OraclePrices` map can overflow + // We don't want to make this const directly a u16 because it is used the block number + // calculations (which are done as u32s) + u16::try_from(ORACLE_WINDOW_SIZE).unwrap(); + + // create the pools for coin in &self.pools { Pallet::::create_pool(*coin).unwrap(); } @@ -311,8 +349,64 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - fn integrity_test() { - assert!(T::MaxSwapPathLength::get() > 1, "the `MaxSwapPathLength` should be greater than 1",); + fn on_finalize(n: BlockNumberFor) { + // we run this on on_finalize because we want to use the last price of the block for a coin. + // This prevents the exploit where a malicious block proposer spikes the price in either + // direction, then includes a swap in the other direction (ensuring they don't get arbitraged + // against) + // Since they'll have to leave the spike present at the end of the block, making the next + // block the one to include any arbitrage transactions (which there's no guarantee they'll + // produce), this cannot be done in a way without significant risk + for coin in Pools::::iter_keys() { + // insert the new price to our oracle window + // The spot price for 1 coin, in atomic units, to SRI is used + let pool_id = Self::get_pool_id(coin, Coin::native()).ok().unwrap(); + let pool_account = Self::get_pool_account(pool_id); + let sri_balance = Self::get_balance(&pool_account, Coin::native()); + let coin_balance = Self::get_balance(&pool_account, coin); + // We use 1 coin to handle rounding errors which may occur with atomic units + // If we used atomic units, any coin whose atomic unit is worth less than SRI's atomic unit + // would cause a 'price' of 0 + // If the decimals aren't large enough to provide sufficient buffer, use 10,000 + let coin_decimals = coin.decimals().max(5); + let accuracy_increase = + HigherPrecisionBalance::from(SubstrateAmount::pow(10, coin_decimals)); + let sri_per_coin = u64::try_from( + accuracy_increase * HigherPrecisionBalance::from(sri_balance) / + HigherPrecisionBalance::from(coin_balance), + ) + .unwrap_or(u64::MAX); + let sri_per_coin = sri_per_coin.to_be_bytes(); + + SpotPriceForBlock::::set(n, coin, sri_per_coin); + + // Include this spot price into the multiset + { + let observed = OraclePrices::::get(coin, sri_per_coin).unwrap_or(0); + OraclePrices::::set(coin, sri_per_coin, Some(observed + 1)); + } + + // pop the earliest key from the window once we reach its full size. + if n >= ORACLE_WINDOW_SIZE.into() { + let start_of_window = n - ORACLE_WINDOW_SIZE.into(); + let start_spot_price = Self::spot_price_for_block(start_of_window, coin); + SpotPriceForBlock::::remove(start_of_window, coin); + // Remove this price from the multiset + OraclePrices::::mutate_exists(coin, start_spot_price, |v| { + *v = Some(v.unwrap() - 1); + if *v == Some(0) { + *v = None; + } + }); + } + + // update the oracle value + let highest_sustained = Self::highest_sustained_price(&coin).unwrap_or(Amount(0)); + let oracle_value = Self::oracle_value(coin).unwrap_or(Amount(0)); + if highest_sustained > oracle_value { + OracleValue::::set(coin, Some(highest_sustained)); + } + } } } @@ -338,6 +432,14 @@ pub mod pallet { Ok(()) } + + /// A hook to be called whenever a network's session is rotated. + pub fn on_new_session(network: NetworkId) { + // reset the oracle value + for coin in network.coins() { + OracleValue::::set(*coin, Self::highest_sustained_price(coin)); + } + } } /// Pallet's callable functions. diff --git a/substrate/dex/pallet/src/mock.rs b/substrate/dex/pallet/src/mock.rs index 9bc30274..8c1863e1 100644 --- a/substrate/dex/pallet/src/mock.rs +++ b/substrate/dex/pallet/src/mock.rs @@ -78,10 +78,12 @@ impl frame_system::Config for Test { impl coins::Config for Test { type RuntimeEvent = RuntimeEvent; + type AllowMint = (); } impl coins::Config for Test { type RuntimeEvent = RuntimeEvent; + type AllowMint = (); } impl Config for Test { diff --git a/substrate/dex/pallet/src/types.rs b/substrate/dex/pallet/src/types.rs index 8be6af56..94dd340e 100644 --- a/substrate/dex/pallet/src/types.rs +++ b/substrate/dex/pallet/src/types.rs @@ -23,6 +23,10 @@ use super::*; use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; +/// This needs to be long enough for arbitrage to occur and make holding +/// any fake price up sufficiently unprofitable. +pub const ORACLE_WINDOW_SIZE: u32 = 1000; + /// Stores the lp_token coin id a particular pool has been assigned. #[derive(Decode, Encode, Default, PartialEq, Eq, MaxEncodedLen, TypeInfo)] pub struct PoolInfo { diff --git a/substrate/primitives/src/networks.rs b/substrate/primitives/src/networks.rs index 80e5f3cb..51ba7e54 100644 --- a/substrate/primitives/src/networks.rs +++ b/substrate/primitives/src/networks.rs @@ -25,6 +25,16 @@ pub enum NetworkId { Ethereum, Monero, } +impl NetworkId { + pub fn coins(&self) -> &'static [Coin] { + match self { + Self::Serai => &[Coin::Serai], + Self::Bitcoin => &[Coin::Bitcoin], + Self::Ethereum => &[Coin::Ether, Coin::Dai], + Self::Monero => &[Coin::Monero], + } + } +} pub const NETWORKS: [NetworkId; 4] = [NetworkId::Serai, NetworkId::Bitcoin, NetworkId::Ethereum, NetworkId::Monero]; diff --git a/substrate/runtime/src/lib.rs b/substrate/runtime/src/lib.rs index 2541d579..91b821ba 100644 --- a/substrate/runtime/src/lib.rs +++ b/substrate/runtime/src/lib.rs @@ -224,10 +224,12 @@ impl transaction_payment::Config for Runtime { impl coins::Config for Runtime { type RuntimeEvent = RuntimeEvent; + type AllowMint = ValidatorSets; } impl coins::Config for Runtime { type RuntimeEvent = RuntimeEvent; + type AllowMint = (); } impl dex::Config for Runtime { diff --git a/substrate/validator-sets/pallet/Cargo.toml b/substrate/validator-sets/pallet/Cargo.toml index d30df1df..1974abbe 100644 --- a/substrate/validator-sets/pallet/Cargo.toml +++ b/substrate/validator-sets/pallet/Cargo.toml @@ -33,6 +33,7 @@ serai-primitives = { path = "../../primitives", default-features = false } validator-sets-primitives = { package = "serai-validator-sets-primitives", path = "../primitives", default-features = false } coins-pallet = { package = "serai-coins-pallet", path = "../../coins/pallet", default-features = false } +dex-pallet = { package = "serai-dex-pallet", path = "../../dex/pallet", default-features = false } [features] std = [ @@ -56,6 +57,7 @@ std = [ "validator-sets-primitives/std", "coins-pallet/std", + "dex-pallet/std", ] runtime-benchmarks = [ diff --git a/substrate/validator-sets/pallet/src/lib.rs b/substrate/validator-sets/pallet/src/lib.rs index 429aa501..8d73b932 100644 --- a/substrate/validator-sets/pallet/src/lib.rs +++ b/substrate/validator-sets/pallet/src/lib.rs @@ -22,7 +22,8 @@ pub mod pallet { pub use validator_sets_primitives as primitives; use primitives::*; - use coins_pallet::Pallet as Coins; + use coins_pallet::{Pallet as Coins, AllowMint}; + use dex_pallet::Pallet as Dex; use pallet_babe::{Pallet as Babe, AuthorityId as BabeAuthorityId}; use pallet_grandpa::{Pallet as Grandpa, AuthorityId as GrandpaAuthorityId}; @@ -31,6 +32,7 @@ pub mod pallet { pub trait Config: frame_system::Config + coins_pallet::Config + + dex_pallet::Config + pallet_babe::Config + pallet_grandpa::Config + TypeInfo @@ -333,6 +335,8 @@ pub mod pallet { impl Pallet { fn new_set(network: NetworkId) { + // TODO: prevent new set if it doesn't have enough stake for economic security. + // Update CurrentSession let session = { let new_session = CurrentSession::::get(network) @@ -411,6 +415,8 @@ pub mod pallet { BadSignature, /// Validator wasn't registered or active. NonExistentValidator, + /// Deallocation would take the stake below what is required. + DeallocationWouldRemoveEconomicSecurity, } #[pallet::hooks] @@ -560,7 +566,16 @@ pub mod pallet { account: T::AccountId, amount: Amount, ) -> Result { - // TODO: Check it's safe to decrease this set's stake by this amount + // Check it's safe to decrease this set's stake by this amount + let new_total_staked = Self::total_allocated_stake(network) + .unwrap() + .0 + .checked_sub(amount.0) + .ok_or(Error::::NotEnoughAllocated)?; + let required_stake = Self::required_stake_for_network(network); + if new_total_staked < required_stake { + Err(Error::::DeallocationWouldRemoveEconomicSecurity)?; + } let old_allocation = Self::allocation((network, account)).ok_or(Error::::NonExistentValidator)?.0; @@ -680,6 +695,8 @@ pub mod pallet { // - The current set was actually established with a completed handover protocol if (network == NetworkId::Serai) || Self::handover_completed(network, current_session) { Pallet::::new_set(network); + // let the Dex know session is rotated. + Dex::::on_new_session(network); } } } @@ -739,6 +756,39 @@ pub mod pallet { validators.into_iter().map(|(id, w)| (GrandpaAuthorityId::from(id), w)).collect(), ); } + + /// Returns the required stake in terms SRI for a given `Balance`. + pub fn required_stake(balance: &Balance) -> SubstrateAmount { + use dex_pallet::HigherPrecisionBalance; + + // This is inclusive to an increase in accuracy + let sri_per_coin = Dex::::oracle_value(balance.coin).unwrap_or(Amount(0)); + + // See dex-pallet for the reasoning on these + let coin_decimals = balance.coin.decimals().max(5); + let accuracy_increase = HigherPrecisionBalance::from(SubstrateAmount::pow(10, coin_decimals)); + + let total_coin_value = u64::try_from( + HigherPrecisionBalance::from(balance.amount.0) * + HigherPrecisionBalance::from(sri_per_coin.0) / + accuracy_increase, + ) + .unwrap_or(u64::MAX); + + // required stake formula (COIN_VALUE * 1.5) + margin(20%) + let required_stake = total_coin_value.saturating_mul(3).saturating_div(2); + required_stake.saturating_add(total_coin_value.saturating_div(5)) + } + + /// Returns the current total required stake for a given `network`. + pub fn required_stake_for_network(network: NetworkId) -> SubstrateAmount { + let mut total_required = 0; + for coin in network.coins() { + let supply = Coins::::supply(coin); + total_required += Self::required_stake(&Balance { coin: *coin, amount: Amount(supply) }); + } + total_required + } } #[pallet::call] @@ -866,6 +916,7 @@ pub mod pallet { Err(Error::DeallocationWouldRemoveFaultTolerance) | Err(Error::NonExistentDeallocation) | Err(Error::NonExistentValidator) | + Err(Error::DeallocationWouldRemoveEconomicSecurity) | Err(Error::BadSignature) => Err(InvalidTransaction::BadProof)?, Err(Error::__Ignore(_, _)) => unreachable!(), Ok(()) => (), @@ -982,6 +1033,18 @@ pub mod pallet { } } + impl AllowMint for Pallet { + fn is_allowed(balance: &Balance) -> bool { + // get the required stake + let current_required = Self::required_stake_for_network(balance.coin.network()); + let new_required = current_required + Self::required_stake(balance); + + // get the total stake for the network & compare. + let staked = Self::total_allocated_stake(balance.coin.network()).unwrap_or(Amount(0)); + staked.0 >= new_required + } + } + impl DisabledValidators for Pallet { fn is_disabled(_: u32) -> bool { // TODO