feat:handle protocol fee interface functions#118
Conversation
WalkthroughThis change introduces protocol fee management to the CampaignDonation contract. It adds storage for fee percentage and recipient address, along with getter and setter functions (restricted to the owner). The withdrawal logic now calculates and transfers protocol fees. The interface is updated to expose these fee management functions. Tests are updated to include protocol fee parameters and a new test for fee calculation is added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Owner
participant CampaignDonationContract
participant ProtocolFeeRecipient
Owner->>CampaignDonationContract: set_protocol_fee_percent(new_fee)
Owner->>CampaignDonationContract: set_protocol_fee_address(new_address)
User->>CampaignDonationContract: withdraw_from_campaign(campaign_id)
CampaignDonationContract->>CampaignDonationContract: calculate_protocol_fee(amount)
alt Protocol fee > 0
CampaignDonationContract->>ProtocolFeeRecipient: transfer(fee_amount)
end
CampaignDonationContract->>User: transfer(full_amount)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/campaign_donation.cairo(5 hunks)src/interfaces/ICampaignDonation.cairo(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
🔇 Additional comments (3)
src/campaign_donation.cairo (3)
63-64: LGTM! Storage variables properly documentedThe storage variables are well-documented with the correct basis points format (10000 basis points).
437-453: LGTM! Protocol fee management functions implemented correctlyThe getter and setter functions are properly implemented with appropriate ownership checks for the setter functions.
582-586: LGTM! Protocol fee calculation is correctThe calculation correctly uses basis points (10000 = 100%).
| /// @return The protocol fee percentage (100 = 1%) | ||
| fn get_protocol_fee_percent(self: @TContractState) -> u256; |
There was a problem hiding this comment.
Inconsistent fee percentage documentation
The comment states "100 = 1%" but the implementation uses basis points where 100 = 1% would be represented as 100 basis points out of 10000. This should be clarified to avoid confusion.
Update the documentation to match the implementation:
- /// @return The protocol fee percentage (100 = 1%)
+ /// @return The protocol fee percentage in basis points (100 = 1%, 10000 = 100%)📝 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.
| /// @return The protocol fee percentage (100 = 1%) | |
| fn get_protocol_fee_percent(self: @TContractState) -> u256; | |
| /// @return The protocol fee percentage in basis points (100 = 1%, 10000 = 100%) | |
| fn get_protocol_fee_percent(self: @TContractState) -> u256; |
🤖 Prompt for AI Agents
In src/interfaces/ICampaignDonation.cairo at lines 227-228, the comment for
get_protocol_fee_percent incorrectly states "100 = 1%" which conflicts with the
basis points implementation. Update the comment to clarify that the fee
percentage is expressed in basis points, where 100 basis points equals 1%, to
align the documentation with the actual implementation.
| /// @param new_fee_percent The new fee percentage to set (100 = 1%) | ||
| fn set_protocol_fee_percent(ref self: TContractState, new_fee_percent: u256); |
There was a problem hiding this comment.
Inconsistent fee percentage documentation
The parameter documentation should also clarify the basis points format.
Update the documentation:
- /// @param new_fee_percent The new fee percentage to set (100 = 1%)
+ /// @param new_fee_percent The new fee percentage to set in basis points (100 = 1%, 10000 = 100%)📝 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.
| /// @param new_fee_percent The new fee percentage to set (100 = 1%) | |
| fn set_protocol_fee_percent(ref self: TContractState, new_fee_percent: u256); | |
| /// @param new_fee_percent The new fee percentage to set in basis points (100 = 1%, 10000 = 100%) | |
| fn set_protocol_fee_percent(ref self: TContractState, new_fee_percent: u256); |
🤖 Prompt for AI Agents
In src/interfaces/ICampaignDonation.cairo at lines 231-232, the documentation
for the parameter new_fee_percent is unclear about the format. Update the
comment to specify that the fee percentage is expressed in basis points, where
100 basis points equal 1%. This clarifies how to interpret the new_fee_percent
value.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/campaign_donation.cairo(6 hunks)tests/test_campaign_donation.cairo(36 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 (1)
- GitHub Check: tests
🔇 Additional comments (5)
tests/test_campaign_donation.cairo (5)
23-30: LGTM: Setup function signature updated correctly.The function signature properly includes the new protocol fee parameters to support the protocol fee management functionality.
40-41: LGTM: Protocol address configuration added.The setup correctly initializes the protocol address and includes it in the contract deployment calldata.
49-50: LGTM: Return values updated consistently.The setup function now returns the protocol owner and address as expected by the updated signature.
535-535: ```shell
#!/bin/bashSearch for any occurrences of protocol fee logic in Cairo contracts
rg protocol_fee -n --glob '*.cairo'
Locate withdrawal functions in Cairo files to inspect fee handling
rg "fn withdraw" -n --glob '*.cairo'
--- `181-181`: ```shell #!/bin/bash # Search for fee logic and transfer calls in donation/withdrawal functions rg -n "fn donate_to_campaign" -C10 --glob "*.cairo" rg -n "fn withdraw_from_campaign" -C10 --glob "*.cairo" rg -n "protocol_fee" -C5 --glob "*.cairo" rg -n "transfer(" -C3 --glob "*.cairo"
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/base/errors.cairo(1 hunks)src/campaign_donation.cairo(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/base/errors.cairo
🔇 Additional comments (4)
src/campaign_donation.cairo (4)
63-66: Storage variables are well-documented and appropriately typed.The protocol fee storage variables are correctly implemented with proper documentation explaining the basis points system.
153-162: Constructor properly initializes protocol fee address.The constructor correctly accepts and initializes the protocol fee address parameter.
443-451: Protocol fee percentage functions are correctly implemented.Both getter and setter functions are properly implemented with appropriate owner authorization and validation to prevent fees exceeding 100%.
591-595: Protocol fee calculation is mathematically correct.The fee calculation properly uses basis points (dividing by 10000) and correctly applies the percentage to the total amount.
1 participant
closes #117
This pull request introduces interface functions for managing protocol fee parameters within the contract. It provides mechanisms to retrieve and update the protocol fee percentage and the associated fee collection address.
Changes Introduced
✅ get_protocol_fee_percent: Returns the current protocol fee rate (e.g., 100 = 1%)
✅ set_protocol_fee_percent: Allows updating the protocol fee percentage
✅ get_protocol_fee_address: Returns the current address used for collecting protocol fees
✅ set_protocol_fee_address: Allows updating the fee collection address
Why It Matters
These functions are essential for enabling dynamic configuration of protocol fees and supporting future integrations with fee management systems.
Summary by CodeRabbit