Skip to content

Commit

Permalink
chore: methods for last paid removed to eliminate ambiguity
Browse files Browse the repository at this point in the history
  • Loading branch information
yahortsaryk committed Nov 15, 2024
1 parent 2bdeb4c commit 342b5f2
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 57 deletions.
2 changes: 2 additions & 0 deletions pallets/ddc-clusters/src/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub struct Cluster<AccountId> {
pub reserve_id: AccountId,
pub props: ClusterProps<AccountId>,
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,
}

Expand Down
14 changes: 4 additions & 10 deletions pallets/ddc-clusters/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -890,24 +890,18 @@ pub mod pallet {
}
}
impl<T: Config> ClusterValidator<T> for Pallet<T> {
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::<T>::try_get(cluster_id).map_err(|_| Error::<T>::ClusterDoesNotExist)?;

cluster.last_validated_era_id = era_id;
Clusters::<T>::insert(cluster_id, cluster);
Self::deposit_event(Event::<T>::ClusterEraValidated {
cluster_id: *cluster_id,
era_id,
});
Self::deposit_event(Event::<T>::ClusterEraPaid { cluster_id: *cluster_id, era_id });

Ok(())
}

fn get_last_validated_era(cluster_id: &ClusterId) -> Result<DdcEra, DispatchError> {
fn get_last_paid_era(cluster_id: &ClusterId) -> Result<DdcEra, DispatchError> {
let cluster =
Clusters::<T>::try_get(cluster_id).map_err(|_| Error::<T>::ClusterDoesNotExist)?;

Expand Down
6 changes: 3 additions & 3 deletions pallets/ddc-clusters/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Test>::ClusterDoesNotExist
);

Expand Down Expand Up @@ -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())
})
}

Expand Down
4 changes: 2 additions & 2 deletions pallets/ddc-customers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pallets/ddc-payouts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ pub mod pallet {
pub enum Error<T> {
BillingReportDoesNotExist,
NotExpectedState,
Unauthorised,
Unauthorized,
BatchIndexAlreadyProcessed,
BatchIndexIsOutOfRange,
BatchesMissed,
Expand Down
33 changes: 15 additions & 18 deletions pallets/ddc-verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ pub mod pallet {
/// Bad requests.
BadRequest,
/// Not a validator.
Unauthorised,
Unauthorized,
/// Already signed era.
AlreadySignedEra,
NotExpectedState,
Expand Down Expand Up @@ -1880,7 +1880,6 @@ pub mod pallet {
cluster_id: &ClusterId,
) -> Result<Option<(DdcEra, i64, i64)>, 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(
Expand Down Expand Up @@ -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<Option<DdcEra>, OCWError> {
Expand Down Expand Up @@ -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 }
})?;

Expand Down Expand Up @@ -3425,7 +3424,7 @@ pub mod pallet {
) -> DispatchResult {
let caller = ensure_signed(origin)?;

ensure!(Self::is_ocw_validator(caller.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(caller.clone()), Error::<T>::Unauthorized);
let mut era_validation = {
let era_validations = <EraValidations<T>>::get(cluster_id, era_activity.id);

Expand Down Expand Up @@ -3547,7 +3546,7 @@ pub mod pallet {
end_era: i64,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);

T::PayoutProcessor::begin_billing_report(cluster_id, era_id, start_era, end_era)?;

Expand All @@ -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::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::begin_charging_customers(cluster_id, era_id, max_batch_index)
}

Expand All @@ -3588,7 +3587,7 @@ pub mod pallet {
batch_proof: MMRProof,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::send_charging_customers_batch(
cluster_id,
era_id,
Expand All @@ -3606,7 +3605,7 @@ pub mod pallet {
era_id: DdcEra,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::end_charging_customers(cluster_id, era_id)
}

Expand All @@ -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::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::begin_rewarding_providers(
cluster_id,
era_id,
Expand All @@ -3640,7 +3639,7 @@ pub mod pallet {
batch_proof: MMRProof,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::send_rewarding_providers_batch(
cluster_id,
era_id,
Expand All @@ -3658,7 +3657,7 @@ pub mod pallet {
era_id: DdcEra,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::end_rewarding_providers(cluster_id, era_id)
}

Expand All @@ -3670,16 +3669,14 @@ pub mod pallet {
era_id: DdcEra,
) -> DispatchResult {
let sender = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(sender.clone()), Error::<T>::Unauthorized);
T::PayoutProcessor::end_billing_report(cluster_id, era_id)?;

let mut era_validation = <EraValidations<T>>::get(cluster_id, era_id).unwrap(); // should exist
era_validation.status = EraValidationStatus::PayoutSuccess;
<EraValidations<T>>::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.
Expand All @@ -3698,7 +3695,7 @@ pub mod pallet {
errors: Vec<OCWError>,
) -> DispatchResult {
let caller = ensure_signed(origin)?;
ensure!(Self::is_ocw_validator(caller.clone()), Error::<T>::Unauthorised);
ensure!(Self::is_ocw_validator(caller.clone()), Error::<T>::Unauthorized);

for error in errors {
match error {
Expand Down
7 changes: 2 additions & 5 deletions pallets/ddc-verification/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,14 +418,11 @@ pub fn new_test_ext() -> sp_io::TestExternalities {

pub struct TestClusterValidator;
impl<T: Config> ClusterValidator<T> 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<DdcEra, DispatchError> {
fn get_last_paid_era(_cluster_id: &ClusterId) -> Result<DdcEra, DispatchError> {
Ok(Default::default())
}
}
Expand Down
35 changes: 20 additions & 15 deletions pallets/ddc-verification/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2355,10 +2355,11 @@ fn test_get_last_validated_era() {
let validators = get_validators();

new_test_ext().execute_with(|| {
assert_ok!(Pallet::<Test>::get_last_validated_era(&cluster_id1, validators[0].clone())
.map(|era| {
assert_ok!(Pallet::<Test>::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(
Expand All @@ -2378,16 +2379,18 @@ fn test_get_last_validated_era() {
<EraValidations<Test>>::insert(cluster_id1, era_1, validation_1);

// still no - different accountid
assert_ok!(Pallet::<Test>::get_last_validated_era(&cluster_id1, validators[0].clone())
.map(|era| {
assert_ok!(Pallet::<Test>::get_last_paid_era(&cluster_id1, validators[0].clone()).map(
|era| {
assert_eq!(era, None);
}));
}
));

// still no - different cluster id
assert_ok!(Pallet::<Test>::get_last_validated_era(&cluster_id2, validators[1].clone())
.map(|era| {
assert_ok!(Pallet::<Test>::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
Expand All @@ -2405,15 +2408,17 @@ fn test_get_last_validated_era() {
<EraValidations<Test>>::insert(cluster_id1, era_2, validation_2);

// Now the last validated era should be ERA_2
assert_ok!(Pallet::<Test>::get_last_validated_era(&cluster_id1, validators[2].clone())
.map(|era| {
assert_ok!(Pallet::<Test>::get_last_paid_era(&cluster_id1, validators[2].clone()).map(
|era| {
assert_eq!(era, Some(era_2));
}));
}
));

assert_ok!(Pallet::<Test>::get_last_validated_era(&cluster_id1, validators[1].clone())
.map(|era| {
assert_ok!(Pallet::<Test>::get_last_paid_era(&cluster_id1, validators[1].clone()).map(
|era| {
assert_eq!(era, Some(era_1));
}));
}
));
});
}

Expand Down
6 changes: 3 additions & 3 deletions primitives/src/traits/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ pub trait ClusterValidator<T: Config> {
///
/// # 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.
Expand All @@ -127,5 +127,5 @@ pub trait ClusterValidator<T: Config> {
/// # Returns
///
/// Returns `Ok(DdcEra)` identifier of the last validated era in cluster
fn get_last_validated_era(cluster_id: &ClusterId) -> Result<DdcEra, DispatchError>;
fn get_last_paid_era(cluster_id: &ClusterId) -> Result<DdcEra, DispatchError>;
}

0 comments on commit 342b5f2

Please sign in to comment.