From 0ea90d054d5aa388c2e95291bc85612339fe63de Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 24 Nov 2023 19:56:57 -0500 Subject: [PATCH] Enable the signals pallet to halt networks' publication of `Batch`s Relevant to #394. Prevents hand-over due to hand-over occurring via a `Batch` publication. Expects a new protocol to restore functionality (after a retirement of the current protocol). --- Cargo.lock | 1 + substrate/in-instructions/pallet/src/lib.rs | 17 +- substrate/signals/pallet/Cargo.toml | 2 + substrate/signals/pallet/src/lib.rs | 274 ++++++++++++++------ 4 files changed, 207 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 67337518..2818e931 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7916,6 +7916,7 @@ dependencies = [ "frame-system", "parity-scale-codec", "scale-info", + "serai-in-instructions-pallet", "serai-primitives", "serai-validator-sets-pallet", "sp-core", diff --git a/substrate/in-instructions/pallet/src/lib.rs b/substrate/in-instructions/pallet/src/lib.rs index 88fb2471..5054466b 100644 --- a/substrate/in-instructions/pallet/src/lib.rs +++ b/substrate/in-instructions/pallet/src/lib.rs @@ -55,6 +55,7 @@ pub mod pallet { pub enum Event { Batch { network: NetworkId, id: u32, block: BlockHash, instructions_hash: [u8; 32] }, InstructionFailure { network: NetworkId, id: u32, index: u32 }, + Halt { network: NetworkId }, } #[pallet::error] @@ -77,6 +78,10 @@ pub mod pallet { pub(crate) type LastBatchBlock = StorageMap<_, Blake2_256, NetworkId, BlockNumberFor, OptionQuery>; + // Halted networks. + #[pallet::storage] + pub(crate) type Halted = StorageMap<_, Blake2_256, NetworkId, (), OptionQuery>; + // The latest block a network has acknowledged as finalized #[pallet::storage] #[pallet::getter(fn latest_network_block)] @@ -208,6 +213,12 @@ pub mod pallet { } Ok(()) } + + pub fn halt(network: NetworkId) -> Result<(), DispatchError> { + Halted::::set(network, Some(())); + Self::deposit_event(Event::Halt { network }); + Ok(()) + } } fn keys_for_network( @@ -301,6 +312,10 @@ pub mod pallet { Err(InvalidTransaction::BadProof)?; } + if Halted::::contains_key(network) { + Err(InvalidTransaction::Custom(1))?; + } + // If it wasn't valid by the prior key, meaning it was valid by the current key, the current // key is publishing `Batch`s. This should only happen once the current key has verified all // `Batch`s published by the prior key, meaning they are accepting the hand-over. @@ -341,7 +356,7 @@ pub mod pallet { // Accordingly, there's no value in writing code to fully slash the network, when such an // even would require a runtime upgrade to fully resolve anyways if instruction.balance.coin.network() != batch.batch.network { - Err(InvalidTransaction::Custom(1))?; + Err(InvalidTransaction::Custom(2))?; } } diff --git a/substrate/signals/pallet/Cargo.toml b/substrate/signals/pallet/Cargo.toml index 9296b6d0..bf2cb4dd 100644 --- a/substrate/signals/pallet/Cargo.toml +++ b/substrate/signals/pallet/Cargo.toml @@ -24,6 +24,7 @@ frame-support = { git = "https://github.com/serai-dex/substrate", default-featur serai-primitives = { path = "../../primitives", default-features = false } validator-sets-pallet = { package = "serai-validator-sets-pallet", path = "../../validator-sets/pallet", default-features = false } +in-instructions-pallet = { package = "serai-in-instructions-pallet", path = "../../in-instructions/pallet", default-features = false } [features] std = [ @@ -38,6 +39,7 @@ std = [ "serai-primitives/std", "validator-sets-pallet/std", + "in-instructions-pallet/std", ] runtime-benchmarks = [ diff --git a/substrate/signals/pallet/src/lib.rs b/substrate/signals/pallet/src/lib.rs index 53a62e8b..91717e5b 100644 --- a/substrate/signals/pallet/src/lib.rs +++ b/substrate/signals/pallet/src/lib.rs @@ -9,38 +9,74 @@ pub mod pallet { use sp_io::hashing::blake2_256; use frame_system::pallet_prelude::*; - use frame_support::pallet_prelude::*; + use frame_support::{pallet_prelude::*, sp_runtime}; use serai_primitives::*; use validator_sets_pallet::{primitives::ValidatorSet, Config as VsConfig, Pallet as VsPallet}; + use in_instructions_pallet::{Config as IiConfig, Pallet as InInstructions}; #[pallet::config] - pub trait Config: frame_system::Config + VsConfig + TypeInfo { + pub trait Config: + frame_system::Config + VsConfig + IiConfig + TypeInfo + { type RuntimeEvent: IsType<::RuntimeEvent> + From>; - type ValidityDuration: Get; - type LockInDuration: Get; + type RetirementValidityDuration: Get; + type RetirementLockInDuration: Get; + } + + #[pallet::genesis_config] + #[derive(Debug, Encode, Decode)] + pub struct GenesisConfig { + _config: PhantomData, + } + impl Default for GenesisConfig { + fn default() -> Self { + GenesisConfig { _config: PhantomData } + } + } + #[pallet::genesis_build] + impl BuildGenesisConfig for GenesisConfig { + fn build(&self) { + // Assert the validity duration is less than the lock-in duration so lock-in periods + // automatically invalidate other retirement signals + assert!(T::RetirementValidityDuration::get() < T::RetirementLockInDuration::get()); + } } #[pallet::pallet] pub struct Pallet(PhantomData); - #[derive(Clone, PartialEq, Eq, Encode, Decode, TypeInfo, MaxEncodedLen)] - struct RegisteredSignal { - signal: [u8; 32], + #[derive(Clone, Copy, PartialEq, Eq, Debug, Encode, Decode, TypeInfo, MaxEncodedLen)] + pub enum SignalId { + Retirement([u8; 32]), + Halt(NetworkId), + } + + #[derive(Clone, PartialEq, Eq, Debug, Encode, Decode, TypeInfo, MaxEncodedLen)] + pub struct RegisteredRetirementSignal { + in_favor_of: [u8; 32], registrant: T::AccountId, registed_at: BlockNumberFor, } + impl RegisteredRetirementSignal { + fn id(&self) -> [u8; 32] { + let mut preimage = b"Signal".to_vec(); + preimage.extend(&self.encode()); + blake2_256(&preimage) + } + } + #[pallet::storage] - type RegisteredSignals = - StorageMap<_, Blake2_128Concat, [u8; 32], RegisteredSignal, OptionQuery>; + type RegisteredRetirementSignals = + StorageMap<_, Blake2_128Concat, [u8; 32], RegisteredRetirementSignal, OptionQuery>; #[pallet::storage] pub type Favors = StorageDoubleMap< _, Blake2_128Concat, - ([u8; 32], NetworkId), + (SignalId, NetworkId), Blake2_128Concat, T::AccountId, (), @@ -49,31 +85,58 @@ pub mod pallet { #[pallet::storage] pub type SetsInFavor = - StorageMap<_, Blake2_128Concat, ([u8; 32], ValidatorSet), (), OptionQuery>; + StorageMap<_, Blake2_128Concat, (SignalId, ValidatorSet), (), OptionQuery>; #[pallet::storage] - pub type LockedInSignal = StorageValue<_, ([u8; 32], BlockNumberFor), OptionQuery>; + pub type LockedInRetirement = + StorageValue<_, ([u8; 32], BlockNumberFor), OptionQuery>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - SignalRegistered { signal_id: [u8; 32], signal: [u8; 32], registrant: T::AccountId }, - SignalRevoked { signal_id: [u8; 32] }, - SignalFavored { signal_id: [u8; 32], by: T::AccountId, for_network: NetworkId }, - SetInFavor { signal_id: [u8; 32], set: ValidatorSet }, - SignalLockedIn { signal_id: [u8; 32] }, - SetNoLongerInFavor { signal_id: [u8; 32], set: ValidatorSet }, - FavorRevoked { signal_id: [u8; 32], by: T::AccountId, for_network: NetworkId }, - AgainstSignal { signal_id: [u8; 32], who: T::AccountId, for_network: NetworkId }, + RetirementSignalRegistered { + signal_id: [u8; 32], + in_favor_of: [u8; 32], + registrant: T::AccountId, + }, + RetirementSignalRevoked { + signal_id: [u8; 32], + }, + SignalFavored { + signal_id: SignalId, + by: T::AccountId, + for_network: NetworkId, + }, + SetInFavor { + signal_id: SignalId, + set: ValidatorSet, + }, + RetirementSignalLockedIn { + signal_id: [u8; 32], + }, + SetNoLongerInFavor { + signal_id: SignalId, + set: ValidatorSet, + }, + FavorRevoked { + signal_id: SignalId, + by: T::AccountId, + for_network: NetworkId, + }, + AgainstSignal { + signal_id: SignalId, + who: T::AccountId, + for_network: NetworkId, + }, } #[pallet::error] pub enum Error { - SignalLockedIn, - SignalAlreadyRegistered, - NotSignalRegistrant, - NonExistantSignal, - ExpiredSignal, + RetirementSignalLockedIn, + RetirementSignalAlreadyRegistered, + NotRetirementSignalRegistrant, + NonExistantRetirementSignal, + ExpiredRetirementSignal, NotValidator, RevokingNonExistantFavor, } @@ -86,7 +149,7 @@ pub mod pallet { // Returns true if this network's current set is in favor of the signal. // // Must only be called for networks which have a set decided. - fn tally_for_network(signal_id: [u8; 32], network: NetworkId) -> Result> { + fn tally_for_network(signal_id: SignalId, network: NetworkId) -> Result> { let this_network_session = VsPallet::::latest_decided_session(network).unwrap(); let this_set = ValidatorSet { network, session: this_network_session }; @@ -145,7 +208,7 @@ pub mod pallet { } } - fn tally_for_all_networks(signal_id: [u8; 32]) -> Result> { + fn tally_for_all_networks(signal_id: SignalId) -> Result> { let mut total_in_favor_stake = 0; let mut total_allocated_stake = 0; for network in serai_primitives::NETWORKS { @@ -171,7 +234,7 @@ pub mod pallet { fn revoke_favor_internal( account: T::AccountId, - signal_id: [u8; 32], + signal_id: SignalId, for_network: NetworkId, ) -> DispatchResult { if !Favors::::contains_key((signal_id, for_network), account) { @@ -191,11 +254,19 @@ pub mod pallet { #[pallet::call] impl Pallet { + /// Register a retirement signal, declaring the consensus protocol this signal is in favor of. + /// + /// Retirement signals are registered so that the proposer, presumably a developer, can revoke + /// the signal if there's a fault discovered. #[pallet::call_index(0)] - #[pallet::weight(0)] - pub fn register_signal(origin: OriginFor, signal: [u8; 32]) -> DispatchResult { - if LockedInSignal::::exists() { - Err::<(), _>(Error::::SignalLockedIn)?; + #[pallet::weight(0)] // TODO + pub fn register_retirement_signal( + origin: OriginFor, + in_favor_of: [u8; 32], + ) -> DispatchResult { + // Don't allow retirement signals to be registered once a retirement has been locked in + if LockedInRetirement::::exists() { + Err::<(), _>(Error::::RetirementSignalLockedIn)?; } let account = ensure_signed(origin)?; @@ -203,68 +274,89 @@ pub mod pallet { // Bind the signal ID to the proposer // This prevents a malicious actor from frontrunning a proposal, causing them to be the // registrant, just to cancel it later - let mut signal_preimage = account.encode(); - signal_preimage.extend(signal); - let signal_id = blake2_256(&signal_preimage); + let signal = RegisteredRetirementSignal { + in_favor_of, + registrant: account, + registed_at: frame_system::Pallet::::block_number(), + }; + let signal_id = signal.id(); - if RegisteredSignals::::get(signal_id).is_some() { - Err::<(), _>(Error::::SignalAlreadyRegistered)?; + if RegisteredRetirementSignals::::get(signal_id).is_some() { + Err::<(), _>(Error::::RetirementSignalAlreadyRegistered)?; } - RegisteredSignals::::set( + + Self::deposit_event(Event::::RetirementSignalRegistered { signal_id, - Some(RegisteredSignal { - signal, - registrant: account, - registed_at: frame_system::Pallet::::block_number(), - }), - ); - Self::deposit_event(Event::::SignalRegistered { signal_id, signal, registrant: account }); + in_favor_of, + registrant: account, + }); + RegisteredRetirementSignals::::set(signal_id, Some(signal)); Ok(()) } #[pallet::call_index(1)] - #[pallet::weight(0)] - pub fn revoke_signal(origin: OriginFor, signal_id: [u8; 32]) -> DispatchResult { + #[pallet::weight(0)] // TODO + pub fn revoke_retirement_signal( + origin: OriginFor, + retirement_signal_id: [u8; 32], + ) -> DispatchResult { let account = ensure_signed(origin)?; - let Some(registered_signal) = RegisteredSignals::::get(signal_id) else { - return Err::<(), _>(Error::::NonExistantSignal.into()); + let Some(registered_signal) = RegisteredRetirementSignals::::get(retirement_signal_id) + else { + return Err::<(), _>(Error::::NonExistantRetirementSignal.into()); }; if account != registered_signal.registrant { - Err::<(), _>(Error::::NotSignalRegistrant)?; + Err::<(), _>(Error::::NotRetirementSignalRegistrant)?; } - RegisteredSignals::::remove(signal_id); + RegisteredRetirementSignals::::remove(retirement_signal_id); // If this signal was locked in, remove it // This lets a post-lock-in discovered fault be prevented from going live without - // intervention by all node runners - if LockedInSignal::::get().map(|(signal_id, _block_number)| signal_id) == Some(signal_id) { - LockedInSignal::::kill(); + // intervention by all validators + if LockedInRetirement::::get().map(|(signal_id, _block_number)| signal_id) == + Some(retirement_signal_id) + { + LockedInRetirement::::kill(); } - Self::deposit_event(Event::::SignalRevoked { signal_id }); + Self::deposit_event(Event::::RetirementSignalRevoked { signal_id: retirement_signal_id }); Ok(()) } #[pallet::call_index(2)] - #[pallet::weight(0)] + #[pallet::weight(0)] // TODO pub fn favor( origin: OriginFor, - signal_id: [u8; 32], + signal_id: SignalId, for_network: NetworkId, ) -> DispatchResult { - if LockedInSignal::::exists() { - Err::<(), _>(Error::::SignalLockedIn)?; - } - let account = ensure_signed(origin)?; - let Some(registered_signal) = RegisteredSignals::::get(signal_id) else { - return Err::<(), _>(Error::::NonExistantSignal.into()); - }; - // Check the signal isn't out of date - if (registered_signal.registed_at + T::ValidityDuration::get().into()) < - frame_system::Pallet::::block_number() - { - Err::<(), _>(Error::::ExpiredSignal)?; + + // If this is a retirement signal, perform the relevant checks + if let SignalId::Retirement(signal_id) = signal_id { + // Make sure a retirement hasn't already been locked in + if LockedInRetirement::::exists() { + Err::<(), _>(Error::::RetirementSignalLockedIn)?; + } + + // Make sure this is a registered retirement + // We don't have to do this for a `Halt` signal as `Halt` doesn't have the registration + // process + let Some(registered_signal) = RegisteredRetirementSignals::::get(signal_id) else { + return Err::<(), _>(Error::::NonExistantRetirementSignal.into()); + }; + + // Check the signal isn't out of date + // This isn't truly necessary since we only track votes from the most recent validator + // sets, ensuring modern relevancy + // The reason to still have it is because locking in a dated runtime may cause a corrupt + // blockchain and lead to a failure in system integrity + // `Halt`, which doesn't have this check, at worst causes temporary downtime + if (registered_signal.registed_at + T::RetirementValidityDuration::get().into()) < + frame_system::Pallet::::block_number() + { + Err::<(), _>(Error::::ExpiredRetirementSignal)?; + } } // Check the signer is a validator @@ -296,11 +388,19 @@ pub mod pallet { if network_in_favor { // If enough are, lock in the signal if Self::tally_for_all_networks(signal_id)? { - LockedInSignal::::set(Some(( - signal_id, - frame_system::Pallet::::block_number() + T::LockInDuration::get().into(), - ))); - Self::deposit_event(Event::SignalLockedIn { signal_id }); + match signal_id { + SignalId::Retirement(signal_id) => { + LockedInRetirement::::set(Some(( + signal_id, + frame_system::Pallet::::block_number() + + T::RetirementLockInDuration::get().into(), + ))); + Self::deposit_event(Event::RetirementSignalLockedIn { signal_id }); + } + SignalId::Halt(network) => { + InInstructions::::halt(network)?; + } + } } } @@ -309,14 +409,14 @@ pub mod pallet { /// Revoke favor into an abstaining position. #[pallet::call_index(3)] - #[pallet::weight(0)] + #[pallet::weight(0)] // TODO pub fn revoke_favor( origin: OriginFor, - signal_id: [u8; 32], + signal_id: SignalId, for_network: NetworkId, ) -> DispatchResult { - if LockedInSignal::::exists() { - Err::<(), _>(Error::::SignalLockedIn)?; + if matches!(&signal_id, SignalId::Retirement(_)) && LockedInRetirement::::exists() { + Err::<(), _>(Error::::RetirementSignalLockedIn)?; } // Doesn't check the signal exists due to later checking the favor exists @@ -332,14 +432,14 @@ pub mod pallet { /// /// If the origin is currently in favor of the signal, their favor will be revoked. #[pallet::call_index(4)] - #[pallet::weight(0)] + #[pallet::weight(0)] // TODO pub fn stand_against( origin: OriginFor, - signal_id: [u8; 32], + signal_id: SignalId, for_network: NetworkId, ) -> DispatchResult { - if LockedInSignal::::exists() { - Err::<(), _>(Error::::SignalLockedIn)?; + if LockedInRetirement::::exists() { + Err::<(), _>(Error::::RetirementSignalLockedIn)?; } let account = ensure_signed(origin)?; @@ -348,8 +448,10 @@ pub mod pallet { Self::revoke_favor_internal(account, signal_id, for_network)?; } else { // Check this Signal exists (which would've been implied by Favors for it existing) - if RegisteredSignals::::get(signal_id).is_none() { - Err::<(), _>(Error::::NonExistantSignal)?; + if let SignalId::Retirement(signal_id) = signal_id { + if RegisteredRetirementSignals::::get(signal_id).is_none() { + Err::<(), _>(Error::::NonExistantRetirementSignal)?; + } } } @@ -365,11 +467,11 @@ pub mod pallet { fn on_initialize(current_number: BlockNumberFor) -> Weight { // If this is the block at which a locked-in signal has been set for long enough, panic // This will prevent this block from executing and halt the chain - if let Some((signal, block_number)) = LockedInSignal::::get() { + if let Some((signal, block_number)) = LockedInRetirement::::get() { if block_number == current_number { panic!( "locked-in signal {} has been set for too long", - sp_core::hexdisplay::HexDisplay::from(&signal) + sp_core::hexdisplay::HexDisplay::from(&signal), ); } }