Mltpl asset type#119
Conversation
|
""" WalkthroughThe changes introduce support for campaign-specific donation tokens, allowing each campaign to define its own donation asset. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Contract
participant TokenContract
User->>Contract: create_campaign(campaign_ref, target_amount, donation_token)
Contract->>Contract: Validate donation_token != 0
alt Invalid token
Contract-->>User: Error(INVALID_DONATION_TOKEN)
else Valid token
Contract->>Contract: Store campaign with donation_token
Contract-->>User: campaign_id
end
User->>Contract: donate_to_campaign(campaign_id, amount)
Contract->>Contract: Retrieve campaign's donation_token
Contract->>TokenContract: Transfer tokens from User to Contract
TokenContract-->>Contract: Transfer result
Contract-->>User: Donation result
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
Poem
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/interfaces/ICampaignDonation.cairo (1)
28-33: Update documentation to include the new parameter.The
donation_tokenparameter should be documented in the function comments for clarity./// Creates a new fundraising campaign /// /// # Arguments /// * `campaign_ref` - A unique 5-character identifier for the campaign /// * `target_amount` - The fundraising goal amount in the donation token +/// * `donation_token` - The ERC-20 token contract address for this campaign /// /// # Returns /// * `u256` - The newly created campaign's ID /// /// # Panics /// * If `campaign_ref` is empty /// * If `campaign_ref` already exists /// * If `target_amount` is zero +/// * If `donation_token` is a zero addresstests/test_campaign_donation.cairo (1)
57-57: Remove or implement the placeholder function.The
deploy_donation_token()function is currently empty. Consider removing it if not needed or adding a TODO comment if it's intended for future implementation.-fn deploy_donation_token() {} +// TODO: Implement deploy_donation_token() function for future use +fn deploy_donation_token() {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/base/errors.cairo(1 hunks)src/campaign_donation.cairo(6 hunks)src/interfaces/ICampaignDonation.cairo(1 hunks)tests/test_campaign_donation.cairo(37 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
🔇 Additional comments (9)
src/base/errors.cairo (1)
137-138: LGTM! Error constant is well-defined.The new
INVALID_DONATION_TOKENerror constant follows the established pattern and provides a clear error message for validation of campaign-specific donation tokens.tests/test_campaign_donation.cairo (2)
99-110: LGTM! Excellent test coverage for validation.The new test properly validates that creating a campaign with a zero donation token address triggers the expected error. This ensures the validation logic works correctly.
62-70: LGTM! Test updates are comprehensive.All test functions have been properly updated to include the new
donation_tokenparameter, maintaining test coverage while adapting to the new interface.src/campaign_donation.cairo (6)
156-167: LGTM! Function signature correctly updated.The
create_campaignfunction now properly accepts thedonation_tokenparameter and passes it to the internal implementation function.
440-471: LGTM! Validation and storage implementation is correct.The
_create_campaignfunction properly validates the donation token address and stores it in the campaign struct. The validation ensures the token address is non-zero, preventing invalid campaigns.
479-479: LGTM! Correct use of campaign-specific token.The donation logic now correctly uses the campaign-specific
donation_tokeninstead of the global token, enabling multi-token support.
542-542: LGTM! Withdrawal uses campaign-specific token.The withdrawal function correctly uses the campaign-specific
donation_tokenfor transfers.
24-26: LGTM! Error import updated correctly.The error imports have been properly updated to include the new
INVALID_DONATION_TOKENconstant used for validation.
417-418: To confirm that each campaign stores its own token and thatcampaign.donation_tokenis available, let’s locate theCampaignsstruct and theget_campaignhelper:#!/bin/bash # Find the Campaigns struct definition rg -n "struct Campaigns" --glob "*.cairo" -A 10 -B 5 # Find the get_campaign implementation rg -n "fn get_campaign" --glob "*.cairo" -A 5 -B 2
|
@mubarak23 ptal |
|
@mubarak23 gm sir, fixed the check and wrote a unit test for it. failing tests are from the code last code merged into the code base https://github.com/Fundable-Protocol/fundable/actions/runs/15964704698/job/45022914786 those are the ones also failing in my pr |
closes #116
create_campaignfunction now requires adonation_token: ContractAddressparameter, making each campaign support its own ERC-20 token.Changes in Detail
1.
ErrorsModuleUsed to validate the provided token address is not zero.
2.
CampaignDonationContractcreate_campaignfunction signature:_create_campaignmethod to accept and storedonation_token:Added validation to reject zero address tokens:
Modified logic in places like
donate_to_campaign,withdraw_funds, etc., to usecampaign.donation_tokeninstead of a globally stored token:3. Interface (
ICampaignDonation)create_campaignsignature withdonation_token.4. Test File (
test_campaign_donation.cairo)Updated all calls to
create_campaign(...)to include thedonation_tokenas an extra argument.Added a new test to assert that an invalid donation token (zero address) causes a panic.
Supports multiple tokens per campaign instead of relying on a single global token.
Prevents campaigns with invalid token addresses from being created.
Improves test coverage and correctness by updating all existing test cases and adding necessary edge cases.
Ensures backward compatibility is broken in a controlled way (tests are updated accordingly).
Summary by CodeRabbit
New Features
Bug Fixes
Tests