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.

Sign up for GitHub

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

Jump to bottom

Aggregated account data #196

Merged
merged 10 commits into from
Apr 6, 2022
Merged

Aggregated account data #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fc0629b
implement aggregated account data rpc
pifragile Apr 5, 2022
9e0c451
remove unused import
pifragile Apr 5, 2022
c6897b9
use plain Decode
pifragile Apr 6, 2022
cf49114
use early return in get_meetup_index
pifragile Apr 6, 2022
6ad9695
remove redundant code
pifragile Apr 6, 2022
49c49cc
use borrow instead of Vec
pifragile Apr 6, 2022
9bd0c73
use plain Decode
pifragile Apr 6, 2022
9386960
remove unwrap
pifragile Apr 6, 2022
a0cd122
Merge branch 'master' into aggregated-account-data
brenzi Apr 6, 2022
2afde33
Merge branch 'master' into aggregated-account-data
brenzi Apr 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion ceremonies/assignment/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,21 @@ pub fn meetup_index(
Some(assignment_fn(participant_index, params, meetup_count)? + 1)
}

pub fn get_meetup_location_index(
meetup_index: MeetupIndexType,
locations: &[Location],
location_assignment_params: AssignmentParams,
) -> Option<MeetupIndexType> {
assignment_fn(meetup_index, location_assignment_params, locations.len() as u64)
}

pub fn meetup_location(
meetup_index: MeetupIndexType,
locations: Vec<Location>,
location_assignment_params: AssignmentParams,
) -> Option<Location> {
let location_idx =
assignment_fn(meetup_index, location_assignment_params, locations.len() as u64)?;
get_meetup_location_index(meetup_index, &locations, location_assignment_params)?;
pifragile marked this conversation as resolved.
Show resolved Hide resolved

if location_idx < locations.len() as u64 {
Some(locations[(location_idx) as usize])
Expand Down
12 changes: 9 additions & 3 deletions ceremonies/rpc/runtime-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@
#[cfg(not(feature = "std"))]
use sp_std::vec::Vec;

use encointer_primitives::ceremonies::{CeremonyIndexType, CommunityReputation};
use encointer_primitives::{
ceremonies::{AggregatedAccountData, CeremonyIndexType, CommunityReputation},
communities::CommunityIdentifier,
};
use sp_api::{Decode, Encode};

sp_api::decl_runtime_apis! {
pub trait CeremoniesApi<AccountId>
where AccountId: Encode + Decode
pub trait CeremoniesApi<AccountId, Moment>
where AccountId: Encode + Decode,
encointer_primitives::ceremonies::AggregatedAccountData<AccountId, Moment>: Decode

{
fn get_reputations(account: &AccountId) -> Vec<(CeremonyIndexType, CommunityReputation)>;
fn get_aggregated_account_data(cid:CommunityIdentifier, account: &AccountId) -> AggregatedAccountData<AccountId, Moment>;
}
}
45 changes: 36 additions & 9 deletions ceremonies/rpc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,39 +27,51 @@ use encointer_ceremonies_rpc_runtime_api::CeremoniesApi as CeremoniesRuntimeApi;
use encointer_primitives::{
ceremonies::{
consts::STORAGE_PREFIX, reputation_cache_dirty_key, reputation_cache_key,
CommunityReputation,
AggregatedAccountData, CommunityReputation,
},
communities::CommunityIdentifier,
scheduler::CeremonyIndexType,
};

#[rpc]
pub trait CeremoniesApi<BlockHash, AccountId>
pub trait CeremoniesApi<BlockHash, AccountId, Moment>
where
AccountId: 'static + Encode + Decode + Send + Sync,
Moment: 'static + Encode + Decode + Send + Sync,
{
#[rpc(name = "ceremonies_getReputations")]
fn get_reputations(
&self,
account: AccountId,
at: Option<BlockHash>,
) -> Result<Vec<(CeremonyIndexType, CommunityReputation)>>;

#[rpc(name = "ceremonies_getAggregatedAccountData")]
fn get_aggregated_account_data(
&self,
cid: CommunityIdentifier,
account: AccountId,
at: Option<BlockHash>,
) -> Result<AggregatedAccountData<AccountId, Moment>>;
}

pub struct Ceremonies<Client, Block, AccountId, S> {
pub struct Ceremonies<Client, Block, AccountId, Moment, S> {
client: Arc<Client>,
deny_unsafe: DenyUnsafe,
storage: Arc<RwLock<S>>,
offchain_indexing: bool,
_marker: std::marker::PhantomData<(Block, AccountId)>,
_marker: std::marker::PhantomData<(Block, AccountId, Moment)>,
}

impl<Client, Block, AccountId, S> Ceremonies<Client, Block, AccountId, S>
impl<Client, Block, AccountId, Moment, S> Ceremonies<Client, Block, AccountId, Moment, S>
where
S: 'static + OffchainStorage,
Block: sp_api::BlockT,
AccountId: 'static + Encode + Decode + Send + Sync,
Moment: 'static + Encode + Decode + Send + Sync,
Client: Send + Sync + 'static + ProvideRuntimeApi<Block> + HeaderBackend<Block>,
Client::Api: CeremoniesRuntimeApi<Block, AccountId>,
Client::Api: CeremoniesRuntimeApi<Block, AccountId, Moment>,
encointer_primitives::ceremonies::AggregatedAccountData<AccountId, Moment>: Decode,
{
/// Create new `Ceremonies` instance with the given reference to the client.
pub fn new(
Expand Down Expand Up @@ -118,14 +130,16 @@ where
}
}

impl<Client, Block, AccountId, S> CeremoniesApi<<Block as BlockT>::Hash, AccountId>
for Ceremonies<Client, Block, AccountId, S>
impl<Client, Block, AccountId, Moment, S> CeremoniesApi<<Block as BlockT>::Hash, AccountId, Moment>
for Ceremonies<Client, Block, AccountId, Moment, S>
where
AccountId: 'static + Clone + Encode + Decode + Send + Sync + PartialEq,
Moment: 'static + Clone + Encode + Decode + Send + Sync + PartialEq,
Block: BlockT,
Client: Send + Sync + 'static + ProvideRuntimeApi<Block> + HeaderBackend<Block>,
Client::Api: CeremoniesRuntimeApi<Block, AccountId>,
Client::Api: CeremoniesRuntimeApi<Block, AccountId, Moment>,
S: 'static + OffchainStorage,
encointer_primitives::ceremonies::AggregatedAccountData<AccountId, Moment>: Decode,
{
fn get_reputations(
&self,
Expand All @@ -148,6 +162,19 @@ where
None => Err(storage_not_found_error(cache_key)),
}
}

fn get_aggregated_account_data(
&self,
cid: CommunityIdentifier,
account: AccountId,
at: Option<<Block as BlockT>::Hash>,
) -> Result<AggregatedAccountData<AccountId, Moment>> {
self.deny_unsafe.check_if_safe()?;
let api = self.client.runtime_api();
let at = BlockId::hash(at.unwrap_or_else(|| self.client.info().best_hash));
api.get_aggregated_account_data(&at, cid, &account)
.map_err(runtime_error_into_rpc_err)
}
}

const RUNTIME_ERROR: i64 = 1; // Arbitrary number, but substrate uses the same
Expand Down
155 changes: 121 additions & 34 deletions ceremonies/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

use codec::{Decode, Encode};
use encointer_ceremonies_assignment::{
assignment_fn_inverse, generate_assignment_function_params,
assignment_fn_inverse, generate_assignment_function_params, get_meetup_location_index,
math::{checked_ceil_division, find_prime_below, find_random_coprime_below},
meetup_index, meetup_location, meetup_time,
};
Expand Down Expand Up @@ -899,6 +899,66 @@ impl<T: Config> Pallet<T> {
.collect()
}

pub fn get_aggregated_account_data(
cid: CommunityIdentifier,
account: &T::AccountId,
) -> AggregatedAccountData<T::AccountId, T::Moment> {
let cindex = <encointer_scheduler::Pallet<T>>::current_ceremony_index();
let aggregated_account_data_global = AggregatedAccountDataGlobal {
ceremony_phase: <encointer_scheduler::Pallet<T>>::current_phase(),
ceremony_index: cindex,
};

let aggregated_account_data_personal: Option<
AggregatedAccountDataPersonal<T::AccountId, T::Moment>,
>;

Copy link
Member

Choose a reason for hiding this comment

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

I'd extract a function here:

fn get_aggregated_account_data_personal(...) -> Option<AggregatedAccountDataPersonal>

So you can do:

AgregatedAccountData {
              global: ...
              personal: get_agregated_account_data_personal(...)
}

This will remove the unreadable branching in the code.

// check if the participant is registered. if not the entire personal field will be None.
if let Some(participant_type) = Self::get_participant_type((cid, cindex), account) {
let mut meetup_location_index: Option<MeetupIndexType> = None;
let mut meetup_time: Option<T::Moment> = None;
let mut meetup_registry: Option<Vec<T::AccountId>> = None;
let mut meetup_index: Option<MeetupIndexType> = None;

Comment on lines +918 to +922
Copy link
Member

Choose a reason for hiding this comment

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

Why do you predefine those fields, we same some lines of code if this is defined in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i do it because i use a bunch of if statements to assign those variables. if i dont predefine them here, they woule be undefined in some cases. if there is a more elegant solution to this, please give me a hint :)

Copy link
Member

@clangenb clangenb Apr 6, 2022 •
edited

Choose a reason for hiding this comment

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

It needs a bit of thinking, but I think you can. The code is hard to read like this, I'd want to change that. Basically, all the fields are null if participant_meetup_index is None correct? Then I'd propose the following:

let participant_meetup_index = match Self::get_meetup_index() {
    Some(p_index) => p_index,
    None => return Some(AggregatedAccountDataPersonal::new_unassigned(participant_type));
}

.... continue as usual

This pattern would, however, require extracting get_agregated_account_data_personal as advised above. I think you did not see that message because it was a hidden conversation.

// check if the participant is already assigned to a meetup
if let Some(participant_meetup_index) = Self::get_meetup_index((cid, cindex), account) {
meetup_index = Some(participant_meetup_index);
let locations = <encointer_communities::Pallet<T>>::get_locations(&cid);
let location_assignment_params = Self::assignments((cid, cindex)).locations;

meetup_location_index = get_meetup_location_index(
participant_meetup_index,
&locations,
location_assignment_params,
);
if let Some(location) =
Self::get_meetup_location((cid, cindex), participant_meetup_index)
{
meetup_time = Self::get_meetup_time(location);
}

meetup_registry =
Some(Self::get_meetup_participants((cid, cindex), participant_meetup_index));
}

aggregated_account_data_personal =
Some(AggregatedAccountDataPersonal::<T::AccountId, T::Moment> {
participant_type,
meetup_index,
meetup_location_index,
meetup_time,
meetup_registry,
});
} else {
aggregated_account_data_personal = None;
}
let aggregated_account_data = AggregatedAccountData::<T::AccountId, T::Moment> {
global: aggregated_account_data_global,
personal: aggregated_account_data_personal,
};
aggregated_account_data
}

fn register(
cid: CommunityIdentifier,
cindex: CeremonyIndexType,
Expand Down Expand Up @@ -1189,6 +1249,25 @@ impl<T: Config> Pallet<T> {
}
}

fn get_participant_type(
pifragile marked this conversation as resolved.
Show resolved Hide resolved
community_ceremony: CommunityCeremony,
participant: &T::AccountId,
) -> Option<ParticipantType> {
if <BootstrapperIndex<T>>::contains_key(community_ceremony, &participant) {
return Some(ParticipantType::Bootstrapper)
}
if <ReputableIndex<T>>::contains_key(community_ceremony, &participant) {
return Some(ParticipantType::Reputable)
}
if <EndorseeIndex<T>>::contains_key(community_ceremony, &participant) {
return Some(ParticipantType::Endorsee)
}
if <NewbieIndex<T>>::contains_key(community_ceremony, &participant) {
return Some(ParticipantType::Newbie)
}
None
}

fn get_meetup_index(
community_ceremony: CommunityCeremony,
participant: &T::AccountId,
Expand All @@ -1198,42 +1277,50 @@ impl<T: Config> Pallet<T> {

let assignment = Self::assignments(community_ceremony);

if <BootstrapperIndex<T>>::contains_key(community_ceremony, &participant) {
let participant_index = Self::bootstrapper_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.bootstrappers {
return meetup_index(
participant_index,
assignment.bootstrappers_reputables,
meetup_count,
)
}
}
if <ReputableIndex<T>>::contains_key(community_ceremony, &participant) {
let participant_index = Self::reputable_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.reputables {
return meetup_index(
participant_index + assignment_count.bootstrappers,
assignment.bootstrappers_reputables,
meetup_count,
)
}
}
let participant_type = Self::get_participant_type(community_ceremony, participant)?;

if <EndorseeIndex<T>>::contains_key(community_ceremony, &participant) {
let participant_index = Self::endorsee_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.endorsees {
return meetup_index(participant_index, assignment.endorsees, meetup_count)
}
}
let (participant_index, assignment_params) = match participant_type {
ParticipantType::Bootstrapper => {
let participant_index =
Self::bootstrapper_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.bootstrappers {
(participant_index, assignment.bootstrappers_reputables)
} else {
return None
}
},
ParticipantType::Reputable => {
let participant_index = Self::reputable_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.reputables {
(
participant_index + assignment_count.bootstrappers,
assignment.bootstrappers_reputables,
)
} else {
return None
}
},

if <NewbieIndex<T>>::contains_key(community_ceremony, &participant) {
let participant_index = Self::newbie_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.newbies {
return meetup_index(participant_index, assignment.newbies, meetup_count)
}
}
ParticipantType::Endorsee => {
let participant_index = Self::endorsee_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.endorsees {
(participant_index, assignment.endorsees)
} else {
return None
}
},

None
ParticipantType::Newbie => {
let participant_index = Self::newbie_index(community_ceremony, &participant) - 1;
if participant_index < assignment_count.newbies {
(participant_index, assignment.newbies)
} else {
return None
}
},
};

meetup_index(participant_index, assignment_params, meetup_count)
}

fn get_meetup_participants(
Expand Down