Skip to content

work on the batch donation function#105

Closed
Abidoyesimze wants to merge 5 commits into
Fundable-Protocol:mainfrom
Abidoyesimze:batchDonation
Closed

work on the batch donation function#105
Abidoyesimze wants to merge 5 commits into
Fundable-Protocol:mainfrom
Abidoyesimze:batchDonation

Conversation

@Abidoyesimze

@Abidoyesimze Abidoyesimze commented Jun 1, 2025

Copy link
Copy Markdown

Summary by CodeRabbit

  • New Features
    • Introduced batch donation functionality, allowing users to donate to multiple campaigns in a single transaction with an all-or-nothing guarantee.
    • Added detailed batch donation result reporting and new events summarizing batch donation outcomes.
    • Implemented automatic capping of donation amounts to campaign targets to prevent overfunding.
  • Bug Fixes
    • Fixed compilation issues and improved gas efficiency in batch donation validations.
  • Tests
    • Disabled certain existing tests related to campaign creation and fund withdrawal; no new tests added.

@coderabbitai

coderabbitai Bot commented Jun 1, 2025

Copy link
Copy Markdown

Walkthrough

The changes introduce a batch donation feature to the campaign donation contract, enabling donors to contribute to multiple campaigns in a single transaction. This includes new functions for validation, auto-capping, processing, and event emission, as well as updates to the interface and some test adjustments. Minor formatting cleanup is also included.

Changes

File(s) Change Summary
src/campaign_donation.cairo Added batch donation logic with validation, auto-capping, single token transfer, new helper functions, refactor, and cleanup.
src/interfaces/ICampaignDonation.cairo Extended interface with batch donation method, new structs/events (DonationResult, BatchDonationProcessed), enhanced documentation.
tests/test_campaign_donation.cairo Disabled three existing tests; minor whitespace cleanup; no new tests added in this diff.
src/lib.cairo Added a trailing newline; no functional changes.

Sequence Diagram(s)

sequenceDiagram
    participant Donor
    participant CampaignDonationContract
    participant ERC20Token

    Donor->>CampaignDonationContract: batch_donate(campaign_amounts)
    CampaignDonationContract->>CampaignDonationContract: _validate_and_calculate_total_optimized(campaign_amounts)
    CampaignDonationContract->>ERC20Token: transfer_from(Donor, Contract, total_amount)
    ERC20Token-->>CampaignDonationContract: Transfer result
    loop For each (campaign_id, amount) in campaign_amounts
        CampaignDonationContract->>CampaignDonationContract: _process_internal_donation_with_return(Donor, campaign_id, amount)
    end
    CampaignDonationContract-->>Donor: Emit BatchDonationProcessed event
Loading

Possibly related issues

Possibly related PRs

Suggested reviewers

  • mubarak23

Poem

In the garden of code, where campaigns bloom,
A rabbit hops, bringing batch gifts to the room.
One hop, many causes, with a single leap,
Donations now bundled, efficiency to keep.
With every campaign, a new hope is sown—
Batch giving, together, we’ve grown!
🐇💌


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
src/campaign_donation.cairo (1)

194-194: 🛠️ Refactor suggestion

Remove duplicate timestamp declaration.

The timestamp variable is already declared at line 156 and not used until line 196. This duplicate declaration is redundant.

Remove this line as the timestamp is already declared above:

-            let timestamp = get_block_timestamp();
🧹 Nitpick comments (1)
src/campaign_donation.cairo (1)

253-254: Consider handling mid-batch campaign completion.

The current validation approach checks all campaigns upfront. If multiple donations target the same campaign, and it reaches its goal mid-batch, the validation would prevent this valid scenario. Consider adjusting the validation to account for cumulative donations within the batch.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ddfda0 and 04d8b91.

📒 Files selected for processing (3)
  • src/campaign_donation.cairo (3 hunks)
  • src/interfaces/ICampaignDonation.cairo (3 hunks)
  • src/lib.cairo (1 hunks)
🔇 Additional comments (5)
src/lib.cairo (1)

7-7: LGTM!

Adding a trailing newline is a good practice for text files.

src/interfaces/ICampaignDonation.cairo (2)

12-18: Well-designed result structure for batch operations.

The DonationResult struct appropriately captures all necessary information for tracking individual donation outcomes within a batch.


90-111: Comprehensive documentation for batch donation function.

The batch_donate function is well-documented with clear requirements, constraints, and effects. The atomic all-or-nothing approach is appropriate for maintaining data consistency.

src/campaign_donation.cairo (2)

374-404: Robust validation logic with overflow protection.

The validation function properly checks all requirements and includes overflow protection for the total calculation. Good defensive programming.


415-417:

Details

❓ Verification inconclusive

Document the auto-capping behavior in the interface.

The implementation automatically caps donations at the remaining amount needed to reach the target. This behavior differs from the single donation function which rejects amounts exceeding the target. This should be clearly documented in the interface to set proper expectations.

Consider whether this auto-capping is the desired behavior for batch donations, as it creates inconsistency with the single donation function.


Document Auto-Capping in Batch Donation Interface

The _process_internal_donation function silently caps any donation amount that exceeds the remaining needed to reach the campaign target. This differs from the single‐donation entrypoint, which currently rejects over-target amounts. To avoid confusion:

  • In src/campaign_donation.cairo, above the _process_internal_donation signature, add a doc comment such as:
    “/// If amount exceeds the remaining needed to hit campaign.target_amount, it is automatically reduced to that remaining amount.”
  • Update your public interface/docs to call out this auto-capping behavior for batch donations.
  • Decide if this silent cap is the intended batch-donation UX or whether you’d prefer to mirror the single-donation rejection behavior for consistency.

Comment on lines +21 to +28
#[derive(Drop, starknet::Event)]
pub struct BatchDonationProcessed {
pub donor: ContractAddress,
pub total_campaigns: u32,
pub successful_donations: u32,
pub total_amount: u256,
pub results: Array<DonationResult>,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Event defined but not emitted in implementation.

The BatchDonationProcessed event is well-structured but is not emitted in the batch_donate implementation. This event should be emitted after successful batch processing to maintain consistency with the interface design.

Run the following script to verify the event usage:


🏁 Script executed:

#!/bin/bash
# Description: Check if BatchDonationProcessed event is emitted in the implementation

# Search for BatchDonationProcessed event emission
rg -A 5 'BatchDonationProcessed' src/campaign_donation.cairo

Length of output: 60


I’ll check for any event emission patterns in the implementation:


🏁 Script executed:

#!/bin/bash
# Search for .emit() calls in the CampaignDonation contract
rg -n '\.emit' src/campaign_donation.cairo

# Look specifically for BatchDonationProcessed.emit
rg -n 'BatchDonationProcessed\s*\.emit' src

# Check for legacy emit_event usage
rg -n 'emit_event' src/campaign_donation.cairo

Length of output: 398


Add missing BatchDonationProcessed event emission

The BatchDonationProcessed event defined in ICampaignDonation.cairo isn’t emitted in the batch_donate implementation (src/campaign_donation.cairo). To maintain interface consistency, emit it after all individual donations have been processed:

• File: src/campaign_donation.cairo
• Function: batch_donate (immediately after the loop emitting Event::Donation)

Example insertion:

// After processing each donation...
// existing loop emitting Event::Donation(...)

let batch_event = BatchDonationProcessed {
    donor,
    total_campaigns: campaigns.len(),      // or your total_campaigns counter
    successful_donations,                  // tracked in the function
    total_amount,                          // accumulated amount
    results,                               // collected DonationResult array
};
self.emit(Event::BatchDonationProcessed(batch_event));

This ensures the batch-summary event is actually emitted.

🤖 Prompt for AI Agents
In src/campaign_donation.cairo within the batch_donate function (after the loop
emitting individual Donation events), add an emission of the
BatchDonationProcessed event defined in src/interfaces/ICampaignDonation.cairo.
Construct a BatchDonationProcessed struct with the donor, total_campaigns
(length of campaigns), successful_donations count, total_amount accumulated, and
results array, then emit it using
self.emit(Event::BatchDonationProcessed(batch_event)) to ensure the event is
properly emitted and interface consistency is maintained.

Comment thread src/campaign_donation.cairo
@Abidoyesimze

Copy link
Copy Markdown
Author

i just update it pls check it out and if there's still any fixes needed let me know

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/campaign_donation.cairo (2)

54-54: Addresses previous review feedback.

The missing BatchDonationProcessed event has been properly added to the event enum, addressing the previous review comment about missing batch event emission.


282-321: Successfully addresses previous review feedback.

The implementation now properly tracks donation results and emits the BatchDonationProcessed event, fully addressing the missing functionality identified in previous reviews.

🧹 Nitpick comments (1)
src/campaign_donation.cairo (1)

449-505:

Details

❓ Verification inconclusive

Review gas efficiency of nested validation loops.

The _validate_and_calculate_total_dynamic function uses nested loops to calculate cumulative amounts per campaign, which could be gas-expensive for larger batches. While the logic appears correct for handling mid-batch campaign completion, consider the gas implications.

For a batch of size 20 with diverse campaigns, this could result in up to 400 iterations (20²).

Consider optimizing the validation by using a temporary mapping structure:

-        // Calculate cumulative amount for this campaign in the batch (simplified)
-        let mut current_batch_total: u256 = 0;
-        let mut j = 0;
-        while j <= i {
-            let (check_campaign_id, check_amount) = *campaign_amounts.at(j);
-            if check_campaign_id == campaign_id {
-                current_batch_total += check_amount;
-            }
-            j += 1;
-        };
+        // Use a more efficient approach with pre-calculated totals per campaign
+        // This would require restructuring to calculate all campaign totals upfront

Verify the gas usage with a test containing maximum batch size targeting the same campaign:


🏁 Script executed:

#!/bin/bash
# Look for existing gas benchmarks or tests for batch donations
rg -A 5 -B 5 "gas|benchmark|MAX_BATCH_SIZE.*test" --type cairo

Length of output: 94


Review gas efficiency of nested validation loops

  • We couldn’t locate any existing gas benchmarks or tests for batch donations using rg -A 5 -B 5 "gas|benchmark|MAX_BATCH_SIZE.*test". Please add or locate benchmarks (e.g., a max-batch-size gas test) and verify the actual gas cost for large batches.
  • To avoid O(n²) behavior, consider precomputing per-campaign totals in a temporary mapping before the main loop, then use those totals instead of recalculating with nested loops:
-        // Calculate cumulative amount for this campaign in the batch (simplified)
-        let mut current_batch_total: u256 = 0;
-        let mut j = 0;
-        while j <= i {
-            let (check_campaign_id, check_amount) = *campaign_amounts.at(j);
-            if check_campaign_id == campaign_id {
-                current_batch_total += check_amount;
-            }
-            j += 1;
-        };
+        // TODO: Pre-calculate campaign batch totals in a temporary mapping
+        // and look up current_batch_total in O(1) instead of re-scanning the array
  • Location: src/campaign_donation.cairo, lines 449–505
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04d8b91 and 2f4a7e4.

📒 Files selected for processing (2)
  • src/campaign_donation.cairo (5 hunks)
  • src/interfaces/ICampaignDonation.cairo (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/interfaces/ICampaignDonation.cairo
🔇 Additional comments (5)
src/campaign_donation.cairo (5)

6-6: LGTM! Proper imports for batch functionality.

The addition of DonationResult and BatchDonationProcessed imports correctly supports the new batch donation feature.


264-280: Excellent optimization with single token transfer.

The implementation correctly performs a single transfer_from for the total amount rather than multiple transfers, which significantly reduces gas costs and improves efficiency.


340-340: Good modernization of deprecated constants.

Correctly updated from the deprecated contract_address_const to starknet::contract_address_const syntax.


507-515: Maintain backward compatibility effectively.

The wrapper function _process_internal_donation properly maintains backward compatibility by delegating to the new implementation while discarding return values.


246-262:

Details

❓ Verification inconclusive

Verify input validation and batch size limits.

The function properly validates input constraints including empty arrays and maximum batch size. The choice of 20 as MAX_BATCH_SIZE appears reasonable for gas limits.

Verify that the batch size limit aligns with realistic gas constraints:


🏁 Script executed:

#!/bin/bash
# Search for any documentation or tests that validate the MAX_BATCH_SIZE choice
rg -A 3 -B 3 "MAX_BATCH_SIZE|batch.*size|20.*batch" --type cairo

Length of output: 96


Let's search all .cairo files directly for any references to MAX_BATCH_SIZE or related batch-size rationale:


🏁 Script executed:

#!/bin/bash
# Search in all .cairo files for MAX_BATCH_SIZE and related comments/tests
rg -A 3 -B 3 "MAX_BATCH_SIZE|batch.*size|20.*batch" -g "*.cairo"

Length of output: 2106


Batch size documentation confirmed, but gas impact needs manual verification

  • Found requirement comments in src/campaign_donation.cairo (around line 246) and in src/interfaces/ICampaignDonation.cairo stating the 20-item batch limit.
  • No existing tests or benchmarks validate that MAX_BATCH_SIZE = 20 aligns with realistic gas usage.

Please review gas consumption for a 20-item batch or add a benchmark/test to ensure this limit is safe.

Comment on lines +520 to +536
fn _process_internal_donation_with_return(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) -> (u256, u256) {
let mut campaign = self.campaigns.read(campaign_id);
let timestamp = get_block_timestamp();

// Calculate actual donation amount (don't exceed target) - AUTO-CAPPING
let remaining_amount = campaign.target_amount - campaign.current_balance;
let actual_amount = if amount > remaining_amount { remaining_amount } else { amount };

// Skip if no amount to donate (campaign already fully funded)
if actual_amount == 0 {
return (0, 0);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify auto-capping logic correctness.

The auto-capping logic that reduces donation amounts when they exceed remaining campaign targets is well-implemented. However, ensure this behavior is clearly documented for users, as they might not expect their donation amounts to be automatically reduced.

Verify that the auto-capping behavior is properly documented and tested:


🏁 Script executed:

#!/bin/bash
# Search for documentation about auto-capping behavior
rg -A 10 -B 5 "auto.*cap|exceed.*target|remaining.*amount" --type md --type cairo

Length of output: 113


Let’s re-run the search across both markdown and Cairo files without relying on ripgrep’s built-in file types:


🏁 Script executed:

#!/bin/bash
# Search for auto-capping documentation in markdown files
rg -A5 -B5 'auto-capping|auto.*cap|exceed.*target|remaining.*amount' --glob '*.md'

# Search for in-code comments about auto-capping in .cairo files
rg -A5 -B5 'auto-capping|auto.*cap|exceed.*target|remaining.*amount' --glob '*.cairo'

Length of output: 8152


Here’s a script to locate and inspect any tests for the auto-capping logic:


🏁 Script executed:

#!/bin/bash
# Find all test files (Cairo and Python)
echo "Cairo tests:"
fd --extension cairo --glob '*test*.cairo'
echo "Python tests:"
fd --extension py --glob '*test*.py'

# Search within those tests for auto-capping or boundary donation scenarios
echo "Scanning Cairo tests for auto-capping patterns..."
fd --extension cairo --glob '*test*.cairo' -x rg -A5 -B5 'auto-capping|exceed.*target|remaining.*amount|_process_internal_donation_with_return' {}

echo "Scanning Python tests for donation capping..."
fd --extension py --glob '*test*.py' -x rg -A5 -B5 'auto-capping|exceed.*target|remaining.*amount|donation' {}

Length of output: 735


Add explicit test coverage for auto-capping logic

Documentation is already in place (in src/campaign_donation.cairo and src/interfaces/ICampaignDonation.cairo), but no existing tests verify that donations over the remaining target are correctly capped.

Please add test cases in tests/test_campaign_donation.cairo covering:

  • A single donation greater than the remaining campaign target is reduced to exactly the remaining amount.
  • Batch donations where cumulative amounts exceed targets are properly capped per campaign.
🤖 Prompt for AI Agents
In src/campaign_donation.cairo around lines 520 to 536, the auto-capping logic
for donations exceeding the remaining campaign target is implemented but lacks
explicit test coverage. Add new test cases in tests/test_campaign_donation.cairo
to verify that a single donation exceeding the remaining target is capped
correctly to the remaining amount, and that batch donations cumulatively
exceeding targets are also properly capped per campaign. This will ensure the
auto-capping behavior is reliably tested.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/campaign_donation.cairo (2)

289-327: Successfully implemented batch event emission and result tracking!

The implementation correctly addresses the previous review feedback:

  • BatchDonationProcessed event is properly emitted after batch processing
  • DonationResult structure tracks individual donation outcomes
  • ✅ Individual Donation events are emitted for each successful donation

The batch donation feature is well-structured with proper result aggregation.


559-566: Well-implemented auto-capping logic!

The auto-capping logic correctly:

  • Calculates the remaining amount needed to reach the campaign target
  • Caps the donation to this remaining amount if it exceeds it
  • Skips creating donation records when the effective amount is 0
  • Prevents overfunding of campaigns

This implementation aligns perfectly with the documented behavior and test expectations.

🧹 Nitpick comments (2)
tests/test_campaign_donation.cairo (1)

519-581: Consider consolidating duplicate test coverage

Both test_single_donation_auto_capping and test_batch_donation_auto_capping_single_campaign test very similar scenarios where donations to a single campaign are auto-capped. The main difference is that one uses donate_to_campaign while the other uses batch_donate with multiple entries for the same campaign.

Consider either:

  1. Removing one of these tests if the coverage is redundant
  2. Clearly differentiating their test objectives in comments
  3. Combining them into a single comprehensive test

Also applies to: 839-901

src/campaign_donation.cairo (1)

479-494: Optimize array rebuilding to improve gas efficiency

The current implementation rebuilds the entire array when updating a campaign total, which is O(n) for each update. This results in O(n²) complexity for the array management portion.

Consider using a more efficient approach:

  1. Sort the input array by campaign_id first (O(n log n))
  2. Then aggregate consecutive entries in a single pass (O(n))
  3. Or accept the temporary overhead during validation since the array is bounded by MAX_BATCH_SIZE=20

The current approach is acceptable given the small batch size limit, but could be optimized if batch sizes increase in the future.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f4a7e4 and 52fcd92.

📒 Files selected for processing (2)
  • src/campaign_donation.cairo (5 hunks)
  • tests/test_campaign_donation.cairo (1 hunks)
🔇 Additional comments (2)
tests/test_campaign_donation.cairo (1)

514-803: Excellent test coverage for auto-capping feature!

The comprehensive test suite effectively addresses the reviewer's previous feedback by covering:

  • Single donation auto-capping
  • Batch donations with multiple campaigns
  • Zero amount scenarios when campaigns are fully funded
  • Mixed scenarios with varying donation states
  • Event emission verification

The tests ensure that the auto-capping logic works correctly and donors are only charged for the actual amounts donated.

src/campaign_donation.cairo (1)

343-349: Good migration from deprecated contract_address_const!

Successfully replaced all instances of the deprecated contract_address_const with try_into().unwrap(). This ensures compatibility with newer Cairo versions.

Also applies to: 393-410

Comment thread tests/test_campaign_donation.cairo Outdated
Comment thread src/campaign_donation.cairo
@mubarak23
mubarak23 self-requested a review June 10, 2025 12:07
@mubarak23

Copy link
Copy Markdown
Contributor

@Abidoyesimze are you still working on fixing this Merge conflicts

@Abidoyesimze

Copy link
Copy Markdown
Author

Ohhh I will check it out now

@mubarak23

Copy link
Copy Markdown
Contributor

@Abidoyesimze are you still working on fixing this merge conflicts

@Abidoyesimze

Abidoyesimze commented Jun 19, 2025 via email

Copy link
Copy Markdown
Author

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (1)
src/campaign_donation.cairo (1)

60-73: Critical: Missing BatchDonationProcessed event in Event enum.

The BatchDonationProcessed event is being emitted on line 370 but is not defined in the Event enum. This will cause compilation errors.

Add the missing event to the Event enum and import the required types:

+use crate::interfaces::ICampaignDonation::{DonationResult, BatchDonationProcessed};

 #[event]
 #[derive(Drop, starknet::Event)]
 enum Event {
     Campaign: Campaign,
     Donation: Donation,
     CampaignWithdrawal: CampaignWithdrawal,
     CampaignUpdated: CampaignUpdated,
     CampaignCancelled: CampaignCancelled,
     CampaignRefunded: CampaignRefunded,
+    BatchDonationProcessed: BatchDonationProcessed,
     #[flat]
     OwnableEvent: OwnableComponent::Event,
     #[flat]
     UpgradeableEvent: UpgradeableComponent::Event,
 }
♻️ Duplicate comments (1)
src/campaign_donation.cairo (1)

757-806: ****

The auto-capping logic implementation is correct and well-documented. Past review comments have already verified this functionality.

🧹 Nitpick comments (1)
src/campaign_donation.cairo (1)

1-1: Minor: Consider updating the comment.

The comment mentions "compilation fixes" but the current changes are primarily about implementing batch donation functionality rather than just fixing compilation issues.

-/// CampaignDonation contract implementation with compilation fixes
+/// CampaignDonation contract implementation with batch donation support
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f099f4a and e213a01.

📒 Files selected for processing (4)
  • src/campaign_donation.cairo (5 hunks)
  • src/interfaces/ICampaignDonation.cairo (3 hunks)
  • src/lib.cairo (1 hunks)
  • tests/test_campaign_donation.cairo (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib.cairo
  • tests/test_campaign_donation.cairo
🔇 Additional comments (9)
src/interfaces/ICampaignDonation.cairo (3)

12-18: Well-designed DonationResult structure.

The DonationResult struct effectively captures all essential information for tracking individual donation outcomes within batch operations, including success status and donation ID for proper auditing.


20-28: Comprehensive BatchDonationProcessed event structure.

The event captures all necessary batch operation metadata including donor, counts, amounts, and detailed results array, enabling proper monitoring and analytics of batch donations.


90-125: Excellent documentation for batch_donate function.

The function documentation clearly explains the all-or-nothing approach, auto-capping behavior, validation requirements, and effects. The distinction between single and batch donation behavior regarding auto-capping is particularly well-documented.

src/campaign_donation.cairo (6)

280-310: Well-structured batch donation validation.

The input validation properly checks for empty arrays, batch size limits, and initializes necessary variables. The MAX_BATCH_SIZE of 20 is reasonable for gas optimization.


310-330: Efficient token transfer approach.

The single token transfer for the total amount is a good gas optimization strategy. The balance and allowance checks before transfer prevent common failure scenarios.


331-377: Excellent result tracking and refund mechanism.

The implementation properly tracks individual donation results and handles the critical case where actual donations are less than transferred amount due to auto-capping. The refund mechanism ensures no funds are left unallocated.


393-399: Good fix for deprecated contract address usage.

Replacing contract_address_const with try_into().unwrap() is the correct approach for modern Cairo. The fix maintains the same functionality while using supported syntax.


600-616: Consistent application of contract address fix.

The systematic replacement of deprecated contract_address_const with try_into().unwrap() across all token address initializations maintains consistency and future compatibility.


657-742: Well-optimized validation algorithm.

The gas-optimized validation function effectively reduces complexity from O(n²) to O(n) by pre-calculating campaign totals. The auto-capping logic correctly handles cases where batch donations exceed remaining campaign targets.

Comment on lines +743 to +806

fn _process_internal_donation(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) {
// Use the new function for backward compatibility
let (_donation_id, _actual_amount) = self._process_internal_donation_with_return(donor, campaign_id, amount);
}

/// Process internal donation with return values for batch tracking
/// If amount exceeds the remaining needed to hit campaign.target_amount,
/// it is automatically reduced to that remaining amount.
fn _process_internal_donation_with_return(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) -> (u256, u256) {
let mut campaign = self.campaigns.read(campaign_id);
let timestamp = get_block_timestamp();

// Calculate actual donation amount (don't exceed target) - AUTO-CAPPING
let remaining_amount = campaign.target_amount - campaign.current_balance;
let actual_amount = if amount > remaining_amount { remaining_amount } else { amount };

// Skip if no amount to donate (campaign already fully funded)
if actual_amount == 0 {
return (0, 0);
}

// Get next donation ID
let donation_id = self.donation_count.read() + 1;

// Update campaign amount
campaign.current_balance = campaign.current_balance + actual_amount;

// If goal reached, mark as closed
if campaign.current_balance >= campaign.target_amount {
campaign.is_goal_reached = true;
campaign.is_closed = true;
}

self.campaigns.write(campaign_id, campaign);

// Create donation record
let donation = Donations { donation_id, donor, campaign_id, amount: actual_amount };

// Properly append to the Vec using push
self.donations.entry(campaign_id).push(donation);

self.donation_count.write(donation_id);

// Update the per-campaign donation count
let campaign_donation_count = self.donation_counts.read(campaign_id);
self.donation_counts.write(campaign_id, campaign_donation_count + 1);

// Emit donation event for each successful donation
self.emit(Event::Donation(Donation { donor, campaign_id, amount: actual_amount, timestamp }));

// Return both donation_id and actual_amount for tracking
(donation_id, actual_amount)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing CampaignBatchTotal struct definition.

The _validate_and_calculate_total_optimized function references CampaignBatchTotal struct but it's not defined anywhere in the file.

Add the missing struct definition before its usage:

+#[derive(Drop, Copy)]
+struct CampaignBatchTotal {
+    campaign_id: u256,
+    total_amount: u256,
+}
+
 #[generate_trait]
 impl InternalImpl of InternalTrait {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn _process_internal_donation(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) {
// Use the new function for backward compatibility
let (_donation_id, _actual_amount) = self._process_internal_donation_with_return(donor, campaign_id, amount);
}
/// Process internal donation with return values for batch tracking
/// If amount exceeds the remaining needed to hit campaign.target_amount,
/// it is automatically reduced to that remaining amount.
fn _process_internal_donation_with_return(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) -> (u256, u256) {
let mut campaign = self.campaigns.read(campaign_id);
let timestamp = get_block_timestamp();
// Calculate actual donation amount (don't exceed target) - AUTO-CAPPING
let remaining_amount = campaign.target_amount - campaign.current_balance;
let actual_amount = if amount > remaining_amount { remaining_amount } else { amount };
// Skip if no amount to donate (campaign already fully funded)
if actual_amount == 0 {
return (0, 0);
}
// Get next donation ID
let donation_id = self.donation_count.read() + 1;
// Update campaign amount
campaign.current_balance = campaign.current_balance + actual_amount;
// If goal reached, mark as closed
if campaign.current_balance >= campaign.target_amount {
campaign.is_goal_reached = true;
campaign.is_closed = true;
}
self.campaigns.write(campaign_id, campaign);
// Create donation record
let donation = Donations { donation_id, donor, campaign_id, amount: actual_amount };
// Properly append to the Vec using push
self.donations.entry(campaign_id).push(donation);
self.donation_count.write(donation_id);
// Update the per-campaign donation count
let campaign_donation_count = self.donation_counts.read(campaign_id);
self.donation_counts.write(campaign_id, campaign_donation_count + 1);
// Emit donation event for each successful donation
self.emit(Event::Donation(Donation { donor, campaign_id, amount: actual_amount, timestamp }));
// Return both donation_id and actual_amount for tracking
(donation_id, actual_amount)
}
// Add the missing struct definition before the generated trait implementation
#[derive(Drop, Copy)]
struct CampaignBatchTotal {
campaign_id: u256,
total_amount: u256,
}
#[generate_trait]
impl InternalImpl of InternalTrait {
fn _process_internal_donation(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) {
// Use the new function for backward compatibility
let (_donation_id, _actual_amount) = self._process_internal_donation_with_return(donor, campaign_id, amount);
}
/// Process internal donation with return values for batch tracking
/// If amount exceeds the remaining needed to hit campaign.target_amount,
/// it is automatically reduced to that remaining amount.
fn _process_internal_donation_with_return(
ref self: ContractState,
donor: ContractAddress,
campaign_id: u256,
amount: u256
) -> (u256, u256) {
let mut campaign = self.campaigns.read(campaign_id);
let timestamp = get_block_timestamp();
// Calculate actual donation amount (don't exceed target) - AUTO-CAPPING
let remaining_amount = campaign.target_amount - campaign.current_balance;
let actual_amount = if amount > remaining_amount { remaining_amount } else { amount };
// Skip if no amount to donate (campaign already fully funded)
if actual_amount == 0 {
return (0, 0);
}
// Get next donation ID
let donation_id = self.donation_count.read() + 1;
// Update campaign amount
campaign.current_balance = campaign.current_balance + actual_amount;
// If goal reached, mark as closed
if campaign.current_balance >= campaign.target_amount {
campaign.is_goal_reached = true;
campaign.is_closed = true;
}
self.campaigns.write(campaign_id, campaign);
// Create donation record
let donation = Donations { donation_id, donor, campaign_id, amount: actual_amount };
// Properly append to the Vec using push
self.donations.entry(campaign_id).push(donation);
self.donation_count.write(donation_id);
// Update the per-campaign donation count
let campaign_donation_count = self.donation_counts.read(campaign_id);
self.donation_counts.write(campaign_id, campaign_donation_count + 1);
// Emit donation event for each successful donation
self.emit(Event::Donation(Donation { donor, campaign_id, amount: actual_amount, timestamp }));
// Return both donation_id and actual_amount for tracking
(donation_id, actual_amount)
}
}
🤖 Prompt for AI Agents
In src/campaign_donation.cairo around lines 743 to 806, the CampaignBatchTotal
struct is referenced but not defined, causing errors. Define the
CampaignBatchTotal struct before its first usage by specifying its fields and
types according to its intended use, ensuring it matches the expected data
structure for batch totals in the campaign donation logic.

@mubarak23

Copy link
Copy Markdown
Contributor

@Abidoyesimze can you format the code and push

@Abidoyesimze

Copy link
Copy Markdown
Author

Yes I'll fix the issue and push soon

@mubarak23

Copy link
Copy Markdown
Contributor

@Abidoyesimze please make sure the build and test pass before you push.

@mubarak23 mubarak23 closed this Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants