Properly handle the error in validator-sets

substrate/validator-sets/src/embedded_elliptic_curve_keys.rs

@@ -1,4 +1,4 @@
-use sp_core::{Encode, sr25519::Public};
+use sp_core::sr25519::Public;
 
 use serai_primitives::{crypto::SignedEmbeddedEllipticCurveKeys, network_id::*};
 

substrate/validator-sets/src/lib.rs

@@ -219,6 +219,11 @@ mod pallet {
     type DelayedDeallocations = DelayedDeallocations<T>;
   }
 
+  #[pallet::event]
+  #[pallet::generate_deposit(pub(super) fn deposit_event)]
+  pub enum Event<T: Config> {}
+
+  /*
   /// The generated key pair for a given validator set instance.
   #[pallet::storage]
   #[pallet::getter(fn keys)]
@@ -285,42 +290,16 @@ mod pallet {
       */
     }
   }
+  */
 
   #[pallet::error]
   pub enum Error<T> {
-    /// Validator Set doesn't exist.
+    /// The provided embedded elliptic curve keys were invalid.
-    NonExistentValidatorSet,
+    InvalidEmbeddedEllipticCurveKeys,
-    /// An invalid embedded elliptic curve key was specified.
+    /// Allocation was erroneous.
-    ///
+    AllocationError(AllocationError),
-    /// This error not being raised does not mean the key was valid. Solely that it wasn't detected
+    /// Deallocation was erroneous.
-    /// by this pallet as invalid.
+    DeallocationError(DeallocationError),
-    InvalidEmbeddedEllipticCurveKey,
-    /// Trying to perform an operation requiring an embedded elliptic curve key, without an
-    /// embedded elliptic curve key.
-    MissingEmbeddedEllipticCurveKey,
-    /// Not enough allocation to obtain a key share in the set.
-    InsufficientAllocation,
-    /// Trying to deallocate more than allocated.
-    NotEnoughAllocated,
-    /// Allocation would cause the validator set to no longer achieve fault tolerance.
-    AllocationWouldRemoveFaultTolerance,
-    /// Allocation would cause the validator set to never be able to achieve fault tolerance.
-    AllocationWouldPreventFaultTolerance,
-    /// Deallocation would remove the participant from the set, despite the validator not
-    /// specifying so.
-    DeallocationWouldRemoveParticipant,
-    /// Deallocation would cause the validator set to no longer achieve fault tolerance.
-    DeallocationWouldRemoveFaultTolerance,
-    /// Deallocation to be claimed doesn't exist.
-    NonExistentDeallocation,
-    /// Validator Set already generated keys.
-    AlreadyGeneratedKeys,
-    /// An invalid MuSig signature was provided.
-    BadSignature,
-    /// Validator wasn't registered or active.
-    NonExistentValidator,
-    /// Deallocation would take the stake below what is required.
-    DeallocationWouldRemoveEconomicSecurity,
   }
 
   /* TODO
@@ -816,7 +795,8 @@ mod pallet {
       let signer = ensure_signed(origin)?;
       <Abstractions<T> as crate::EmbeddedEllipticCurveKeys>::set_embedded_elliptic_curve_keys(
         signer, keys,
-      )?;
+      )
+      .map_err(|()| Error::<T>::InvalidEmbeddedEllipticCurveKeys)?;
       Ok(())
     }
 
@@ -824,21 +804,13 @@ mod pallet {
     #[pallet::weight(0)] // TODO
     pub fn allocate(origin: OriginFor<T>, network: NetworkId, amount: Amount) -> DispatchResult {
       let validator = ensure_signed(origin)?;
-      // If this network utilizes embedded elliptic curve(s), require the validator to have set the
-      // appropriate key(s)
-      if <
-        Abstractions::<T>
-          as
-        crate::EmbeddedEllipticCurveKeys
-      >::still_needs_to_set_embedded_elliptic_curve_keys(network, validator) {
-        Err(Error::<T>::MissingEmbeddedEllipticCurveKey)?;
-      }
       Coins::<T>::transfer_internal(
         validator,
         Self::account(),
         Balance { coin: Coin::Serai, amount },
       )?;
-      Abstractions::<T>::increase_allocation(network, validator, amount, false)?;
+      Abstractions::<T>::increase_allocation(network, validator, amount, false)
+        .map_err(Error::<T>::AllocationError)?;
       Ok(())
     }
 
@@ -847,7 +819,8 @@ mod pallet {
     pub fn deallocate(origin: OriginFor<T>, network: NetworkId, amount: Amount) -> DispatchResult {
       let account = ensure_signed(origin)?;
 
-      let deallocation_timeline = Abstractions::<T>::decrease_allocation(network, account, amount)?;
+      let deallocation_timeline = Abstractions::<T>::decrease_allocation(network, account, amount)
+        .map_err(Error::<T>::DeallocationError)?;
       if matches!(deallocation_timeline, DeallocationTimeline::Immediate) {
         Coins::<T>::transfer_internal(
           Self::account(),

substrate/validator-sets/src/sessions.rs

@@ -9,7 +9,7 @@ use serai_primitives::{
 
 use frame_support::storage::{StorageValue, StorageMap, StorageDoubleMap, StoragePrefixedMap};
 
-use crate::allocations::*;
+use crate::{embedded_elliptic_curve_keys::EmbeddedEllipticCurveKeys, allocations::Allocations};
 
 /// The list of genesis validators.
 pub(crate) type GenesisValidators = BoundedVec<Public, ConstU32<{ MAX_KEY_SHARES_PER_SET_U32 }>>;
@@ -17,7 +17,7 @@ pub(crate) type GenesisValidators = BoundedVec<Public, ConstU32<{ MAX_KEY_SHARES
 /// The key for the SelectedValidators map.
 pub(crate) type SelectedValidatorsKey = (ValidatorSet, [u8; 16], Public);
 
-pub(crate) trait SessionsStorage: AllocationsStorage {
+pub(crate) trait SessionsStorage: EmbeddedEllipticCurveKeys + Allocations {
   /// The genesis validators
   ///
   /// The usage of is shared with the rest of the pallet. `Sessions` only reads it.
@@ -92,20 +92,44 @@ fn clear_selected_validators<Storage: StoragePrefixedMap<u64>>(set: ValidatorSet
   ));
 }
 
-pub(crate) enum AllocationError {
+/// An error when allocating.
+#[derive(
+  scale::Encode,
+  scale::Decode,
+  scale::DecodeWithMemTracking,
+  scale_info::TypeInfo,
+  frame_support::PalletError,
+)]
+pub enum AllocationError {
+  /// The validator set didn't define an allocation requirement for a key share.
   NoAllocationPerKeyShareSet,
+  /// Validator is missing embedded elliptic curve keys.
+  MissingEmbeddedEllipticCurveKeys,
+  /// The allocation is less than the key share.
   AllocationLessThanKeyShare,
+  /// This allocation would introduce a single point of failure.
   IntroducesSinglePointOfFailure,
 }
 
-#[must_use]
 pub(crate) enum DeallocationTimeline {
   Immediate,
   Delayed { unlocks_at: Session },
 }
-pub(crate) enum DeallocationError {
+
+/// An error when deallocating.
+#[derive(
+  scale::Encode,
+  scale::Decode,
+  scale::DecodeWithMemTracking,
+  scale_info::TypeInfo,
+  frame_support::PalletError,
+)]
+pub enum DeallocationError {
+  /// The validator set didn't define an allocation requirement for a key share.
   NoAllocationPerKeyShareSet,
+  /// Not enough was allocated to enable this amount to be deallocated.
   NotEnoughAllocated,
+  /// The remaining allocation was non-zero and would be less than a key share.
   RemainingAllocationLessThanKeyShare,
 }
 
@@ -272,6 +296,10 @@ impl<Storage: SessionsStorage> Sessions for Storage {
       Err(AllocationError::NoAllocationPerKeyShareSet)?
     };
 
+    if Self::still_needs_to_set_embedded_elliptic_curve_keys(network, validator) {
+      Err(AllocationError::MissingEmbeddedEllipticCurveKeys)?;
+    }
+
     let old_allocation = Self::get_allocation(network, validator).unwrap_or(Amount(0));
     // Safe so long as the SRI supply fits within a u64, per assumptions on how this is called
     let new_allocation = (old_allocation + amount).unwrap();