Feat/analytics and insight functions and tests#106
Feat/analytics and insight functions and tests#106Utilitycoder merged 11 commits intoFundable-Protocol:mainfrom FrankiePower:feat/analytics_and_insight_functions
Conversation
WalkthroughTwo analytics functions, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CampaignDonationContract
User->>CampaignDonationContract: donate(campaign_id, amount)
alt First-time donor to campaign
CampaignDonationContract->>CampaignDonationContract: Mark donor as contributed
CampaignDonationContract->>CampaignDonationContract: Increment unique_donors_count
end
CampaignDonationContract->>CampaignDonationContract: Update campaign balance
User->>CampaignDonationContract: get_campaign_progress(campaign_id)
CampaignDonationContract->>CampaignDonationContract: Calculate (balance * 100 / target), cap at 100
CampaignDonationContract-->>User: progress_percentage
User->>CampaignDonationContract: get_campaign_donor_count(campaign_id)
CampaignDonationContract-->>User: unique_donors_count
Estimated code review effort2 (~18 minutes) Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@mubarak23 this pr is up for review and is ready to be merged. |
|
@FrankiePower , fix the merge conflict above |
mubarak23
left a comment
mubarak23
left a comment
There was a problem hiding this comment.
LGFM,
let fix the merge conflict
|
@FrankiePower are you still working on fixing this merge conflict |
|
yes i am sorry about the delay. i will get back to you by end of day |
|
@FrankiePower are you still working on fixing this merge conflict |
|
@mubarak23 chief sorry i wasn't able to attend to this since, but i am free now and would like to resolve it |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Campaign Functions Lack Validation, Risk Overflow
The get_campaign_progress function has a potential u256 overflow: the multiplication (campaign.current_balance * 100) can panic if current_balance is very large. Additionally, both get_campaign_progress and get_campaign_donor_count functions lack campaign ID validation. They return misleading default values (0) for non-existent campaign IDs, contradicting the PR description's claim of adding validation.
src/campaign_donation.cairo#L311-L337
fundable/src/campaign_donation.cairo
Lines 311 to 337 in 2984080
Bug: Campaign ID Validation Missing
The get_campaign_donor_count function lacks validation for the campaign_id parameter. For non-existent campaign IDs, it misleadingly returns 0, implying a valid campaign with no donors rather than indicating an invalid ID. This contradicts the PR description's stated intent to include campaign ID validation.
src/campaign_donation.cairo#L333-L337
fundable/src/campaign_donation.cairo
Lines 333 to 337 in 2984080
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
|
@mubarak23 chief i am done with this issue. The PR is ready to be reviewed and merged. |
|
Please fix your test and formatting. And it will be merged asap. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_campaign_donation.cairo (1)
806-806: Remove stray formatting marker.There's a stray
~marker on this line that appears to be a leftover from the diff formatting.Apply this diff to clean up the formatting:
- stop_cheat_caller_address(campaign_donation.contract_address);~ + stop_cheat_caller_address(campaign_donation.contract_address);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/campaign_donation.cairo(3 hunks)tests/test_campaign_donation.cairo(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/campaign_donation.cairo
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (5)
tests/test_campaign_donation.cairo (5)
596-639: LGTM! Comprehensive test coverage for campaign progress calculation.The test effectively validates the
get_campaign_progressfunction across multiple scenarios with clear percentage calculations. The strategic donation amounts (300/1000=30%, 500/1000=50%, 1000/1000=100%) make the test easy to understand and verify.
641-694: Excellent precision testing for edge cases.This test thoroughly validates the rounding behavior of the progress calculation, including the critical edge case where 99.9% should round down to 99%. The incremental donation approach effectively tests various precision scenarios.
810-867: Well-structured test for unique donor counting.The test effectively validates the donor count functionality by systematically testing donations from different users and verifying the count increments correctly at each step. The setup properly handles multiple users with appropriate token approvals.
869-903: Critical test for unique donor logic.This test validates the essential requirement that multiple donations from the same donor should only count as one unique donor. This is crucial for the integrity of the donor counting feature.
905-960: Excellent validation of campaign-isolated donor counting.This test ensures that unique donor counts are properly tracked independently across different campaigns, validating that the same donor can contribute to multiple campaigns and be counted separately in each. The cross-campaign donation logic is well-tested.
|
Thanks for the good work, Frank! |
3 participants
Pull Request: Implement Analytics Functions for CampaignDonation Contract
🧠 Summary
This PR implements the
get_campaign_progressandget_campaign_donor_countfunctions in theCampaignDonationcontract, providing analytics for campaign performance. It enhancesget_campaign_progresswith capping, validation, rounding, and documentation, and addsget_campaign_donor_countwith gas-optimized unique donor tracking. Comprehensive tests cover all edge cases, ensuring accuracy and robustness.📌 Type of Change
Context
This PR completes the analytics functions for the
CampaignDonationcontract, enabling campaign owners and donors to track progress and donor engagement.get_campaign_progresscalculates the percentage of funds raised, andget_campaign_donor_countcounts unique donors. Fixes address gaps inget_campaign_progress(capping, validation, rounding) and implementget_campaign_donor_countwith a gas-efficient storage-based approach usingunique_donorsmapping.📝 Changes Description
get_campaign_progress:campaign_id <= campaign_counts).(balance * 1000) / target / 10, round up if ≥ 0.5).get_campaign_donor_count:unique_donorsmapping, updated indonate_to_campaign.0_u32for invalid IDs.Tests:
test_get_campaign_progressfor empty, partially funded, fully funded, overfunded.test_get_campaign_donor_countfor no donors, single donor, multiple donors, repeated donors.Design Considerations
u8(0-100) requirement, tested with 1500/1000.unique_donorsmapping forget_campaign_donor_countto avoid iterating donations, reducing gas for large lists.get_campaign_progressto ensure precision.is_cancelledfield).Acceptance Criteria
@ContractState, no state changes.unique_donorsmapping optimizes donor count.📸 Evidence
On Tests:
⏰ Time Spent Breakdown
Total: ~8 hours
get_campaign_progress, implementedget_campaign_donor_count, added tests (~4 hours).Checklist
get_campaign_progressandget_campaign_donor_count.unique_donorsmapping.Conclusion
The PR fully implements
get_campaign_progressandget_campaign_donor_count, addressing all task requirements, technical considerations, and acceptance criteria. Both functions are accurate, read-only, and gas-efficient, with comprehensive tests and clear documentation. Theunique_donorsmapping optimizes donor counting, and all edge cases (including invalid IDs and cancellation assumptions) are handled.Summary by CodeRabbit
New Features
Tests