Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump substrate v0.9.28 and add a MeetupResult in the IssuedRewards storage. #274

Merged
merged 11 commits into from
Nov 9, 2022
Prev Previous commit
Next Next commit
Use of strongly typed MeetupResult
echevrier committed Oct 31, 2022
commit b0ecca04264be66fa7509f3f25919696d745b189
2 changes: 1 addition & 1 deletion ceremonies/src/benchmarking.rs
Original file line number Diff line number Diff line change
@@ -149,7 +149,7 @@ where
);

let cindex = encointer_scheduler::Pallet::<T>::current_ceremony_index();
IssuedRewards::<T>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<T>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let proof = create_proof_of_attendance::<T>(prover_account, cid, cindex - 1, prover);
proof
}
43 changes: 26 additions & 17 deletions ceremonies/src/lib.rs
Original file line number Diff line number Diff line change
@@ -49,10 +49,9 @@ use frame_support::{
};
use frame_system::ensure_signed;
use log::{debug, error, info, trace, warn};
use scale_info::{prelude::*, TypeInfo};
use scale_info::TypeInfo;
use sp_runtime::traits::{CheckedSub, IdentifyAccount, Member, Verify};
use sp_std::{cmp::max, prelude::*, vec};

// Logger target
const LOG: &str = "encointer";

@@ -64,7 +63,6 @@ mod storage_helper;
#[frame_support::pallet]
pub mod pallet {
use super::*;
use encointer_primitives::common::PalletString;
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

@@ -592,21 +590,16 @@ pub mod pallet {
Ok(participant_judgements) => participant_judgements,
// handle errors
Err(err) => {
// only mark issuance as complete in registering phase
// because in attesting phase there could be a failing early payout attempt
if current_phase == CeremonyPhaseType::Registering {
info!(target: LOG, "marking issuance as completed for failed meetup.");
let msg: PalletString = PalletString::from(format!("Error::{:?}", err));
<IssuedRewards<T>>::insert((cid, cindex), meetup_index, msg);
return Ok(Pays::No.into())
}
match err {
let (error, meetup_result) = match err {
MeetupValidationError::BallotEmpty => {
debug!(
target: LOG,
"ballot empty for meetup {:?}, cid: {:?}", meetup_index, cid
);
return Err(<Error<T>>::VotesNotDependable.into())
(
Err(<Error<T>>::VotesNotDependable.into()),
MeetupResult::VotesNotDependable,
)
},
MeetupValidationError::NoDependableVote => {
debug!(
@@ -615,15 +608,31 @@ pub mod pallet {
meetup_index,
cid
);
return Err(<Error<T>>::VotesNotDependable.into())
(
Err(<Error<T>>::VotesNotDependable.into()),
MeetupResult::VotesNotDependable,
)
},
MeetupValidationError::IndexOutOfBounds => {
debug!(
target: LOG,
"index out of bounds for meetup {:?}, cid: {:?}", meetup_index, cid
);
return Err(<Error<T>>::MeetupValidationIndexOutOfBounds.into())
(
Err(<Error<T>>::MeetupValidationIndexOutOfBounds.into()),
MeetupResult::MeetupValidationIndexOutOfBounds,
)
},
};
// only mark issuance as complete in registering phase
// because in attesting phase there could be a failing early payout attempt
if current_phase == CeremonyPhaseType::Registering {
info!(target: LOG, "marking issuance as completed for failed meetup.");

<IssuedRewards<T>>::insert((cid, cindex), meetup_index, meetup_result);
return Ok(Pays::No.into())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also return the error here. Otherwise, the user expects some rewards.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we return an error, the storage (IssuedRewards) will not be updated. We did these changes to fix it: #275
The "error" feedback is in the MeetupResult:
MeetupResult::VotesNotDependable or MeetupResult::MeetupValidationIndexOutOfBounds. Perhaps we can improve the name: MeetupResult::VotesNotDependableError, MeetupResult::MeetupValidationIndexOutOfBoundsError ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, I completely forgot that this was the cause of this PR. In that case, we should additionally add an event:

MeetupEvaluated(MeetupResult)

} else {
return error
}
},
};
@@ -1147,7 +1156,7 @@ pub mod pallet {
CommunityCeremony,
Blake2_128Concat,
MeetupIndexType,
PalletString,
MeetupResult,
ValueQuery,
>;

@@ -1895,7 +1904,7 @@ impl<T: Config> Pallet<T> {
sp_io::offchain_index::set(&reputation_cache_dirty_key(participant), &true.encode());
}

<IssuedRewards<T>>::insert((cid, cindex), meetup_idx, successful_claim_rewards());
<IssuedRewards<T>>::insert((cid, cindex), meetup_idx, MeetupResult::Ok);
info!(target: LOG, "issuing rewards completed");

Self::deposit_event(Event::RewardsIssued(
48 changes: 24 additions & 24 deletions ceremonies/src/tests.rs
Original file line number Diff line number Diff line change
@@ -348,7 +348,7 @@ fn registering_participant_works() {
let newbies = add_population(2, 2);
let newbie_1 = account_id(&newbies[0]);
let newbie_2 = account_id(&newbies[01]);
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
assert_ok!(register(newbie_1.clone(), cid, None));
assert_eq!(EncointerCeremonies::newbie_count((cid, cindex)), 1);

@@ -753,8 +753,8 @@ fn claim_rewards_works() {
run_to_next_phase();
// Registering
EncointerCeremonies::claim_rewards(Origin::signed(account_id(&alice)), cid, None).ok();
let msg = IssuedRewards::<TestRuntime>::get((cid, cindex), 1);
assert_eq!(msg, successful_claim_rewards());
let meetup_result = IssuedRewards::<TestRuntime>::get((cid, cindex), 1);
assert_eq!(meetup_result, MeetupResult::Ok);

assert!(event_deposited::<TestRuntime>(Event::RewardsIssued(cid, 1, 2).into()));

@@ -1040,8 +1040,8 @@ fn claim_rewards_error_results_in_meetup_marked_as_completed() {
.is_ok());
// in registering, the meetup is marked as completed
assert!(IssuedRewards::<TestRuntime>::contains_key((cid, cindex), 1));
let msg = IssuedRewards::<TestRuntime>::get((cid, cindex), 1);
assert_eq!(msg, format!("Error::{:?}", MeetupValidationError::NoDependableVote));
let meetup_result = IssuedRewards::<TestRuntime>::get((cid, cindex), 1);
assert_eq!(meetup_result, MeetupResult::VotesNotDependable);
});
}

@@ -1277,7 +1277,7 @@ fn register_with_reputation_works() {

// see if Zoran can register with his fresh key
// for the next ceremony claiming his former attendance
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let proof = prove_attendance(account_id(&zoran_new), cid, cindex - 1, &zoran);
assert_ok!(register(account_id(&zoran_new), cid, Some(proof)));
assert_eq!(
@@ -1678,7 +1678,7 @@ fn ceremony_index_and_purging_registry_works() {
IssuedRewards::<TestRuntime>::insert(
(cid, EncointerScheduler::current_ceremony_index()),
0,
successful_claim_rewards(),
MeetupResult::Ok,
);

run_to_next_phase();
@@ -1742,7 +1742,7 @@ fn grow_population_and_removing_community_works() {
IssuedRewards::<TestRuntime>::insert(
(cid, EncointerScheduler::current_ceremony_index() - 1),
0,
successful_claim_rewards(),
MeetupResult::Ok,
);
participants.iter().for_each(|p| {
assert!(
@@ -1950,8 +1950,8 @@ fn update_inactivity_counters_works() {

let mut cindex = 5;

IssuedRewards::<TestRuntime>::insert((cid0, cindex), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid1, cindex), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid0, cindex), 0, MeetupResult::Ok);
IssuedRewards::<TestRuntime>::insert((cid1, cindex), 0, MeetupResult::Ok);

let timeout = 1;
assert_eq!(
@@ -1960,7 +1960,7 @@ fn update_inactivity_counters_works() {
);

cindex += 1;
IssuedRewards::<TestRuntime>::insert((cid0, cindex), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid0, cindex), 0, MeetupResult::Ok);
assert_eq!(
EncointerCeremonies::update_inactivity_counters(cindex, timeout, vec![cid0, cid1]),
vec![]
@@ -2010,7 +2010,7 @@ fn purge_inactive_communities_works() {
IssuedRewards::<TestRuntime>::insert(
(cid, EncointerScheduler::current_ceremony_index()),
0,
successful_claim_rewards(),
MeetupResult::Ok,
);
run_to_next_phase();
run_to_next_phase();
@@ -2365,7 +2365,7 @@ fn remove_participant_from_registry_works() {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();

IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);

@@ -2426,7 +2426,7 @@ fn remove_participant_from_registry_works_for_all_participant_types() {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();

IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);

@@ -2478,7 +2478,7 @@ fn remove_participant_from_registry_with_no_participants_fails() {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();

IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);

@@ -2494,7 +2494,7 @@ fn upgrade_registration_works() {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();

IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2524,7 +2524,7 @@ fn upgrade_registration_fails_if_not_registered_or_not_newbie() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2559,7 +2559,7 @@ fn upgrade_registration_fails_in_wrong_phase() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2586,7 +2586,7 @@ fn upgrade_registration_fails_with_inexistent_community() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2612,7 +2612,7 @@ fn unregister_participant_works_with_reputables() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2653,7 +2653,7 @@ fn unregister_participant_fails_with_reputables_and_wrong_reputation() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2701,7 +2701,7 @@ fn unregister_participant_works_with_newbies() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2725,7 +2725,7 @@ fn unregister_participant_fails_in_wrong_phase() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
@@ -2750,7 +2750,7 @@ fn unregister_participant_fails_with_inexistent_community() {
new_test_ext().execute_with(|| {
let cid = register_test_community::<TestRuntime>(None, 0.0, 0.0);
let cindex = EncointerScheduler::current_ceremony_index();
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, successful_claim_rewards());
IssuedRewards::<TestRuntime>::insert((cid, cindex - 1), 0, MeetupResult::Ok);
let bootstrapper = account_id(&AccountKeyring::Ferdie.pair());
EncointerCommunities::insert_bootstrappers(cid, vec![bootstrapper.clone()]);
assert!(EncointerBalances::issue(cid, &bootstrapper, NominalIncome::from_num(1)).is_ok());
26 changes: 17 additions & 9 deletions primitives/src/ceremonies.rs
Original file line number Diff line number Diff line change
@@ -17,10 +17,7 @@
#[cfg(feature = "serde_derive")]
use serde::{Deserialize, Serialize};

use crate::{
common::PalletString,
communities::{CommunityIdentifier, Location},
};
use crate::communities::{CommunityIdentifier, Location};

use codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
@@ -294,7 +291,6 @@ pub mod consts {
/// Dirty bit key for reputation offchain storage
pub const REPUTATION_CACHE_DIRTY_KEY: &[u8] = b"reputation_cache_dirty";
pub const STORAGE_REPUTATION_KEY: &[u8; 10] = b"reputation";
pub const CLAIM_REWARD_OK: &str = "Ok";
}

pub fn reputation_cache_key<Account: Encode>(account: &Account) -> Vec<u8> {
@@ -305,10 +301,6 @@ pub fn reputation_cache_dirty_key<Account: Encode>(account: &Account) -> Vec<u8>
(consts::REPUTATION_CACHE_DIRTY_KEY, account).encode()
}

pub fn successful_claim_rewards() -> PalletString {
PalletString::from(consts::CLAIM_REWARD_OK)
}

#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)]
#[cfg_attr(feature = "serde_derive", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "serde_derive", serde(rename_all = "camelCase"))]
@@ -356,6 +348,22 @@ pub struct ReputationCacheValue {
pub reputation: Vec<(CeremonyIndexType, CommunityReputation)>,
}

#[derive(Encode, Decode, Copy, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
#[cfg_attr(feature = "serde_derive", derive(Serialize, Deserialize))]
pub enum MeetupResult {
Ok,
VotesNotDependable,
MeetupValidationIndexOutOfBounds,
NoResult,
Other,
echevrier marked this conversation as resolved.
Show resolved Hide resolved
}

impl Default for MeetupResult {
fn default() -> Self {
MeetupResult::NoResult
}
}
echevrier marked this conversation as resolved.
Show resolved Hide resolved

#[cfg(test)]
mod tests {
use super::*;