Update FeeSharingCollector to withhold part of fees to the protocol#572
Update FeeSharingCollector to withhold part of fees to the protocol#572tjcloa merged 4 commits intodevelopmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a token-specific fee withholding feature to the FeeSharingCollector contract, allowing the protocol to withhold 100% of fees for specific tokens instead of distributing them to stakers. The implementation includes list management functions, withdrawal capabilities, and comprehensive test coverage.
Changes:
- Added token withhold list functionality with add/remove operations restricted to the contract owner
- Modified fee distribution logic to check if a token is in the withhold list before creating staker checkpoints
- Implemented withdrawal functions (single and batch) for accumulated protocol withheld fees with proper reentrancy protection
- Updated deployment script to automatically configure RBTC, ZUSD, and WRBTC as withheld tokens via multisig transactions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/FeeSharingCollectorTokenWithholdingTest.js | Comprehensive test suite covering list management, withholding behavior, withdrawal functions, and edge cases for the new feature |
| deployment/deploy/1020-deploy-FeeSharingCollector.js | Updated deployment script to generate multisig transactions for adding RBTC, ZUSD, and WRBTC to the withhold list |
| contracts/mockup/FeeSharingCollectorMockup.sol | Updated mockup to use the new _addCheckpointOrWithholdProtocolFee function name |
| contracts/governance/IFeeSharingCollector.sol | Added interface definitions for new functions and moved event declarations from implementation to interface |
| contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol | Added storage for protocol withhold token list (EnumerableAddressSet) and accumulated fees mapping |
| contracts/governance/FeeSharingCollector/FeeSharingCollector.sol | Renamed _addCheckpoint to _addCheckpointOrWithholdProtocolFee with logic to either withhold fees or distribute to stakers; added list management and withdrawal functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
contracts/governance/FeeSharingCollector/FeeSharingCollector.sol
Outdated
Show resolved
Hide resolved
contracts/governance/FeeSharingCollector/FeeSharingCollector.sol
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| /// @notice An event emitted when revenue is accumulated. | ||
| event ProtocolRevenueAccumulated(address indexed token, uint256 amount); |
There was a problem hiding this comment.
should we add the sender param here?
event ProtocolRevenueAccumulated(address indexed sender, address indexed token, uint256 amount);
There was a problem hiding this comment.
why do you think it matters who executed the function?
There was a problem hiding this comment.
Was thinking it's for consistency, since other event is also storing this sender as well,
FeeWithdrawnInRBTC for example
But i will leave it to u as optional
FeeSharingCollector should withhold a portion of fees for the protocol in certain tokens