Skip to content

Commit

Permalink
Working paging with tests, reversing order of returned values to be l…
Browse files Browse the repository at this point in the history
…atest to earliest
  • Loading branch information
max-dfinity committed Aug 16, 2024
1 parent b694b84 commit 1af36ac
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 16 deletions.
3 changes: 2 additions & 1 deletion rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4574,7 +4574,8 @@ pub struct DateRangeFilter {
pub end_timestamp_seconds: Option<u64>,
}

/// A Request to list minted node provider rewards
/// A Request to list minted node provider rewards. Rewards are listed in descending order of date
/// minted, meaning that the latest rewards are always returned first.
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Debug, Clone, PartialEq)]
Expand Down
4 changes: 2 additions & 2 deletions rs/nns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,9 +743,9 @@ fn list_node_provider_rewards() {

#[candid_method(query, rename = "list_node_provider_rewards")]
fn list_node_provider_rewards_(
_req: ListNodeProviderRewardsRequest,
req: ListNodeProviderRewardsRequest,
) -> ListNodeProviderRewardsResponse {
let rewards = governance().list_node_provider_rewards(5);
let rewards = governance().list_node_provider_rewards(5, req.page);

ListNodeProviderRewardsResponse {
rewards: rewards
Expand Down
8 changes: 6 additions & 2 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4312,9 +4312,13 @@ impl Governance {
self.heap_data.most_recent_monthly_node_provider_rewards = Some(most_recent_rewards);
}

pub fn list_node_provider_rewards(&self, limit: u64) -> Vec<MonthlyNodeProviderRewards> {
pub fn list_node_provider_rewards(
&self,
limit: u64,
page: Option<u32>,
) -> Vec<MonthlyNodeProviderRewards> {
let limit = limit.min(MAX_LIST_NODE_PROVIDER_REWARDS_RESULTS);
list_node_provider_rewards(limit)
list_node_provider_rewards(limit, page)
.into_iter()
.map(|archived| match archived.version {
Some(archived_monthly_node_provider_rewards::Version::Version1(v1)) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ fn test_list_node_provider_rewards_api() {

governance.update_most_recent_monthly_node_provider_rewards(rewards_2.clone());

let result = governance.list_node_provider_rewards(2);
let result = governance.list_node_provider_rewards(2, None);

assert_eq!(result, vec![rewards_1, rewards_2]);
assert_eq!(result, vec![rewards_2, rewards_1]);
}

#[test]
Expand Down Expand Up @@ -170,12 +170,12 @@ fn test_list_node_provider_rewards_api_with_paging_and_filters() {
governance.update_most_recent_monthly_node_provider_rewards(rewards_5.clone());
governance.update_most_recent_monthly_node_provider_rewards(rewards_6.clone());

let result = governance.list_node_provider_rewards(6);
let result = governance.list_node_provider_rewards(6, None);

// limit of 5
assert_eq!(result.len(), 5);
assert_eq!(
result,
vec![rewards_2, rewards_3, rewards_4, rewards_5, rewards_6]
vec![rewards_6, rewards_5, rewards_4, rewards_3, rewards_2]
);
}
22 changes: 15 additions & 7 deletions rs/nns/governance/src/node_provider_rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
},
storage::with_node_provider_rewards_log,
};
use ic_stable_structures::{Memory, Storable};

pub(crate) fn record_node_provider_rewards(most_recent_rewards: MonthlyNodeProviderRewards) {
let rewards = ArchivedMonthlyNodeProviderRewards {
Expand All @@ -30,16 +31,23 @@ pub(crate) fn latest_node_provider_rewards() -> Option<ArchivedMonthlyNodeProvid
})
}

pub(crate) fn list_node_provider_rewards(limit: u64) -> Vec<ArchivedMonthlyNodeProviderRewards> {
pub(crate) fn list_node_provider_rewards(
limit: u64,
page: Option<u32>,
) -> Vec<ArchivedMonthlyNodeProviderRewards> {
use dfn_core::println;
// limit try_into is safe because of u32 is always equal or smaller than usize
let page = page.unwrap_or(0);

// If we have 10 entries, they're 0..9
// If we are getting newest first, we want to return 9..4, then 4..0

with_node_provider_rewards_log(|log| {
let len = log.len();
let skip = if len >= limit { len - limit } else { 0 };

log.iter()
.skip(skip.try_into().unwrap())
.take(limit.try_into().unwrap())
let end_range = len.saturating_sub(page as u64 * limit);
let start_range = end_range.saturating_sub(limit);
(start_range..end_range)
.rev()
.flat_map(|index| log.get(index))
.collect()
})
}
Expand Down

0 comments on commit 1af36ac

Please sign in to comment.