Conversation
…lated error handling
WalkthroughThe changes introduce support for specifying a donation token on a per-campaign basis instead of using a global token. This involves updating function signatures, internal logic, error handling, and tests to accept, validate, and use a campaign-specific donation token. Minor formatting and import order adjustments are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Contract
participant ERC20Token
User->>Contract: create_campaign(campaign_ref, target_amount, donation_token)
Contract->>Contract: validate donation_token != 0
alt valid token
Contract->>Contract: store campaign with donation_token
Contract-->>User: campaign created
else invalid token
Contract-->>User: revert with INVALID_DONATION_TOKEN
end
User->>Contract: donate_to_campaign(campaign_id, amount)
Contract->>Contract: get campaign's donation_token
Contract->>ERC20Token: transferFrom(User, Contract, amount)
ERC20Token-->>Contract: transfer success/failure
Contract-->>User: donation result
User->>Contract: withdraw_from_campaign(campaign_id)
Contract->>Contract: get campaign's donation_token
Contract->>ERC20Token: transfer(Contract, campaign_owner, amount)
ERC20Token-->>Contract: transfer result
Contract-->>User: withdrawal result
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
tests/test_campaign_donation.cairo (1)
1198-1211: Fix missing donation_token parameter.The test is using the old
create_campaignsignature without the requireddonation_tokenparameter.Apply this fix:
- let campaign_id = campaign_donation.create_campaign('Test', target_amount); + let campaign_id = campaign_donation.create_campaign('Test', target_amount, token_address);src/campaign_donation.cairo (1)
407-445: Fix inconsistent donation token usage in refunds.The
claim_refundfunction still uses the global donation token instead of the campaign-specific token. This is inconsistent with the new per-campaign token design.Apply this fix:
- // Transfer tokens back to donor - let donation_token = self.donation_token.read(); - let token = IERC20Dispatcher { contract_address: donation_token }; + // Transfer tokens back to donor + let donation_token = campaign.donation_token; + let token = IERC20Dispatcher { contract_address: donation_token };
🧹 Nitpick comments (1)
tests/test_campaign_donation.cairo (1)
308-321: Remove duplicate test.This test duplicates the functionality of
test_create_campaign_invalid_donation_token(lines 109-121). Both tests verify the same scenario of creating a campaign with a zero address as the donation token.Consider removing this duplicate test to avoid redundancy in the test suite.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/base/errors.cairo(1 hunks)src/campaign_donation.cairo(6 hunks)src/interfaces/ICampaignDonation.cairo(2 hunks)src/payment_stream.cairo(4 hunks)tests/test_campaign_donation.cairo(33 hunks)tests/test_payment_stream.cairo(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
🔇 Additional comments (15)
src/base/errors.cairo (1)
136-137: LGTM! Well-defined error constant for donation token validation.The new error constant follows established naming conventions and will be used to validate donation token addresses during campaign creation.
tests/test_payment_stream.cairo (1)
647-648: Good formatting improvement for readability.The multi-line format makes the chained method call more readable without affecting functionality.
src/interfaces/ICampaignDonation.cairo (1)
20-20: Good documentation for the new donation_token parameter.The parameter documentation clearly explains its purpose and aligns with the per-campaign token support objectives.
src/payment_stream.cairo (4)
25-28: Minor import reordering for consistency.The reordering of error constant imports doesn't affect functionality and may improve organization.
466-466: Good cleanup of unnecessary blank line.Removing the extraneous blank line improves code consistency.
807-810: Improved readability with multi-line event emission.The multi-line format makes the event emission more readable and consistent with similar patterns in the codebase.
915-915: Good cleanup of unnecessary blank line.Removing the extraneous blank line improves code consistency within the cancel function.
tests/test_campaign_donation.cairo (4)
71-79: LGTM!The test correctly passes the donation token to
create_campaign, aligning with the new per-campaign donation token feature.
98-106: LGTM!The test correctly includes the donation token parameter while maintaining its focus on zero amount validation.
109-121: LGTM!Good test coverage for the new invalid donation token validation. The test correctly verifies that campaign creation fails with a zero address as the donation token.
358-375: LGTM!Excellent test coverage for multiple campaigns with different donation tokens. This properly validates the per-campaign donation token feature.
src/campaign_donation.cairo (4)
166-192: LGTM!The function signature correctly includes the new
donation_tokenparameter and properly passes it to the internal implementation.
469-500: LGTM!The internal implementation correctly validates the donation token (ensuring it's not zero) and stores it in the campaign struct.
502-528: LGTM!The function correctly retrieves the donation token from the specific campaign, properly implementing the per-campaign token feature.
561-596: LGTM!The function correctly retrieves the donation token from the campaign for withdrawal operations.
| fn create_campaign( | ||
| ref self: TContractState, campaign_ref: felt252, target_amount: u256, | ||
| ref self: TContractState, | ||
| campaign_ref: felt252, | ||
| target_amount: u256, | ||
| donation_token: ContractAddress, | ||
| ) -> u256; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify consistent implementation of the new donation_token parameter.
The interface change correctly adds support for per-campaign donation tokens. Ensure that all implementations and calls to this method are updated to include the new parameter.
🏁 Script executed:
#!/bin/bash
# Description: Verify all implementations and calls to create_campaign include the donation_token parameter
# Search for create_campaign implementations
ast-grep --pattern $'fn create_campaign($$$) -> $_ {
$$$
}'
# Search for create_campaign method calls
rg -A 5 "create_campaign\(" --type cairoLength of output: 135
🏁 Script executed:
#!/bin/bash
# Locate the ICampaignDonation interface file
INTERFACE_FILE=$(fd ICampaignDonation.cairo)
# Show the interface and its method signature
echo "== Interface: $INTERFACE_FILE =="
sed -n '1,200p' "$INTERFACE_FILE"
# Search for any impl blocks of this interface
echo "== Implementations of ICampaignDonation =="
rg -n "impl ICampaignDonation" --glob "*.cairo" || echo "No impl blocks found"
# Search for all create_campaign definitions (to catch overrides)
echo "== Definitions of fn create_campaign =="
rg -n "fn create_campaign" --glob "*.cairo" || echo "No fn create_campaign definitions found"
# Search for all calls to create_campaign(
echo "== Calls to create_campaign( =="
rg -n "create_campaign\(" --glob "*.cairo" || echo "No calls to create_campaign found"Length of output: 13621
Fix missing donation_token in test calls
The interface and contract implementation already include the new donation_token parameter. However, there’s at least one test invoking create_campaign with only two arguments:
• tests/test_campaign_donation.cairo:1211
- let campaign_id = campaign_donation.create_campaign('Test', target_amount);
+ let campaign_id = campaign_donation.create_campaign('Test', target_amount, donation_token);Please update this call (and any others missing the third argument) to include donation_token so tests compile and run as expected.
📝 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.
| fn create_campaign( | |
| ref self: TContractState, campaign_ref: felt252, target_amount: u256, | |
| ref self: TContractState, | |
| campaign_ref: felt252, | |
| target_amount: u256, | |
| donation_token: ContractAddress, | |
| ) -> u256; | |
| - let campaign_id = campaign_donation.create_campaign('Test', target_amount); | |
| + let campaign_id = campaign_donation.create_campaign('Test', target_amount, donation_token); |
🤖 Prompt for AI Agents
In src/interfaces/ICampaignDonation.cairo around lines 29 to 34, the
create_campaign function signature includes a new donation_token parameter, but
test calls such as in tests/test_campaign_donation.cairo at line 1211 are
missing this argument. Update all calls to create_campaign in the test files to
pass the donation_token argument as the third parameter to match the updated
interface and ensure tests compile and run correctly.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Style