From 342b5f2711567d5d4b1415f33996213e5e961fdc Mon Sep 17 00:00:00 2001 From: yahortsaryk Date: Fri, 15 Nov 2024 13:43:37 +0100 Subject: [PATCH] chore: methods for last paid removed to eliminate ambiguity --- pallets/ddc-clusters/src/cluster.rs | 2 ++ pallets/ddc-clusters/src/lib.rs | 14 +++-------- pallets/ddc-clusters/src/tests.rs | 6 ++--- pallets/ddc-customers/src/lib.rs | 4 +-- pallets/ddc-payouts/src/lib.rs | 2 +- pallets/ddc-verification/src/lib.rs | 33 ++++++++++++------------- pallets/ddc-verification/src/mock.rs | 7 ++---- pallets/ddc-verification/src/tests.rs | 35 +++++++++++++++------------ primitives/src/traits/cluster.rs | 6 ++--- 9 files changed, 52 insertions(+), 57 deletions(-) diff --git a/pallets/ddc-clusters/src/cluster.rs b/pallets/ddc-clusters/src/cluster.rs index bc3fe15d5..9ce44edd8 100644 --- a/pallets/ddc-clusters/src/cluster.rs +++ b/pallets/ddc-clusters/src/cluster.rs @@ -15,6 +15,8 @@ pub struct Cluster { pub reserve_id: AccountId, pub props: ClusterProps, pub status: ClusterStatus, + // todo(yahortsaryk): this should be renamed to `last_paid_era` to eliminate ambiguity, + // as the validation step is decoupled from payout step. pub last_validated_era_id: DdcEra, } diff --git a/pallets/ddc-clusters/src/lib.rs b/pallets/ddc-clusters/src/lib.rs index c96f4e2d5..75a8bbd99 100644 --- a/pallets/ddc-clusters/src/lib.rs +++ b/pallets/ddc-clusters/src/lib.rs @@ -106,7 +106,7 @@ pub mod pallet { ClusterUnbonding { cluster_id: ClusterId }, ClusterUnbonded { cluster_id: ClusterId }, ClusterNodeValidated { cluster_id: ClusterId, node_pub_key: NodePubKey, succeeded: bool }, - ClusterEraValidated { cluster_id: ClusterId, era_id: DdcEra }, + ClusterEraPaid { cluster_id: ClusterId, era_id: DdcEra }, } #[pallet::error] @@ -890,24 +890,18 @@ pub mod pallet { } } impl ClusterValidator for Pallet { - fn set_last_validated_era( - cluster_id: &ClusterId, - era_id: DdcEra, - ) -> Result<(), DispatchError> { + fn set_last_paid_era(cluster_id: &ClusterId, era_id: DdcEra) -> Result<(), DispatchError> { let mut cluster = Clusters::::try_get(cluster_id).map_err(|_| Error::::ClusterDoesNotExist)?; cluster.last_validated_era_id = era_id; Clusters::::insert(cluster_id, cluster); - Self::deposit_event(Event::::ClusterEraValidated { - cluster_id: *cluster_id, - era_id, - }); + Self::deposit_event(Event::::ClusterEraPaid { cluster_id: *cluster_id, era_id }); Ok(()) } - fn get_last_validated_era(cluster_id: &ClusterId) -> Result { + fn get_last_paid_era(cluster_id: &ClusterId) -> Result { let cluster = Clusters::::try_get(cluster_id).map_err(|_| Error::::ClusterDoesNotExist)?; diff --git a/pallets/ddc-clusters/src/tests.rs b/pallets/ddc-clusters/src/tests.rs index 56c9d8237..fc9b41e8d 100644 --- a/pallets/ddc-clusters/src/tests.rs +++ b/pallets/ddc-clusters/src/tests.rs @@ -590,7 +590,7 @@ fn set_last_validated_era_works() { // Cluster doesn't exist assert_noop!( - DdcClusters::set_last_validated_era(&cluster_id, era_id), + DdcClusters::set_last_paid_era(&cluster_id, era_id), Error::::ClusterDoesNotExist ); @@ -619,14 +619,14 @@ fn set_last_validated_era_works() { } )); - assert_ok!(DdcClusters::set_last_validated_era(&cluster_id, era_id)); + assert_ok!(DdcClusters::set_last_paid_era(&cluster_id, era_id)); let updated_cluster = DdcClusters::clusters(cluster_id).unwrap(); assert_eq!(updated_cluster.last_validated_era_id, era_id); // Checking that event was emitted assert_eq!(System::events().len(), 3); - System::assert_last_event(Event::ClusterEraValidated { cluster_id, era_id }.into()) + System::assert_last_event(Event::ClusterEraPaid { cluster_id, era_id }.into()) }) } diff --git a/pallets/ddc-customers/src/lib.rs b/pallets/ddc-customers/src/lib.rs index ff7475645..a5a1914e1 100644 --- a/pallets/ddc-customers/src/lib.rs +++ b/pallets/ddc-customers/src/lib.rs @@ -236,8 +236,8 @@ pub mod pallet { BucketDoesNotExist, /// DDC Cluster with provided id doesn't exist ClusterDoesNotExist, - // unauthorised operation - Unauthorised, + // unauthorized operation + Unauthorized, // Arithmetic overflow ArithmeticOverflow, // Arithmetic underflow diff --git a/pallets/ddc-payouts/src/lib.rs b/pallets/ddc-payouts/src/lib.rs index 4adb919a5..2bb5edc78 100644 --- a/pallets/ddc-payouts/src/lib.rs +++ b/pallets/ddc-payouts/src/lib.rs @@ -215,7 +215,7 @@ pub mod pallet { pub enum Error { BillingReportDoesNotExist, NotExpectedState, - Unauthorised, + Unauthorized, BatchIndexAlreadyProcessed, BatchIndexIsOutOfRange, BatchesMissed, diff --git a/pallets/ddc-verification/src/lib.rs b/pallets/ddc-verification/src/lib.rs index ae57f1e2b..b7ffca894 100644 --- a/pallets/ddc-verification/src/lib.rs +++ b/pallets/ddc-verification/src/lib.rs @@ -474,7 +474,7 @@ pub mod pallet { /// Bad requests. BadRequest, /// Not a validator. - Unauthorised, + Unauthorized, /// Already signed era. AlreadySignedEra, NotExpectedState, @@ -1880,7 +1880,6 @@ pub mod pallet { cluster_id: &ClusterId, ) -> Result, OCWError> { Ok(Self::get_era_for_payout(cluster_id, EraValidationStatus::ReadyForPayout)) - // todo! get start and end values based on result } pub(crate) fn prepare_begin_charging_customers( @@ -2737,7 +2736,7 @@ pub mod pallet { /// - `Ok(None)`: The validator did not participate in any era for the given cluster. /// - `Err(OCWError)`: An error occurred while retrieving the data. // todo! add tests for start and end era - pub(crate) fn get_last_validated_era( + pub(crate) fn get_last_paid_era( cluster_id: &ClusterId, validator: T::AccountId, ) -> Result, OCWError> { @@ -2783,11 +2782,11 @@ pub mod pallet { let this_validator = Self::fetch_verification_account_id()?; let last_validated_era_by_this_validator = - Self::get_last_validated_era(cluster_id, this_validator)? + Self::get_last_paid_era(cluster_id, this_validator)? .unwrap_or_else(DdcEra::default); let last_paid_era_for_cluster = - T::ClusterValidator::get_last_validated_era(cluster_id).map_err(|_| { + T::ClusterValidator::get_last_paid_era(cluster_id).map_err(|_| { OCWError::EraRetrievalError { cluster_id: *cluster_id, node_pub_key: None } })?; @@ -3425,7 +3424,7 @@ pub mod pallet { ) -> DispatchResult { let caller = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(caller.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(caller.clone()), Error::::Unauthorized); let mut era_validation = { let era_validations = >::get(cluster_id, era_activity.id); @@ -3547,7 +3546,7 @@ pub mod pallet { end_era: i64, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::begin_billing_report(cluster_id, era_id, start_era, end_era)?; @@ -3573,7 +3572,7 @@ pub mod pallet { max_batch_index: BatchIndex, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::begin_charging_customers(cluster_id, era_id, max_batch_index) } @@ -3588,7 +3587,7 @@ pub mod pallet { batch_proof: MMRProof, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::send_charging_customers_batch( cluster_id, era_id, @@ -3606,7 +3605,7 @@ pub mod pallet { era_id: DdcEra, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::end_charging_customers(cluster_id, era_id) } @@ -3620,7 +3619,7 @@ pub mod pallet { total_node_usage: NodeUsage, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::begin_rewarding_providers( cluster_id, era_id, @@ -3640,7 +3639,7 @@ pub mod pallet { batch_proof: MMRProof, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::send_rewarding_providers_batch( cluster_id, era_id, @@ -3658,7 +3657,7 @@ pub mod pallet { era_id: DdcEra, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::end_rewarding_providers(cluster_id, era_id) } @@ -3670,16 +3669,14 @@ pub mod pallet { era_id: DdcEra, ) -> DispatchResult { let sender = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(sender.clone()), Error::::Unauthorized); T::PayoutProcessor::end_billing_report(cluster_id, era_id)?; let mut era_validation = >::get(cluster_id, era_id).unwrap(); // should exist era_validation.status = EraValidationStatus::PayoutSuccess; >::insert(cluster_id, era_id, era_validation); - // todo(yahortsaryk): this should be renamed to `last_paid_era` to eliminate ambiguity, - // as the validation step is decoupled from payout step. - T::ClusterValidator::set_last_validated_era(&cluster_id, era_id) + T::ClusterValidator::set_last_paid_era(&cluster_id, era_id) } /// Emit consensus errors. @@ -3698,7 +3695,7 @@ pub mod pallet { errors: Vec, ) -> DispatchResult { let caller = ensure_signed(origin)?; - ensure!(Self::is_ocw_validator(caller.clone()), Error::::Unauthorised); + ensure!(Self::is_ocw_validator(caller.clone()), Error::::Unauthorized); for error in errors { match error { diff --git a/pallets/ddc-verification/src/mock.rs b/pallets/ddc-verification/src/mock.rs index c768e3668..1df98ad92 100644 --- a/pallets/ddc-verification/src/mock.rs +++ b/pallets/ddc-verification/src/mock.rs @@ -418,14 +418,11 @@ pub fn new_test_ext() -> sp_io::TestExternalities { pub struct TestClusterValidator; impl ClusterValidator for TestClusterValidator { - fn set_last_validated_era( - _cluster_id: &ClusterId, - _era_id: DdcEra, - ) -> Result<(), DispatchError> { + fn set_last_paid_era(_cluster_id: &ClusterId, _era_id: DdcEra) -> Result<(), DispatchError> { unimplemented!() } - fn get_last_validated_era(_cluster_id: &ClusterId) -> Result { + fn get_last_paid_era(_cluster_id: &ClusterId) -> Result { Ok(Default::default()) } } diff --git a/pallets/ddc-verification/src/tests.rs b/pallets/ddc-verification/src/tests.rs index 9442324cc..a308d76f9 100644 --- a/pallets/ddc-verification/src/tests.rs +++ b/pallets/ddc-verification/src/tests.rs @@ -2355,10 +2355,11 @@ fn test_get_last_validated_era() { let validators = get_validators(); new_test_ext().execute_with(|| { - assert_ok!(Pallet::::get_last_validated_era(&cluster_id1, validators[0].clone()) - .map(|era| { + assert_ok!(Pallet::::get_last_paid_era(&cluster_id1, validators[0].clone()).map( + |era| { assert_eq!(era, None); - })); + } + )); let mut validators_map_1 = BTreeMap::new(); validators_map_1.insert( @@ -2378,16 +2379,18 @@ fn test_get_last_validated_era() { >::insert(cluster_id1, era_1, validation_1); // still no - different accountid - assert_ok!(Pallet::::get_last_validated_era(&cluster_id1, validators[0].clone()) - .map(|era| { + assert_ok!(Pallet::::get_last_paid_era(&cluster_id1, validators[0].clone()).map( + |era| { assert_eq!(era, None); - })); + } + )); // still no - different cluster id - assert_ok!(Pallet::::get_last_validated_era(&cluster_id2, validators[1].clone()) - .map(|era| { + assert_ok!(Pallet::::get_last_paid_era(&cluster_id2, validators[1].clone()).map( + |era| { assert_eq!(era, None); - })); + } + )); let mut validators_map_2 = BTreeMap::new(); validators_map_2 @@ -2405,15 +2408,17 @@ fn test_get_last_validated_era() { >::insert(cluster_id1, era_2, validation_2); // Now the last validated era should be ERA_2 - assert_ok!(Pallet::::get_last_validated_era(&cluster_id1, validators[2].clone()) - .map(|era| { + assert_ok!(Pallet::::get_last_paid_era(&cluster_id1, validators[2].clone()).map( + |era| { assert_eq!(era, Some(era_2)); - })); + } + )); - assert_ok!(Pallet::::get_last_validated_era(&cluster_id1, validators[1].clone()) - .map(|era| { + assert_ok!(Pallet::::get_last_paid_era(&cluster_id1, validators[1].clone()).map( + |era| { assert_eq!(era, Some(era_1)); - })); + } + )); }); } diff --git a/primitives/src/traits/cluster.rs b/primitives/src/traits/cluster.rs index efc95d807..ad7ec9a4a 100644 --- a/primitives/src/traits/cluster.rs +++ b/primitives/src/traits/cluster.rs @@ -113,8 +113,8 @@ pub trait ClusterValidator { /// /// # Events /// - /// Emits `ClusterEraValidated` event if the operation is successful. - fn set_last_validated_era(cluster_id: &ClusterId, era_id: DdcEra) -> Result<(), DispatchError>; + /// Emits `ClusterEraPaid` event if the operation is successful. + fn set_last_paid_era(cluster_id: &ClusterId, era_id: DdcEra) -> Result<(), DispatchError>; /// Retrieves the `last_validated_era_id` for the given cluster /// update. @@ -127,5 +127,5 @@ pub trait ClusterValidator { /// # Returns /// /// Returns `Ok(DdcEra)` identifier of the last validated era in cluster - fn get_last_validated_era(cluster_id: &ClusterId) -> Result; + fn get_last_paid_era(cluster_id: &ClusterId) -> Result; }