Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
sasa-tomic
left a comment
sasa-tomic
left a comment
There was a problem hiding this comment.
This is great to have, thanks Pietro! It would very useful to have a few pages of docs: what it is, how to use it, etc., if not too much work. I can get this going if you want, just let me know.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 1
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| let nrc_xdr_permyriad: u64 = daily_rewards.iter().map(|(_, reward)| reward.rewards_total_xdr_permyriad.unwrap()).sum(); | ||
| let principal: PrincipalId = provider_id.to_string().parse().unwrap(); | ||
|
|
||
| let governance_icp = gov_rewards_map.remove(&principal).unwrap() / 100_000_000; |
There was a problem hiding this comment.
Bug: Governance Reward Missing Provider Causes Panic
Potential panic if provider exists in NRC results but not in governance rewards map. The code calls gov_rewards_map.remove(&principal).unwrap() which will panic if the provider ID is not found in the governance map. This can happen if a provider exists in the Node Rewards Canister but wasn't included in the governance snapshot (e.g., newly added providers, or data synchronization issues). The code should handle the missing case gracefully, either by skipping the provider with a warning or using a default value of 0 for governance rewards.
| let end_day = DateTime::from_timestamp(last.timestamp as i64, 0) | ||
| .unwrap() | ||
| .date_naive() | ||
| .pred_opt() |
There was a problem hiding this comment.
Bug: Unchecked unwraps cause panics on timestamp parsing failure
Multiple unchecked unwrap() calls on DateTime::from_timestamp() and pred_opt(). If timestamp conversion fails or pred_opt() returns None, the code will panic instead of returning a proper error.
| .into_iter() | ||
| .map(|r| (r.node_provider.unwrap().id.unwrap(), r.amount_e8s)) | ||
| .collect(); | ||
| let xdr_permyriad_per_icp: u64 = last.xdr_conversion_rate.clone().unwrap().xdr_permyriad_per_icp.unwrap(); |
There was a problem hiding this comment.
Bug: Unchecked unwraps cause governance panic on None fields
Unchecked unwrap() calls on node_provider, id, xdr_conversion_rate, and xdr_permyriad_per_icp. If any of these Optional fields are None in the governance response, the code will panic.
| self.create_node_metrics_csv(&provider_dir, daily_rewards)?; | ||
| } | ||
|
|
||
| // Generate subnets failure rates CSV in the rewards directory |
There was a problem hiding this comment.
Bug: Uncaught Panics on File System Operations Errors
Multiple unwrap() calls on file system operations (create_dir_all, Writer::from_path, write_record, flush) throughout csv_generator.rs. These operations can fail due to permission issues, disk space, etc., but failures will cause panics instead of returning proper errors.
| .and_utc() | ||
| .timestamp(); | ||
| let start_day = DateTime::from_timestamp(last_rewards.timestamp as i64, 0).unwrap().date_naive(); | ||
| let end_day = DateTime::from_timestamp(end_ts, 0).unwrap().date_naive(); |
There was a problem hiding this comment.
Bug: Unsafe unwrap on timestamp causes panic
The code calls .unwrap() on DateTime::from_timestamp() which will panic if the timestamp is out of range. This should use proper error handling with ? operator or ok_or_else().
2 participants
Note
Introduce a node-rewards CLI that queries and compares NRC vs Governance rewards with CSV export, plus docs, plotting script, and supporting canister client APIs.
node-rewardscommand with modesongoingandpast-rewards <YYYY-MM>.NRCand Governance, compare totals, and show daily summaries.base_rewards.csv,base_rewards_type3.csv,rewards_summary.csv,node_metrics_by_day.csv,node_metrics_by_node.csv) plus globalsubnets_failure_rates.csv.NodeRewardsCanisterWrapperto queryget_node_providers_rewards_calculation.GovernanceCanisterWrapperwithlist_node_provider_rewards.docs/node-rewards.md; linked inmkdocs.yml.scripts/plot_subnet_failure_rates.pyto visualize subnet failure rates from exported CSVs.csv,csv_to_table,ic-node-rewards-canister-api,rust_decimal; wire intors/cliandic-canisters..gitignoretweak.Written by Cursor Bugbot for commit e2626cd. This will update automatically on new commits. Configure here.